git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jonathan Tan <jonathantanmy@google.com>, git@vger.kernel.org
Subject: [PATCH 1/3] index-pack: restore "resolving deltas" progress meter
Date: Wed, 7 Oct 2020 14:19:23 -0400	[thread overview]
Message-ID: <20201007181923.GA1976631@coredump.intra.peff.net> (raw)
In-Reply-To: <20201007181708.GA222564@coredump.intra.peff.net>

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>
---
 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
-- 
2.29.0.rc0.520.gccaf68d5cd


  reply	other threads:[~2020-10-07 18:19 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-05 22:41 [ANNOUNCE] Git v2.29.0-rc0 Junio C Hamano
2020-10-05 23:33 ` Bryan Turner
2020-10-05 23:42 ` Randall S. Becker
2020-10-06  3:57 ` Martin Ågren
2020-10-06  6:08   ` Junio C Hamano
2020-10-07  9:54 ` Ævar Arnfjörð Bjarmason
2020-10-07 15:39 ` Jeff King
2020-10-07 15:45   ` Jeff King
2020-10-07 17:38     ` Junio C Hamano
2020-10-07 18:17       ` [PATCH 0/3] jt/threaded-inex-pack leftovers Jeff King
2020-10-07 18:19         ` Jeff King [this message]
2020-10-07 18:50           ` [PATCH 1/3] index-pack: restore "resolving deltas" progress meter Junio C Hamano
2020-10-07 18:19         ` [PATCH 2/3] index-pack: drop type_cas mutex Jeff King
2020-10-07 20:09           ` Jonathan Tan
2020-10-07 18:19         ` [PATCH 3/3] index-pack: stop mentioning find_unresolved_deltas() Jeff King
2020-10-07 18:41       ` [ANNOUNCE] Git v2.29.0-rc0 Jonathan Tan
2020-10-07 18:48         ` Jeff King
2020-10-07 20:16           ` [PATCH] index-pack: make get_base_data() comment clearer Jonathan Tan
2020-10-07 20:46             ` Junio C Hamano
2020-10-07 22:28 ` [ANNOUNCE] Git v2.29.0-rc0 Philippe Blain
2020-10-09 19:51 ` Randall S. Becker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201007181923.GA1976631@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).