Re: [PATCH 1/3] index-pack: restore "resolving deltas" progress meter
Jeff King <peff@peff.net> writes:
Show 20 quoted lines
> 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.
Show 36 quoted lines
> ---
> 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