commit-reach: terminate merge-base walk when one side is exhausted

[spkrka]

Optimize paint_down_to_common() for merge-base queries that hit
large one-sided histories.

When the walk from one side reaches a commit with a very low
generation number that the other side never paints, the walk is
forced to drain most of the graph. A common trigger is a
repository import that grafts a separate history with its own root,
but any merge that introduces a low-generation commit never painted
by the other side has the same effect.

A new merge-base candidate can only be discovered when exclusive
PARENT1 and PARENT2 paint meet. This series teaches
paint_down_to_common() to stop as soon as one side has no exclusive
commits left in the queue; once one side is exhausted, no further
candidates can appear.

origin/HEAD  o   o  PR HEAD
             |   |
   (import)  o   :
            / \ /
           |   o  merge-base
           |   |
           :   :  (~2.5M commits)
           |   |
import root   main root

In the RFC thread [1], Derrick Stolee provided a criss-cross
counterexample that sharpened the halt condition, and Elijah Newren
independently discovered the same optimization and shared an
implementation in PR #2150 [2]. Patch 3 incorporates test
cases from Elijah's branch.

This series implements the optimization only after the walk enters
the ordered region, where generation ordering guarantees that paint
on visited commits is final.

Patch 2 adds a test_trace2_data_singular helper to
test-lib-functions.sh that reports expected/actual values on
assertion failure instead of a silent grep exit. This was
invaluable during development for iterating on step counts
across the series, and should be valuable for repairing tests
after future algorithmic changes. Happy to drop it if it is
considered unnecessary infrastructure.

The final patch removes the commit-date ordering fallback
introduced by 091f4cf (commit: don't use generation numbers
if not needed, 2018-08-30). With side-exhaustion in place,
the fallback is no longer needed for performance, and removing it
ensures the queue is always generation-ordered regardless of graph
version, so every termination condition can rely on a single
ordering invariant. This patch can be dropped if
the scope is too broad for this series.

Benchmarks

Trace2 step counts are deterministic (measured via
trace2_data_intmax added in patch 5). Wall-clock times are
best-of-11 runs.

2.6M-commit monorepo with commit-graph:

                                      steps              wall-clock
merge-base --all  (across import)  2143438 ->      3     3.67s ->    5ms
merge-base --all  (1000 apart)     2692915 ->   1035     4.41s ->    7ms
merge-base --all  (5000 apart)     2692915 ->   6401     4.45s ->   13ms
merge-base --all  (HEAD vs import) 2698872 ->  45960     4.50s ->   79ms
merge-tree        (across import)  2143438 ->      3     4.42s ->   11ms

git.git (88k commits, commit-graph):

                                      steps              wall-clock
merge-base --all v2.0.0 v2.55.0-rc1 72264 ->  44589      110ms ->   68ms
merge-base --all HEAD HEAD~1000      9891 ->   3828       18ms ->   10ms
merge-base --all HEAD HEAD~10000    72303 ->  41487      101ms ->   50ms

This series is based on master.

[1] https://lore.kernel.org/git/CAL71e4Ps-2_0+uuZu43N9pFnXBemoAohPs_eyRJf8taXHJPAXQ@mail.gmail.com/T/#u
[2] #2150

Changes since v7:

• Moved topo_ceiling from patch 10 into patch 8 where the
  side-exhaustion gate first needs it, so V1_MAX saturation
  is handled correctly at every commit in the series.

• Renamed "finite/INFINITY region" to "ordered/unordered region"
  in documentation and in general tried to tighten up the documentation
  around this.

• Added code comment explaining why termination conditions must
  be checked before decrementing counters in paint_queue_get().

• Minor wording and formatting fixes in commit messages, test
  comments, and the t6099 ASCII graph.

Changes since v6:

• Now based on master; all prerequisite topics have graduated.

• Added a topological ceiling concept for v1 commit-graph
  support. When the commit-graph uses v1 topological levels
  (no GDAT chunk), generation numbers saturate at V1_MAX,
  breaking ordering guarantees in the same way as INFINITY.
  Patch 10 introduces a topo_ceiling (V1_MAX for v1, INFINITY
  for v2) that the side-exhaustion and single-result gates
  compare against, so saturated commits are treated as
  unordered.

• Used $LF variable instead of a literal newline in the
  test_trace2_data_singular helper (patch 2), matching the
  existing pattern in test-lib. (Suggested by Rene Scharfe.)

• Improved the min_generation / generation cutoff documentation
  to explain why callers can safely terminate early, rather than
  just stating the threshold rule.

Changes since v5:

• Rebased on next, which now contains kk/commit-reach-find-all-fix.
  The gen_ordered guard from that topic is carried through patches
  7-9 via state.gen_ordered, then removed in patch 10 along with
  the date-ordering fallback.

• Minor documentation and test comment improvements.

Changes since v4:

• New patch 2/10: added test_trace2_data_singular helper to
  test-lib-functions.sh. Shows expected/actual values on
  assertion failure instead of a silent grep failure. Makes
  iterating on step counts much easier.

• New patch 6/10: added clock-skew topologies (se-, se2-)
  that expose side-exhaustion bugs when the commit-date ordering
  fallback fires with a v1 commit graph. All topologies use a
  shared skew_commit helper. Includes step count assertions for
  edge-case tests from patch 3.

• Folded the nonstale_queue dedup wrapper removal (previously
  separate patch 6/8) into the paint_state introduction in
  patch 7/10.

• New patch 10/10: remove the commit-date ordering fallback in
  paint_down_to_common(). The fallback (091cf18e) was a
  performance optimization for v1 commit graphs, but it breaks
  the generation ordering invariant that both the side-exhaustion
  and single-result optimizations depend on. With
  side-exhaustion in place, the fallback is no longer needed.
  If kept, this supersedes the separate
  "commit-reach: fix !FIND_ALL early exit with v1 commit graph"
  topic.

Changes since v3:

• Fixed BUG assertion that was accidentally made unconditional
  in v3: restored the min_generation guard so it only fires
  when generation-based ordering is active.

• Moved generation cutoff and single-result termination
  conditions into the documentation in patch 1, since they
  describe existing behavior.

• Renamed paint_state counter fields for clarity: p1_count ->
  parent1_count, p2_count -> parent2_count, pending_merge_bases
  -> mb_candidate_count. Changed counter types from int to
  size_t. (Suggested by Rene Scharfe.)

Changes since v2:

• New patch 9/10 (was 8/8): moved the min_generation termination
  check and the last_gen monotonicity assertion into
  paint_queue_get(), consolidating halt conditions.
  commit_graph_generation() is now called once per dequeued
  commit and shared across all checks.

• Moved all halt conditions inside paint_queue_get() with the
  "pop first" form: pop, check, then decrement counters. This
  keeps the optimization commit's diff minimal (just inserting
  the new checks between pop and decrement).

• Shortened the doc comment on paint_queue_get() to describe
  what it does rather than how. Inline comments on each
  return NULL explain the specific halt condition.

• Replaced the manual commit-graph setup in the step-count test
  with run_all_modes, which now sets GIT_TRACE2_EVENT per mode
  and produces trace-mode-{none,full,half,no-gdat}.txt files.

• Added a test_paint_down_steps helper for concise 4-mode step
  assertions with diagnostic output on mismatch (prints
  "expected X, got Y" instead of a silent grep failure).

• Added step-count assertions to the single-walk edge-case
  tests: in_merge_bases_many:self, pending-stale,
  infinity-both-sides, mixed-finite-infinity.

• Included step counts alongside wall-clock times in the
  benchmark tables.

Changes since v1:

• Reordered patches: documentation first (describing the existing
  algorithm), tests before code changes, so they demonstrate
  passing with old logic first.

• Dropped the ahead_behind decoupling patch. paint_state is now
  a NEW struct alongside nonstale_queue instead of replacing it.
  ahead_behind() is completely untouched.

• Removed nonstale_queue_put_dedup() and
  nonstale_queue_get_dedup() (dead code after the conversion) in
  a separate commit.

• Renamed: struct paint_queue -> paint_state, field pq -> queue,
  paint_count_add/remove -> paint_count_update (single function
  with signed delta parameter).

• Split the old paint_count_transition (which handled both old
  and new flags in one call) into separate remove/add calls with
  a signed delta. This eliminates the need for the case 0
  handler (which tracked "not in the queue") and allows an
  exhaustive switch on (PARENT1 | PARENT2 | STALE) that
  documents all valid flag combinations, with BUG() in default.

• Added trace2_data_intmax() instrumentation to report the number
  of commits visited per paint walk (separate commit), with
  step-count assertions in tests for deterministic regression
  detection.

cc: Derrick Stolee stolee@gmail.com
cc: Elijah Newren newren@gmail.com
cc: Kristofer Karlsson krka@spotify.com
cc: René Scharfe l.s.r@web.de
cc: SZEDER Gábor szeder.dev@gmail.com

[spkrka]

/preview

[gitgitgadget]

Preview email sent as pull.2149.git.1781946989.gitgitgadget@gmail.com

[spkrka]

/submit

[gitgitgadget]

Submitted as pull.2149.git.1781951820.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2149/spkrka/side-exhaust-pr-v1

To fetch this version to local tag pr-2149/spkrka/side-exhaust-pr-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2149/spkrka/side-exhaust-pr-v1

[gitgitgadget]

This patch series was integrated into seen via git@418052d.

[spkrka]

/submit

[gitgitgadget]

Submitted as pull.2149.v2.git.1782303254.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2149/spkrka/side-exhaust-pr-v2

To fetch this version to local tag pr-2149/spkrka/side-exhaust-pr-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2149/spkrka/side-exhaust-pr-v2

[gitgitgadget]

This branch is now known as kk/merge-base-exhaustion.

[gitgitgadget]

There was a status update in the "Cooking" section about the branch kk/merge-base-exhaustion on the Git mailing list:

The merge-base computation has been optimized by stopping the walk
early when one side's exclusive commits in the queue are exhausted,
yielding significant speedups for queries with one-sided histories.

Expecting a reroll.
cf. <CAL71e4ONrNcOh82upp8YT0b_0nZhxtEv_zFDb_0Sknfc9_kJWA@mail.gmail.com>
cf. <CAL71e4OTYDrbY-kjeJSa-iWtUN=96Q+2_vEB-_tScu_afPS6EA@mail.gmail.com>
source: <pull.2149.v7.git.1786013982.gitgitgadget@gmail.com>

[gitgitgadget]

There was a status update in the "Cooking" section about the branch kk/merge-base-exhaustion on the Git mailing list:

The merge-base computation has been optimized by stopping the walk
early when one side's exclusive commits in the queue are exhausted,
yielding significant speedups for queries with one-sided histories.

Expecting a reroll.
cf. <CAL71e4P1hRcDk0TiR_Gjapf=EScGNbeHqSQ6BqeiAkEvczNu4g@mail.gmail.com>
source: <pull.2149.v7.git.1786013982.gitgitgadget@gmail.com>

[spkrka]

/submit

[gitgitgadget]

Submitted as pull.2149.v8.git.1786440533.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2149/spkrka/side-exhaust-pr-v8

To fetch this version to local tag pr-2149/spkrka/side-exhaust-pr-v8:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2149/spkrka/side-exhaust-pr-v8

[gitgitgadget]

This patch series is no longer integrated into seen.

[gitgitgadget]

This patch series was integrated into seen via git@0178b62.

[gitgitgadget]

Elijah Newren wrote on the Git mailing list (how to reply to this email):

On Tue, Aug 11, 2026 at 2:42 AM Kristofer Karlsson via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> Changes since v7:
>
>  * Moved topo_ceiling from patch 10 into patch 8 where the side-exhaustion
>    gate first needs it, so V1_MAX saturation is handled correctly at every
>    commit in the series.
>
>  * Renamed "finite/INFINITY region" to "ordered/unordered region" in
>    documentation and in general tried to tighten up the documentation around
>    this.
>
>  * Added code comment explaining why termination conditions must be checked
>    before decrementing counters in paint_queue_get().
>
>  * Minor wording and formatting fixes in commit messages, test comments, and
>    the t6099 ASCII graph.

I am quite pleased with how this series has turned out.  Not only does
it provide nice speedups, I think the structure of the series is
particularly nicely set up in a way that helps guide the discovery of
the idea behind the optimization for others to read, documents and
tests everything logically and thoroughly, and was a pleasant read.

I brought up several issues in previous versions, and this round
addresses them all.

Reviewed-by: Elijah Newren <newren@gmail.com>


But, I just have to comment on a few things individually...

>      @@ Documentation/technical/paint-down-to-common.adoc (new)
>       +
>       +When the commit-graph has generation numbers v1 and no
>       +generation floor is specified, topological ordering
>      -+(via generation numbers) is disabled.  Topological levels are
>      ++(via generation numbers) is disabled. Topological levels are
>       +correct but unbalanced -- ordering by such generation numbers
>       +can sometimes cause the walk to detour too far before finding
>      -+merge bases.  Commit-date ordering typically reaches them in
>      ++merge bases. Commit-date ordering typically reaches them in
>       +fewer steps -- see this change for more details:
>       +
>       +   091f4cf3 (commit: don't use generation numbers if not needed,
>      @@ Documentation/technical/paint-down-to-common.adoc (new)
>       +of both worlds and do not need this fallback.
>       +
>       +For v1, `paint_down_to_common()` falls back to pure commit-date
>      -+ordering via `compare_commits_by_commit_date`.  Because commit
>      ++ordering via `compare_commits_by_commit_date`. Because commit
>       +dates are not monotonic (clock skew, rebases, etc.), the queue
>       +may visit commits out of topological order.

Don't think for a second that I didn't notice you murdering these
double spaces.  You villain!  ;-)

>   3:  f857577e0c !  3:  6208bcf3b3 t6600: add test cases for side-exhaustion edge cases
>      @@ t/t6600-test-reach.sh: test_expect_success 'setup' '
>       + #   ps-T1   ps-T2
>       + #
>       + # where ps-T1=merge(ps-Z,ps-B), ps-T2=merge(ps-W,ps-B), so
>      -+ # merge-base(ps-T1,ps-T2) = ps-B. During the walk, ps-X transitions
>      ++ # merge-base(ps-T1,ps-T2) = ps-B.  During the walk, ps-X transitions
>       + # to (PARENT1|PARENT2) via ps-Z and ps-W before ps-B is dequeued;
>       + # then the STALE-walk from ps-B transitions ps-X to
>       + # (PARENT1|PARENT2|STALE).
>      @@ t/t6600-test-reach.sh: test_expect_success 'setup' '
>       +
>       + # Build a side topology that lives entirely outside the half
>       + # commit-graph and has non-monotonic commit dates, to exercise the
>      -+ # INFINITY-gate in paint_down_to_common. With both tips outside
>      ++ # INFINITY-gate in paint_down_to_common.  With both tips outside
>       + # the graph, generation is INFINITY and the queue falls back to
>       + # commit-date order, which here is non-monotonic.
>       + #
>      @@ t/t6600-test-reach.sh: test_expect_success 'get_merge_bases_many' '
>       +
>       +test_expect_success 'get_merge_bases_many:pending-stale' '
>       + # Exercises the (PARENT1|PARENT2) -> (...|STALE) transition path in
>      -+ # paint_down_to_common(). See the topology comment in the setup test.
>      ++ # paint_down_to_common().  See the topology comment in the setup test.
>       + cat >input <<-\EOF &&
>       + A:ps-T1
>       + X:ps-T2
>      @@ t/t6600-test-reach.sh: test_expect_success 'get_merge_bases_many' '
>       +'
>       +
>       +test_expect_success 'get_merge_bases_many:infinity-both-sides' '
>      -+ # Exercises the push-time INFINITY-gate in paint_down_to_common(). See
>      ++ # Exercises the push-time INFINITY-gate in paint_down_to_common().  See
>       + # the pi-* topology comment in the setup test.
>       + cat >input <<-\EOF &&
>       + A:pi-X

...and now you're just toying with us.  :-)

>   8:  391fa07783 !  8:  4a6603731c commit-reach: terminate merge-base walk when one paint side is exhausted
[...]
>  10:  b655b24dc0 ! 10:  677e25077c commit-reach: remove commit-date ordering fallback
[...]

Okay, my last two range-diff comments were just jokes, but more
serious now: I think you did a nice job on the re-splitting.  Thanks
for doing that!

[gitgitgadget]

Kristofer Karlsson wrote on the Git mailing list (how to reply to this email):

On Wed, 12 Aug 2026 at 08:23, Elijah Newren <newren@gmail.com> wrote:
>
> I am quite pleased with how this series has turned out.  Not only does
> it provide nice speedups, I think the structure of the series is
> particularly nicely set up in a way that helps guide the discovery of
> the idea behind the optimization for others to read, documents and
> tests everything logically and thoroughly, and was a pleasant read.
>
> I brought up several issues in previous versions, and this round
> addresses them all.

That's very much appreciated, I know this has been a lot of content
to review and there has been some unfortunate churn, especially
around the textual sections.

> >       +For v1, `paint_down_to_common()` falls back to pure commit-date
> >      -+ordering via `compare_commits_by_commit_date`.  Because commit
> >      ++ordering via `compare_commits_by_commit_date`. Because commit
> >       +dates are not monotonic (clock skew, rebases, etc.), the queue
> >       +may visit commits out of topological order.
>
> Don't think for a second that I didn't notice you murdering these
> double spaces.  You villain!  ;-)

Yes, well spotted -- this was unfortunate but needed I think.
I realized that this document was inconsistent, so I decided
to standardize on single spaces because that was the smaller
change and it also aligned with other technical documents
(such as commit-graph.adoc) ...

> >   3:  f857577e0c !  3:  6208bcf3b3 t6600: add test cases for side-exhaustion edge cases
>
> ...and now you're just toying with us.  :-)

... and here I just restored your tests to their original glory,
despite making the patch series as a whole feel a bit inconsistent.

Overall though, I wonder if it would be useful to codify a
preference for double or single spacing after periods, even if
that (obviously) does not imply we should reformat the entire
code-base directly.

> Okay, my last two range-diff comments were just jokes, but more
> serious now: I think you did a nice job on the re-splitting.  Thanks
> for doing that!

Thank you, writing documentation feels harder (for me)
than coding, but I'm somewhat happy with the end state now :)

- Kristofer

[gitgitgadget]

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Kristofer Karlsson <krka@spotify.com> writes:

> ... and here I just restored your tests to their original glory,
> despite making the patch series as a whole feel a bit inconsistent.
>
> Overall though, I wonder if it would be useful to codify a
> preference for double or single spacing after periods, even if
> that (obviously) does not imply we should reformat the entire
> code-base directly.

This is a thing that is best left for "match the surrounding area"
rule.  As you mentioned earlier, it is not something we want to go
back and churn every line to make it consistent.

>> Okay, my last two range-diff comments were just jokes, but more
>> serious now: I think you did a nice job on the re-splitting.  Thanks
>> for doing that!
>
> Thank you, writing documentation feels harder (for me)
> than coding, but I'm somewhat happy with the end state now :)

Good.  We can declare victory and mark the topic for 'next' now?

Thanks, both of you.

[gitgitgadget]

Kristofer Karlsson wrote on the Git mailing list (how to reply to this email):

On Wed, 12 Aug 2026 at 20:24, Junio C Hamano <gitster@pobox.com> wrote:
>
> Good.  We can declare victory and mark the topic for 'next' now?
>
> Thanks, both of you.

Yes, I would be very happy to have it advance to next,
though I am of course biased.

Thanks,
Kristofer

[gitgitgadget]

Elijah Newren wrote on the Git mailing list (how to reply to this email):

On Wed, Aug 12, 2026 at 11:24 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Good.  We can declare victory and mark the topic for 'next' now?

Yes, I believe it's ready now.

[gitgitgadget]

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Elijah Newren <newren@gmail.com> writes:

> On Wed, Aug 12, 2026 at 11:24 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Good.  We can declare victory and mark the topic for 'next' now?
>
> Yes, I believe it's ready now.

Thanks.

[gitgitgadget]

There was a status update in the "Cooking" section about the branch kk/merge-base-exhaustion on the Git mailing list:

The merge-base computation has been optimized by stopping the walk
early when one side's exclusive commits in the queue are exhausted,
yielding significant speedups for queries with one-sided histories.

Will merge to 'next'?
cf. <CABPp-BHuh_8q6Hy2-Bk7H6Chdb4+eeW1f4LZU0szZ4zU9Eeo+w@mail.gmail.com>
cf. <xmqqbjb7w1dq.fsf@gitster.g>
source: <pull.2149.v8.git.1786440533.gitgitgadget@gmail.com>