Subject: Re: [PATCH 1/2] commit-graph: don't show progress percentages while expanding reachable commits

From: SZEDER Gábor <[email-masked]>
Date: Sat, 7 Sep 2019 12:34:07 +0200

On Sat, Sep 07, 2019 at 01:01:33AM -0400, Jeff King wrote:
> From: SZEDER Gábor <[email-masked]>
> 
> Commit 49bbc57a57 (commit-graph write: emit a percentage for all
> progress, 2019-01-19) was a bit overeager when it added progress
> percentages to the "Expanding reachable commits in commit graph" phase
> as well, because most of the time the number of commits that phase has
> to iterate over is not known in advance and grows significantly, and,
> consequently, we end up with nonsensical numbers:
> 
>   $ git commit-graph write --reachable
>   Expanding reachable commits in commit graph: 138606% (824706/595), done.
>   [...]
> 
>   $ git rev-parse v5.0 | git commit-graph write --stdin-commits
>   Expanding reachable commits in commit graph: 81264400% (812644/1), done.
>   [...]
> 
> Even worse, because the percentage grows so quickly, the progress code
> outputs much more often than it should (because it ticks every second,
> or every 1%), slowing the whole process down. My time for "git
> commit-graph write --reachable" on linux.git went from 13.463s to
> 12.521s with this patch, ~7% savings.

Oh, interesting.

> Therefore, don't show progress percentages in the "Expanding reachable
> commits in commit graph" phase.
> 
> Note that the current code does sometimes do the right thing, if we
> picked up all commits initially (e.g., omitting "--reachable" in a
> fully-packed repository would get the correct count without any parent
> traversal). So it may be possible to come up with a way to tell when we
> could use a percentage here. But in the meantime, let's make sure we
> robustly avoid printing nonsense.
> 
> Signed-off-by: SZEDER Gábor <[email-masked]>
> Signed-off-by: Jeff King <[email-masked]>
> ---
> Compared to the original from:
> 
>   https://public-inbox.org/git/[email-masked]/
> 
> I rebased it to handle code movement, added in the timing data, and
> tried to summarize the discussion from the thread.

Thanks for resurrecting this patch and for the summary paragraph.