commit-reach: remove commit-date ordering fallback
Remove the fallback that switched paint_down_to_common() from generation ordering to commit-date ordering when the commit-graph lacks corrected commit dates (v1 graph with topo levels only). The fallback was added in 091f4cf3 (commit: don't use generation numbers if not needed, 2018-08-30) to avoid a performance regression on the Linux kernel repo where v1 topo levels caused "git merge-base v4.8 v4.9" to walk 636k commits instead of 167k. A side branch with a low topo level stayed in the queue behind a long chain, preventing early STALE propagation. Side-exhaustion (added in the previous commits) solves this differently by terminating the walk as soon as one paint side empties from the queue, preventing the deep walk regardless of queue ordering. Benchmarks of "git merge-base --all v4.8 v4.9" on the Linux kernel repo show that side-exhaustion reduces the step count far below what the date-ordering fallback achieved: steps time no graph, baseline: 167,413 3.25 s v1 graph, baseline: 167,413 0.25 s v2 graph, baseline: 167,441 0.29 s v1 graph, this series: 5,725 0.02 s v2 graph, this series: 3,887 0.01 s With generation ordering always active, the existing min_generation check in paint_queue_get() correctly identifies when the walk has reached the finite generation region. The date ordering fallback broke this invariant: a commit could have a finite topo level while the queue was date-ordered, causing the early exit to fire before all merge bases were found. Also remove corrected_commit_dates_enabled() from commit-graph.c which has no remaining callers. Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Kristofer Karlsson committed
Jul 11, 2026 at 13:27 UTC
db8791aaf25f182a2a80126c018c97d3ece198f4
5 files changed
+21
-85
Documentation/technical/paint-down-to-common.adoc
+6
-45
@@ -44,10 +44,6 @@ ancestor is necessarily redundant.
44
INFINITY and finite generation regions
45
--------------------------------------
46
47
-The properties in this section assume generation-number ordering (the
48
-default comparator). They do NOT hold when the date-ordering fallback
49
-is active -- see <<date-ordering-fallback>>.
50
-
47
The commit-graph stores a generation number for each commit.
48
Commits not in the commit-graph have generation
49
`GENERATION_NUMBER_INFINITY`. The graph is closed under
@@ -82,10 +78,12 @@ traversal: children are always visited before their parents. This
78
means that paint on already-visited commits is final -- no future
79
traversal step can add paint to them.
80
85
-In the INFINITY region, commit-date ordering can violate this: a
86
-parent with a later date can be visited before a child with an earlier
87
-date. Paint flags are therefore NOT final at visit time, and a
88
-commit visited with only one side's paint may later gain the other.
81
+In the INFINITY region, all commits share the same generation
82
+value, so the queue breaks ties by commit date. This can violate
83
+topological ordering: a parent with a later date can be visited
84
+before a child with an earlier date. Paint flags are therefore
85
+NOT final at visit time, and a commit visited with only one
86
+side's paint may later gain the other.
87
88
Paint flags are only added, never removed. Since each flag can be set
89
at most once per commit, the number of times a commit can be
@@ -149,43 +147,6 @@ descendant of this candidate (generation ordering guarantees
147
children are visited first), so it cannot be redundant and the walk
148
can stop immediately.
149
152
-This optimization is NOT safe when the date-ordering fallback is
153
-active, because commit-date order can visit a deeper ancestor
154
-before a shallower one -- see <<date-ordering-fallback>>.
155
-
156
-[[date-ordering-fallback]]
157
-Date-ordering fallback
158
-----------------------
159
-
160
-When the commit-graph has generation numbers v1 and no
161
-generation floor is specified, topological ordering
162
-(via generation numbers) is disabled. Topological levels are
163
-correct but unbalanced -- ordering by such generation numbers
164
-can sometimes cause the walk to detour too far before finding
165
-merge bases. Commit-date ordering typically reaches them in
166
-fewer steps -- see this change for more details:
167
-
168
- 091f4cf3 (commit: don't use generation numbers if not needed,
169
- 2018-08-30)
170
-
171
-With generation number v2 (corrected commit dates) we have the best
172
-of both worlds and do not need this fallback.
173
-
174
-For v1, `paint_down_to_common()` falls back to pure commit-date
175
-ordering via `compare_commits_by_commit_date`. Because commit
176
-dates are not monotonic (clock skew, rebases, etc.), the queue
177
-may visit commits out of topological order.
178
-
179
-This disables the optimizations that depend on generation ordering:
180
-
181
- - *Single result*: the first merge-base candidate found may not
182
- be the shallowest, because a deeper ancestor with a higher
183
- commit date can be dequeued first.
184
-
185
- - *Side exhaustion*: one paint side can appear to drain from the
186
- queue while commits from that side are still waiting with lower
187
- dates, causing premature termination.
188
-
150
Related documentation
151
---------------------
152
commit-graph.c
-11
@@ -793,17 +793,6 @@ int generation_numbers_enabled(struct repository *r)
793
return !!first_generation;
794
}
795
796
-int corrected_commit_dates_enabled(struct repository *r)
797
-{
798
- struct commit_graph *g;
799
-
800
- g = prepare_commit_graph(r);
801
- if (!g || !g->num_commits)
802
- return 0;
803
-
804
- return g->read_generation_data;
805
-}
806
-
796
struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)
797
{
798
struct commit_graph *g;
commit-graph.h
-6
@@ -136,12 +136,6 @@ struct commit_graph *parse_commit_graph(struct repository *r,
136
*/
137
int generation_numbers_enabled(struct repository *r);
138
139
-/*
140
- * Return 1 if and only if the repository has a commit-graph
141
- * file and generation data chunk has been written for the file.
142
- */
143
-int corrected_commit_dates_enabled(struct repository *r);
144
-
139
struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r);
140
141
enum commit_graph_write_flags {
commit-reach.c
+6
-9
@@ -89,7 +89,6 @@ struct paint_state {
89
size_t parent1_count;
90
size_t parent2_count;
91
size_t mb_candidate_count;
92
- int gen_ordered;
92
timestamp_t min_generation;
93
timestamp_t last_gen;
94
};
@@ -166,7 +165,6 @@ static struct commit *paint_queue_get(struct paint_state *state)
165
166
/* one side is exhausted */
167
if ((!state->parent1_count || !state->parent2_count) &&
169
- state->gen_ordered &&
168
generation < GENERATION_NUMBER_INFINITY)
169
return NULL;
170
}
@@ -187,9 +185,13 @@ static int paint_down_to_common(struct repository *r,
185
enum merge_base_flags mb_flags,
186
struct commit_list **result)
187
{
188
+ /*
189
+ * Generation ordering is required for the side-exhaustion and
190
+ * single-result early exits, which rely on topological traversal
191
+ * order (children visited before parents) in the finite region.
192
+ */
193
struct paint_state state = {
191
- .queue = { compare_commits_by_gen_then_commit_date },
192
- .gen_ordered = 1,
194
+ .queue = { compare_commits_by_gen_then_commit_date }
195
};
196
struct commit *commit;
197
int i;
@@ -198,10 +200,6 @@ static int paint_down_to_common(struct repository *r,
200
201
state.min_generation = min_generation;
202
state.last_gen = GENERATION_NUMBER_INFINITY;
201
- if (!min_generation && !corrected_commit_dates_enabled(r)) {
202
- state.queue.compare = compare_commits_by_commit_date;
203
- state.gen_ordered = 0;
204
- }
203
204
one->object.flags |= PARENT1;
205
if (!n) {
@@ -229,7 +227,6 @@ static int paint_down_to_common(struct repository *r,
227
* descendant of this one.
228
*/
229
if (!(mb_flags & MERGE_BASE_FIND_ALL) &&
232
- state.gen_ordered &&
230
state.last_gen < GENERATION_NUMBER_INFINITY)
231
break;
232
}
t/t6600-test-reach.sh
+9
-14
@@ -381,7 +381,7 @@ test_expect_success 'get_merge_bases_many:infinity-both-sides' '
381
git rev-parse pi-B
382
} >expect &&
383
test_all_modes get_merge_bases_many &&
384
- test_paint_down_steps 5 4 5 5
384
+ test_paint_down_steps 5 4 5 4
385
'
386
387
test_expect_success 'setup mixed finite/INFINITY topology' '
@@ -414,31 +414,26 @@ test_expect_success 'merge-base --all commit-walk steps' '
414
>input &&
415
git rev-parse commit-9-1 >expect &&
416
run_all_modes git merge-base --all commit-9-9 commit-9-1 &&
417
- test_paint_down_steps 81 9 57 81
417
+ test_paint_down_steps 81 9 57 37
418
'
419
420
test_expect_success 'merge-base --all with clock skew (side-exhaustion)' '
421
- # Verify correct merge base under clock skew. se-D (the
422
- # merge base) has a higher date than its child se-C.
423
- # Generation ordering ensures se-C is visited before se-D,
424
- # so P1 paint propagates correctly and se-D is found.
421
+ # Verify that the merge base is computed correctly even
422
+ # when commits have non-monotonic commit dates.
423
>input &&
424
git rev-parse se-D >expect &&
425
run_all_modes git merge-base --all se-A se-B &&
428
- test_paint_down_steps 6 4 6 6
426
+ test_paint_down_steps 6 4 6 4
427
'
428
429
test_expect_success 'merge-base --all with clock skew and redundant ancestor (side-exhaustion)' '
432
- # Verify correct merge base when clock skew could cause a
433
- # too-deep result. MB1 is the correct merge base; MB2 is
434
- # its ancestor. A reaches MB2 via E (high date) and MB1
435
- # via C (low date). Generation ordering ensures C is
436
- # visited before side-exhaustion fires, so MB1 is found
437
- # and remove_redundant correctly discards MB2.
430
+ # Verify that the correct merge base is found even when
431
+ # non-monotonic commit dates could cause a redundant
432
+ # ancestor to be visited first.
433
>input &&
434
git rev-parse se2-MB1 >expect &&
435
run_all_modes git merge-base --all se2-A se2-B &&
441
- test_paint_down_steps 8 6 8 8
436
+ test_paint_down_steps 8 6 8 6
437
'
438
439
test_expect_success 'reduce_heads' '