Re: [PATCH 1/2] commit-graph write: add progress output

[Derrick Stolee <stolee@gmail.com>]

On 9/4/2018 4:27 PM, Ævar Arnfjörð Bjarmason wrote:
> @@ -591,8 +597,13 @@ static void close_reachable(struct packed_oid_list *oids)
>   {
>   	int i;
>   	struct commit *commit;
> +	struct progress *progress = NULL;
> +	int j = 0;

The change below over-counts the number of commits we are processing (by 
at least double, possibly triple).


> +	progress = start_delayed_progress(
> +		_("Annotating commits in commit graph"), 0);
>   	for (i = 0; i < oids->nr; i++) {
> +		display_progress(progress, ++j);
>   		commit = lookup_commit(the_repository, &oids->list[i]);
>   		if (commit)
>   			commit->object.flags |= UNINTERESTING;
This count is the number of oids given to the method. For 'git 
commit-graph write --reachable', this will be the number of refs.
> @@ -604,6 +615,7 @@ static void close_reachable(struct packed_oid_list *oids)
>   	 * closure.
>   	 */
>   	for (i = 0; i < oids->nr; i++) {
> +		display_progress(progress, ++j);
>   		commit = lookup_commit(the_repository, &oids->list[i]);
>   
>   		if (commit && !parse_commit(commit))

This is the important count, since we will be parsing commits and adding 
their parents to the list. The bulk of the work happens here.

> @@ -611,19 +623,25 @@ static void close_reachable(struct packed_oid_list *oids)
>   	}
>   
>   	for (i = 0; i < oids->nr; i++) {
> +		display_progress(progress, ++j);
>   		commit = lookup_commit(the_repository, &oids->list[i]);
This iterates through the commits a second time and removes the 
UNINTERESTING flag.
>   
>   		if (commit)
>   			commit->object.flags &= ~UNINTERESTING;
>   	}
> +	stop_progress(&progress);
>   }

I think it is good to have the progress start before the first loop and 
end after the third loop, but the middle loop has the important count.

I tried deleting the first and third display_progress() methods and 
re-ran the process on the Linux repo and did not notice a delay at the 
0% and 100% progress spots. The count matches the number of commits.

Thanks,

-Stolee