SZEDER Gábor <szeder.dev@xxxxxxxxx> writes:
>> for (i = 0; i < oids->nr; i++) {
>> + display_progress(progress, ++j);
>> commit = lookup_commit(the_repository, &oids->list[i]);
>>
>> if (commit && !parse_commit(commit))
>> @@ -611,19 +624,28 @@ static void close_reachable(struct packed_oid_list *oids)
>> }
>
> The above loops have already counted all the commits, and, more
> importantly, did all the hard work that takes time and makes the
> progress indicator useful.
>
>> for (i = 0; i < oids->nr; i++) {
>> + display_progress(progress, ++j);
>
> This display_progress() call, however, doesn't seem to be necessary.
> First, it counts all commits for a second time, resulting in the ~2x
> difference compared to the actual number of commits, and then causing
> my confusion. Second, all what this loop is doing is setting a flag
> in commits that were already looked up and parsed in the above loops.
> IOW this loop is very fast, and the progress indicator jumps from
> ~780k right to 1.5M, even on my tiny laptop, so it doesn't need a
> progress indicator at all.
Makes sense. If this second iteration were also time consuming,
then it probably is a good idea to split these into two separate
phases? "Counting 1...N" followed by "Inspecting 1...N" or
something like that. Of course, if the latter does not take much
time, then doing without any progress indicator is also fine.
