From b865c2bcff53a32637aac426dd2c6ef4a4c27077 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Mon, 6 Jul 2026 15:35:42 +0200 Subject: [PATCH 1/2] commit-graph: add trace2 instrumentation for generation DFS 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 --- 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 801471a09802d9..4e39a048c46155 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 49a057cc2eb65d..f9c57760f48144 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 Date: Mon, 6 Jul 2026 15:36:47 +0200 Subject: [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers Fix a regression introduced in 199d452758 (commit-graph: fix "filling in" topological levels, 2025-04-07) where the loop propagating the topo_levels slab to each layer of the commit-graph chain always assigned to `g->topo_levels` (the topmost layer) instead of `chain->topo_levels` (the current iteration variable). This meant only the topmost layer had its topo_levels pointer set. When compute_reachable_generation_numbers() ran for an incremental write, commits parsed from lower layers had their topo levels left at zero in the slab, since fill_commit_graph_info() could not store them without the pointer. The DFS then re-walked the entire commit ancestry instead of stopping at commits with known levels. On a repository with 2.78M commits and a multi-layer split commit-graph, this caused a single incremental commit-graph write to spend ~3.7 seconds in the generation DFS instead of microseconds. Signed-off-by: Kristofer Karlsson --- commit-graph.c | 2 +- t/t5324-split-commit-graph.sh | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/commit-graph.c b/commit-graph.c index 4e39a048c46155..c2a711cceb17f3 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2610,7 +2610,7 @@ int write_commit_graph(struct odb_source *source, g = prepare_commit_graph(ctx.r); for (struct commit_graph *chain = g; chain; chain = chain->base_graph) - g->topo_levels = &topo_levels; + chain->topo_levels = &topo_levels; if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS) ctx.changed_paths = 1; diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh index f9c57760f48144..9e5ab7dbd07553 100755 --- a/t/t5324-split-commit-graph.sh +++ b/t/t5324-split-commit-graph.sh @@ -738,11 +738,7 @@ test_expect_success 'incremental write reads topo levels from all layers' ' 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