git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 7, 2020, 18:50 UTC
Message-ID
<xmqqlfgh27lq.fsf@gitster.c.googlers.com>
In-Reply-To
<20201007181923.GA1976631@coredump.intra.peff.net>
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
Previous: Jeff KingNext: Jeff King
Message 12 of 21 in “[ANNOUNCE] Git v2.29.0-rc0”
  1. Junio C HamanoOct 5, 2020
  2. Bryan TurnerOct 5, 2020
  3. Randall S. BeckerOct 5, 2020
  4. Martin ÅgrenOct 6, 2020
  5. Junio C HamanoOct 6, 2020
  6. Ævar Arnfjörð BjarmasonOct 7, 2020
  7. Jeff KingOct 7, 2020
  8. Jeff KingOct 7, 2020
  9. Junio C HamanoOct 7, 2020
  10. 0/3 jt/threaded-inex-pack leftoversJeff King, Oct 7, 2020
  11. 1/3 index-pack: restore "resolving deltas" progress meterJeff King, Oct 7, 2020
  12. Junio C HamanoOct 7, 2020
  13. 2/3 index-pack: drop type_cas mutexJeff King, Oct 7, 2020
  14. Jonathan TanOct 7, 2020
  15. 3/3 index-pack: stop mentioning find_unresolved_deltas()Jeff King, Oct 7, 2020
  16. Jonathan TanOct 7, 2020
  17. Jeff KingOct 7, 2020
  18. index-pack: make get_base_data() comment clearerJonathan Tan, Oct 7, 2020
  19. Junio C HamanoOct 7, 2020
  20. Philippe BlainOct 7, 2020
  21. Randall S. BeckerOct 9, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.