commit-reach: early termination for paint_down_to_common()#2150
Open
newren wants to merge 1 commit into
Open
Conversation
When computing merge bases, paint_down_to_common() can waste enormous
time processing STALE commits after the merge bases have already been
found. This happens when a merge commit introduces a side branch rooted
far back in history, creating a large "generation gap" that forces the
algorithm to traverse hundreds of thousands of intermediate commits to
exhaustively propagate STALE flags.
Add an optimization that checks whether all merge bases have been found
and terminates early. The key insight: a new merge base requires both
PARENT1 and PARENT2 flags to meet at a commit for the first time. Once
either side has no exclusive (non-STALE) commits left in the queue, no
new merge bases can appear because that flag can only propagate through
STALE commits which already carry both flags.
Track exclusive territory with O(1) counters (p1_exclusive_count,
p2_exclusive_count, pending_merges_count) that are maintained
incrementally:
- Incremented when commits are enqueued with exclusive flags
- Decremented when commits are dequeued with exclusive flags
- Adjusted at paint time when a commit transitions out of exclusive
status (e.g., a PARENT2-exclusive commit gains PARENT1)
The termination check is placed after parent processing (not before) to
ensure the last exclusive commit on a side has its parents fully painted
before we decide to stop; a parent might become a merge-base candidate,
in which case pending_merges_count is incremented and the check
correctly does not fire.
Safety: this optimization is gated on corrected_commit_dates_enabled(),
which guarantees child_gen > parent_gen and thus monotonic dequeue order
(parents never processed before children) for commits covered by the
commit graph. The `(p->object.flags & flags) == flags` check on the
parent-visit path also prevents enqueueing a second copy of a commit in
the same flag state, even through a diamond merge. Together these two
properties make the counters exact: each enqueue in an exclusive
non-STALE state contributes +1, balanced by either the transition that
takes the commit out of that state or by its eventual dequeue. The
counters are therefore always >= 0; runtime BUG() checks verify this
invariant.
We additionally disable the optimization whenever both sides have ever
touched a commit with GENERATION_NUMBER_INFINITY (i.e., not in the
commit graph), since those sort by raw timestamp which may not be
monotonic between parent and child. Detection happens at push time --
both at the initial pushes of `one` and `twos[]` and whenever a parent
is enqueued during the walk -- because a cross-side flag transition on
an already-dequeued INFINITY parent can otherwise desync the exclusive
counts before the gate would fire if checked only at dequeue. When only
one side has INFINITY commits, the other side's territory stays entirely
in graph space (commit-graph writes are ancestry-closed) and the
optimization remains active. In that disabled case the counters are no
longer maintained and the BUG() checks are correspondingly suppressed.
The optimization can therefore help only when the corrected-date commit
graph covers at least one side of the queried tips (e.g., after
`git maintenance run` or `git commit-graph write --reachable`).
In repo_in_merge_bases_many(), short-circuit when the queried commit is
itself one of the references: a commit is trivially reachable from
itself, and detecting this avoids invoking paint_down_to_common() with
`one == twos[i]`, which would paint PARENT1|PARENT2 on the same commit
during the initial pushes and trip the counter invariants described
above. Add an `in_merge_bases_many:self` case to t6600-test-reach.sh to
guard against this regression.
Also guard against duplicate entries within twos[] at the initial-push
site: skip pushing twos[i] if PARENT2 is already set on it. Without
this guard, a duplicate inflates p2_exclusive_count by one extra,
leaving a "phantom" count that can delay the early-termination check
(though it never affects correctness, since the other side eventually
exhausts its exclusive territory). Add a
`get_merge_bases_many:duplicate-twos` case to t6600-test-reach.sh that
exercises this with the corrected-date commit graph in place.
Also account for an additional flag transition in the counter logic:
when a descendant merge-base propagates STALE up to a commit that is
still a pending (PARENT1|PARENT2) candidate, the candidate transitions
to (PARENT1|PARENT2|STALE) before being dequeued. At its later dequeue
the dequeue-time decrement does not fire (the STALE bit causes the
(PARENT1|PARENT2)&&!RESULT check to fail), so without compensation
pending_merges_count would be permanently inflated by one and the
early-termination check would silently never fire for the rest of the
walk. Decrement pending_merges_count at the transition site to keep the
count exact (guarded by !RESULT to avoid decrementing twice when STALE
arrives at an already-recorded merge-base). Add a
`get_merge_bases_many:pending-stale` case to t6600-test-reach.sh that
constructs a side topology where one merge-base candidate is a
descendant of another, so that STALE propagation from the recorded
candidate reaches a still-pending one. The 10x10 grid in t6600 does not
exercise this transition because no merge-base candidate there is a
descendant of another.
Add a `get_merge_bases_many:infinity-both-sides` case to
t6600-test-reach.sh covering the INFINITY-gate. It builds a tiny side
topology with non-monotonic commit dates whose tips are outside the half
commit-graph, so both sides enter the walk with INFINITY generation.
Without the push-time gate, the walk would underflow p1_exclusive_count
when the merge-base candidate propagates STALE up to an already-dequeued
INFINITY parent.
Broad correctness of the optimization is already covered by the existing
t6600-test-reach.sh suite: run_all_modes runs every reach test under
four commit-graph configurations -- no graph, full graph, partial graph,
and graph without corrected dates -- exercising both the
use_early_termination=0 path and the optimization-enabled path,
including partial-graph mode where queried tips may be outside the
commit graph. The new `infinity-both-sides` case specifically hits the
initial push-time INFINITY gate in that partial-graph mode. The few new
cases above only target the corner cases this patch specifically
introduces.
Add p6012-merge-base-deep-side-branch.sh to demonstrate the speedup with
a synthetic repository. The test creates a 500K-commit main branch with
a side branch forked from commit 10K that has old timestamps. It then
merges the side branch at the tip. A PR branch forks from main just
before the merge. Computing merge-base(main, pr) must discover that the
merge-base is at the tip of main (just below the merge), but without
early termination, the algorithm chases STALE flags all the way back
through the 490K commits between the side branch fork point and the
merge-base. The test also verifies the merge-base result for
correctness.
Performance on a large production repository:
Before: merge-base HEAD PR_head = 0.68s (334K wasted iterations)
After: merge-base HEAD PR_head = 0.01s (terminates at ~50 iterations)
No regressions on other merge-base queries (all faster or same).
Assisted-by: Claude Opus 4.6/4.7 & GPT 5.5
Signed-off-by: Elijah Newren <newren@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a patch I discovered last month (and which Patrick Lühne independently discovered earlier this week), and is now live at GitHub. It feels slightly incomplete in that it aborts when both sides have an infinite generation number -- I think it could be extended to that case, but due to other fires, I didn't have the time to complete it.
...the optimization was also independently discovered by Kristoffer Karlsson; see https://lore.kernel.org/git/CAL71e4PD+zT2jLjjvC7EuYX5z6v_VafnWOUDHeEDBq2LGOK7Pw@mail.gmail.com/T/#mcf16970e250e7caf74f090cc48545b3b947949fd.