Re: [PATCH 1/3] index-pack: restore "resolving deltas" progress meter

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> Commit f08cbf60fe (index-pack: make quantum of work smaller, 2020-09-08)
> refactored the main loop in threaded_second_pass(), but also deleted the
> call to display_progress() at the top of the loop. This means that users
> typically see no progress at all during the delta resolution phase (and
> for large repositories, Git appears to hang).
>
> This looks like an accident that was unrelated to the intended change of
> that commit, since we continue to update nr_resolved_deltas in
> resolve_delta(). Let's restore the call to get that progress back.
>
> We'll also add a test that confirms we generate the expected progress.
> This isn't perfect, as it wouldn't catch a bug where progress was
> delayed to the end. That was probably possible to trigger when receiving
> a thin pack, because we'd eventually call display_progress() from
> fix_unresolved_deltas(), but only once after doing all the work.
> However, since our test case generates a complete pack, it reliably
> demonstrates this particular bug and its fix. And we can't do better
> without making the test racy.
>
> Signed-off-by: Jeff King <peff@peff.net>

Thanks.  Queued with Ack from jtan.

> ---
>  builtin/index-pack.c  | 4 ++++
>  t/t5302-pack-index.sh | 7 +++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 8da4031e72..0f05533902 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1028,6 +1028,10 @@ static void *threaded_second_pass(void *data)
>  		struct object_entry *child_obj;
>  		struct base_data *child;
>  
> +		counter_lock();
> +		display_progress(progress, nr_resolved_deltas);
> +		counter_unlock();
> +
>  		work_lock();
>  		if (list_empty(&work_head)) {
>  			/*
> diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
> index c92e553a2f..7c9d687367 100755
> --- a/t/t5302-pack-index.sh
> +++ b/t/t5302-pack-index.sh
> @@ -277,4 +277,11 @@ test_expect_success 'index-pack --fsck-objects also warns upon missing tagger in
>  	grep "^warning:.* expected .tagger. line" err
>  '
>  
> +test_expect_success 'index-pack -v --stdin produces progress for both phases' '
> +	pack=$(git pack-objects --all pack </dev/null) &&
> +	GIT_PROGRESS_DELAY=0 git index-pack -v --stdin <pack-$pack.pack 2>err &&
> +	test_i18ngrep "Receiving objects" err &&
> +	test_i18ngrep "Resolving deltas" err
> +'
> +
>  test_done