All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Han-Wen Nienhuys" <hanwen@google.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 12/12] gc + reflog: don't stall before initial "git gc" progress output
Date: Tue, 30 Nov 2021 22:38:14 +0100	[thread overview]
Message-ID: <patch-12.12-fc2b15d0abe-20211130T213319Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com>

Change the "git reflog expire --all" progress output to show progress
on mark_reachable() in addition to the "Expiring reflogs" output added
in the preceding commit.

This is a major UI improvement to the "git gc" progress output, which
in large repositories would seem to hang before the "initial"
"Enumerating objects" phase. E.g. on a linux.git checkout I've got it
previously took me around 10 seconds before I saw any output.

With the change to add the "Expiring reflogs" output in the preceding
commit that ~10 second delay was perhaps ~8-9, now it's pretty much
guaranteed to show output within the first couple of seconds (our
default progress delay).

Why? Because as we iterate through our reflogs to expire in
reflog_exire() we'd always check the reflog for "HEAD" first, and as
we hadn't previously marked anything REACHABLE in mark_reachable()
would almost certainly end up needing to walk a substantial amount of
history to mark commits. In linux.git north of 1 million commits.

Now we'll instead show:

    $ git gc
    Iterating (un)reachable HEAD: 1137354, done.
    Expiring reflogs: 100% (45/45), done.
    ^Enmerating objects: 662499^C
    [...]

The implementation is a bit of a hack. Ideally we'd support nested
progress bars, and the ability to "attach" a sub-progress bar to a
running one. I.e. to optimistically show either:

    Expiring reflogs: X (Y/Z)

Or:

    Expiring reflogs: X (Y/Z) iterating unreachable HEAD: 123456

In this case the problem is that if we have done our initial big
"REACHABLE" iteration we're usually going to process the reflog quite
quickly, but if we haven't we'd stall.

So as a hack pass down a "show_progress" to reflog_expiry_prepare(),
which will only be shown if we haven't processed the UE_HEAD
case. We'll then start a progress bar, which when passed through to
mark_reachable() will almost certainly result in a substantial initial
iteration.

Then when we've done that initial walk we'll stop that progress bar in
our main loop, and start the "Expiring reflogs" progress bar. We'll
usually start it at 2/X, since our "HEAD" was the 1/X usurped by the
"Iterating (un)reachable HEAD" progress bar.

The "usually" would assume that we wouldn't hit the
"cb->no_reachable_progress = 1" case being added here. I.e. if our
configuration is such that we're not going to do the UE_HEAD walk
we'll just fall back on only using "Expiring reflogs". We'll still
start the count at the 2nd reflog, but it shouldn't matter, on e.g. my
linux.git checkout it doesn't, the progress bar goes by too fast to
notice.

It would have been nicer to be able to compute the
"cb->unreachable_expire_kind" before we call reflog_expire(), but to
do that we'd need the "oid", which we'll only know once we lock it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/reflog.c | 53 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 1a2a210ecf1..ef4f039a20c 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -47,7 +47,11 @@ struct expire_reflog_policy_cb {
 	struct commit *tip_commit;
 	struct commit_list *tips;
 	unsigned int dry_run:1,
-		     verbose:1;
+		     verbose:1,
+		     show_progress:1,
+		     no_reachable_progress:1;
+	struct progress *reachable_progress;
+	uint64_t reachable_progress_cnt;
 };
 
 struct worktree_reflogs {
@@ -235,6 +239,8 @@ static void mark_reachable(struct expire_reflog_policy_cb *cb)
 	while (pending) {
 		struct commit_list *parent;
 		struct commit *commit = pop_commit(&pending);
+
+		display_progress(cb->reachable_progress, ++cb->reachable_progress_cnt);
 		if (commit->object.flags & REACHABLE)
 			continue;
 		if (parse_commit(commit))
@@ -375,16 +381,27 @@ static void reflog_expiry_prepare(const char *refname,
 		cb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;
 	}
 
-	if (cb->cmd.expire_unreachable <= cb->cmd.expire_total)
+	if (cb->cmd.expire_unreachable <= cb->cmd.expire_total) {
+		if (cb->show_progress &&
+		    cb->unreachable_expire_kind == UE_HEAD)
+			cb->no_reachable_progress = 1;
 		cb->unreachable_expire_kind = UE_ALWAYS;
+	}
 
 	switch (cb->unreachable_expire_kind) {
 	case UE_ALWAYS:
 		return;
 	case UE_HEAD:
+		if (cb->show_progress) {
+			const char *s = _("Iterating (un)reachable HEAD");
+			cb->reachable_progress = start_delayed_progress(s, 0);
+		}
 		for_each_ref(push_tip_to_list, &cb->tips);
-		for (elem = cb->tips; elem; elem = elem->next)
+		for (elem = cb->tips; elem; elem = elem->next) {
+			display_progress(cb->reachable_progress,
+					 ++cb->reachable_progress_cnt);
 			commit_list_insert(elem->item, &cb->mark_list);
+		}
 		break;
 	case UE_NORMAL:
 		commit_list_insert(commit, &cb->mark_list);
@@ -633,9 +650,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 		struct worktree_reflogs collected = {
 			.reflogs = STRING_LIST_INIT_NODUP,
 		};
-		struct string_list_item *item;
 		struct worktree **worktrees, **p;
-		uint64_t progress_cnt;
+		int show_head_progress = show_progress;
+		size_t j;
 
 		if (show_progress)
 			collected.progress = start_delayed_progress(_("Enumerating reflogs"),
@@ -652,27 +669,31 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 		free_worktrees(worktrees);
 		stop_progress(&collected.progress);
 
-		if (show_progress) {
-			progress_cnt = 0;
-			progress = start_delayed_progress(_("Expiring reflogs"),
-							  collected.reflogs.nr);
-		}
-
-		for_each_string_list_item(item, &collected.reflogs) {
+		for (j = 0; j < collected.reflogs.nr; j++) {
+			const char *string = collected.reflogs.items[j].string;
 			struct expire_reflog_policy_cb cb = {
 				.cmd = cmd,
 				.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
 				.verbose = verbose,
+				.show_progress = show_head_progress,
 			};
-
-			display_progress(progress, ++progress_cnt);
-			set_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);
-			status |= reflog_expire(item->string, flags,
+			display_progress(progress, j + 1);
+			set_reflog_expiry_param(&cb.cmd, explicit_expiry, string);
+			status |= reflog_expire(string, flags,
 						reflog_expiry_prepare,
 						should_expire_reflog_ent,
 						reflog_expiry_cleanup,
 						&cb);
+
+			if (show_head_progress &&
+			    (cb.reachable_progress || cb.no_reachable_progress)) {
+				stop_progress(&cb.reachable_progress);
+				show_head_progress = 0;
+				progress = start_delayed_progress(_("Expiring reflogs"),
+								  collected.reflogs.nr);
+			}
 		}
+		display_progress(progress, collected.reflogs.nr); /* only HEAD? */
 		stop_progress(&progress);
 		collected.reflogs.strdup_strings = 1;
 		string_list_clear(&collected.reflogs, 0);
-- 
2.34.1.877.g7d5b0a3b8a6


  parent reply	other threads:[~2021-11-30 21:39 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-30 21:38 [PATCH 00/12] reflog + gc: refactor, progress output, reftable-readyness Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 01/12] reflog delete: narrow scope of "cmd" passed to count_reflog_ent() Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 02/12] reflog expire: narrow scope of "cb" in cmd_reflog_expire() Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 03/12] reflog: change one->many worktree->refnames to use a string_list Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 04/12] reflog expire: don't do negative comparison on enum values Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 05/12] reflog expire: refactor & use "tip_commit" only for UE_NORMAL Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 06/12] reflog expire: don't use lookup_commit_reference_gently() Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 07/12] reflog: reduce scope of "struct rev_info" Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 08/12] refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 09/12] reflog + refs-backend: move "verbose" out of the backend Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 10/12] reflog expire: add progress output on --stale-fix Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` [PATCH 11/12] gc + reflog: emit progress output from "reflog expire --all" Ævar Arnfjörð Bjarmason
2021-11-30 21:38 ` Ævar Arnfjörð Bjarmason [this message]
2021-12-16 13:45 ` [PATCH v2 0/9] reftable prep: have builtin/reflog.c handle "--verbose" Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 1/9] reflog delete: narrow scope of "cmd" passed to count_reflog_ent() Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 2/9] reflog expire: narrow scope of "cb" in cmd_reflog_expire() Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 3/9] reflog: change one->many worktree->refnames to use a string_list Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 4/9] reflog expire: don't do negative comparison on enum values Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 5/9] reflog expire: refactor & use "tip_commit" only for UE_NORMAL Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 6/9] reflog expire: don't use lookup_commit_reference_gently() Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 7/9] reflog: reduce scope of "struct rev_info" Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 8/9] refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN Ævar Arnfjörð Bjarmason
2021-12-16 13:45   ` [PATCH v2 9/9] reflog + refs-backend: move "verbose" out of the backend Ævar Arnfjörð Bjarmason
2021-12-16 22:27   ` [PATCH v2 0/9] reftable prep: have builtin/reflog.c handle "--verbose" Junio C Hamano
2021-12-16 23:31     ` Ævar Arnfjörð Bjarmason
2021-12-21 14:24   ` Han-Wen Nienhuys
2021-12-21 15:21     ` Ævar Arnfjörð Bjarmason
2021-12-22  4:06   ` [PATCH v3 " Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 1/9] reflog delete: narrow scope of "cmd" passed to count_reflog_ent() Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 2/9] reflog expire: narrow scope of "cb" in cmd_reflog_expire() Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 3/9] reflog: change one->many worktree->refnames to use a string_list Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 4/9] reflog expire: use "switch" over enum values Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 5/9] reflog expire: refactor & use "tip_commit" only for UE_NORMAL Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 6/9] reflog expire: don't use lookup_commit_reference_gently() Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 7/9] reflog: reduce scope of "struct rev_info" Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 8/9] refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN Ævar Arnfjörð Bjarmason
2021-12-22  4:06     ` [PATCH v3 9/9] reflog + refs-backend: move "verbose" out of the backend Ævar Arnfjörð Bjarmason

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

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

  git send-email \
    --in-reply-to=patch-12.12-fc2b15d0abe-20211130T213319Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hanwen@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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.