All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Kristofer Karlsson <krka@spotify.com>,
	Kristofer Karlsson <krka@spotify.com>
Subject: [PATCH 1/2] commit-graph: add trace2 instrumentation for generation DFS
Date: Tue, 07 Jul 2026 09:59:42 +0000	[thread overview]
Message-ID: <b865c2bcff53a32637aac426dd2c6ef4a4c27077.1783418384.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2170.git.1783418384.gitgitgadget@gmail.com>

From: Kristofer Karlsson <krka@spotify.com>

Add a step counter and trace2_data_intmax call to
compute_reachable_generation_numbers() to make the cost of
the generation number DFS observable.  This exposes a
regression introduced in 199d452758 (commit-graph: fix
"filling in" topological levels, 2025-04-07) where
incremental commit-graph writes re-walk the entire commit
ancestry instead of reading topo levels from lower graph
layers.

Add a test that demonstrates the problem: with a two-layer
split commit-graph, writing a new incremental layer for a
commit whose parent is in the base layer walks all the way
down to the root (7 steps for 5 base commits) instead of
reading the existing topo level and stopping immediately
(1 step).

Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
 commit-graph.c                |  5 +++++
 t/t5324-split-commit-graph.sh | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/commit-graph.c b/commit-graph.c
index 801471a098..4e39a048c4 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1653,6 +1653,7 @@ static void compute_reachable_generation_numbers(
 {
 	int i;
 	struct commit_list *list = NULL;
+	intmax_t steps = 0;
 
 	for (i = 0; i < info->commits->nr; i++) {
 		struct commit *c = info->commits->items[i];
@@ -1671,6 +1672,7 @@ static void compute_reachable_generation_numbers(
 			int all_parents_computed = 1;
 			timestamp_t max_gen = 0;
 
+			steps++;
 			for (parent = current->parents; parent; parent = parent->next) {
 				repo_parse_commit(info->r, parent->item);
 				gen = info->get_generation(parent->item, info->data);
@@ -1694,6 +1696,9 @@ static void compute_reachable_generation_numbers(
 			}
 		}
 	}
+
+	trace2_data_intmax("commit-graph", info->r,
+			   "generation-dfs-steps", steps);
 }
 
 static timestamp_t get_topo_level(struct commit *c, void *data)
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index 49a057cc2e..f9c57760f4 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -718,6 +718,34 @@ test_expect_success 'write generation data chunk when commit-graph chain is repl
 	)
 '
 
+test_expect_success 'incremental write reads topo levels from all layers' '
+	git init topo-from-lower &&
+	(
+		cd topo-from-lower &&
+
+		for i in $(test_seq 5)
+		do
+			test_commit base-$i || return 1
+		done &&
+		git commit-graph write --reachable &&
+
+		test_commit extra &&
+		git commit-graph write --reachable --split=no-merge &&
+
+		git checkout base-3 &&
+		test_commit new-branch &&
+
+		GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
+			git commit-graph write --reachable --split=no-merge &&
+
+		# BUG: topo levels from lower graph layers are not
+		# propagated, so the DFS re-walks from base-3 down to
+		# the root (7 steps) instead of reading topo levels
+		# from the existing graph (1 step).
+		test_trace2_data commit-graph generation-dfs-steps 7 <trace.txt
+	)
+'
+
 test_expect_success 'temporary graph layer is discarded upon failure' '
 	git init layer-discard &&
 	(
-- 
gitgitgadget


  reply	other threads:[~2026-07-07  9:59 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  9:59 [PATCH 0/2] commit-graph: fix topo_levels slab propagation regression Kristofer Karlsson via GitGitGadget
2026-07-07  9:59 ` Kristofer Karlsson via GitGitGadget [this message]
2026-07-07 13:46   ` [PATCH 1/2] commit-graph: add trace2 instrumentation for generation DFS Taylor Blau
2026-07-07 14:08     ` Kristofer Karlsson
2026-07-10 22:09       ` Taylor Blau
2026-07-10 22:28         ` Junio C Hamano
2026-07-10 22:56           ` Taylor Blau
2026-07-11 21:18             ` Junio C Hamano
2026-07-13 19:55               ` Kristofer Karlsson
2026-07-13 20:42                 ` Junio C Hamano
2026-07-13 22:17                   ` Kristofer Karlsson
2026-07-07 16:55   ` Junio C Hamano
2026-07-07 17:39     ` Kristofer Karlsson
2026-07-07  9:59 ` [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers Kristofer Karlsson via GitGitGadget
2026-07-07 13:49   ` Taylor Blau
2026-07-07 14:02     ` Kristofer Karlsson
2026-07-07 14:57     ` Kristofer Karlsson
2026-07-10 22:14       ` Taylor Blau
2026-07-13  6:16         ` Patrick Steinhardt
2026-07-14  3:31           ` Taylor Blau
2026-07-07 17:00   ` Junio C Hamano
2026-07-07 17:42     ` Kristofer Karlsson
2026-07-07 20:13       ` Junio C Hamano
2026-07-09 13:43   ` Patrick Steinhardt
2026-07-09 15:02 ` [PATCH v2 0/2] commit-graph: fix topo_levels slab propagation regression Kristofer Karlsson via GitGitGadget
2026-07-09 15:03   ` [PATCH v2 1/2] commit-graph: add trace2 instrumentation for generation DFS Kristofer Karlsson via GitGitGadget
2026-07-09 15:03   ` [PATCH v2 2/2] commit-graph: propagate topo_levels slab to all chain layers Kristofer Karlsson via GitGitGadget
2026-07-10 22:15   ` [PATCH v2 0/2] commit-graph: fix topo_levels slab propagation regression Taylor Blau

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=b865c2bcff53a32637aac426dd2c6ef4a4c27077.1783418384.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=krka@spotify.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.