commit-reach: use object flags for tips_reachable_from_bases() by spkrka · Pull Request #2116 · gitgitgadget/git

spkrka · 2026-05-15T17:58:02Z

This replaces the O(C*T) linear scan in tips_reachable_from_bases()
with an O(1) flag check using the RESULT object flag.

The function is called by git for-each-ref --merged and
git branch/tag --contains/--no-contains via reach_filter() in
ref-filter.c.

Benchmarks on a merge-heavy monorepo (2.3M commits, 10,000 refs):

for-each-ref --merged HEAD: 6.6s → 1.6s (4.1x)
for-each-ref --no-merged HEAD: 6.7s → 1.7s (4.0x)

On linux.git with 10,000 synthetic branches at the root commit:

for-each-ref --merged HEAD: 1.35s → 0.35s (3.9x)
for-each-ref --no-merged HEAD: 1.82s → 0.31s (5.9x)

v2 of this patch, addressing Jeff King's feedback:

Replaced the decoration hash with the RESULT object flag (simpler,
no extra data structure, handles duplicate tips naturally)
Fixed early-termination bug when multiple refs point to the same
commit (the decoration API overwrites on duplicate keys)
Removed the now-unused index field from struct commit_and_index
Diff is +11/-12 lines

cc: Jeff King peff@peff.net
cc: Kristofer Karlsson krka@spotify.com
cc: Derrick Stolee stolee@gmail.com

spkrka · 2026-05-15T18:06:38Z

/submit

gitgitgadget · 2026-05-15T18:07:59Z

Submitted as pull.2116.git.1778868463992.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2116/spkrka/tips-reachable-minimal-v1

To fetch this version to local tag pr-2116/spkrka/tips-reachable-minimal-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2116/spkrka/tips-reachable-minimal-v1

gitgitgadget · 2026-05-15T21:19:00Z

Jeff King wrote on the Git mailing list (how to reply to this email):

On Fri, May 15, 2026 at 06:07:43PM +0000, Kristofer Karlsson via GitGitGadget wrote:

> From: Kristofer Karlsson <krka@spotify.com>
> 
> tips_reachable_from_bases() walks the commit graph from a set of base
> commits to find which tip commits are reachable.  The inner loop does
> a linear scan over the tips array to check whether each visited commit
> is a tip, making the overall cost O(C * T) where C is commits walked
> and T is the number of tips.
> 
> Replace the linear scan with the decoration hash for lookups, reducing
> the per-commit tip check from O(T) to O(1) and the overall cost from
> O(C * T) to O(C + T).
> 
> This function is called by `git for-each-ref --merged` and
> `git branch/tag --contains/--no-contains` via reach_filter() in
> ref-filter.c.
> 
> Benchmark on a merge-heavy monorepo (2.3M commits, 10,000 refs):
> 
>   Command                           Before    After   Speedup
>   for-each-ref --merged HEAD        6.64s     1.66s     4.0x
>   for-each-ref --no-merged HEAD     6.75s     1.74s     3.9x
>   branch --merged HEAD              0.68s     0.61s      10%
>   branch --no-merged HEAD           0.65s     0.61s       8%
>   tag --merged HEAD                 0.12s     0.12s       -
> 
> The large speedup for for-each-ref is because it checks all 10,000
> refs as tips, making the O(T) inner loop expensive.  The branch
> subcommand only checks local branches (fewer tips), so the improvement
> is smaller.

Hmm, I couldn't reproduce the speedup on something like linux.git (~1.4M
commits) with a lot of synthetic branches. I'd think that old branches
would be the most expensive, so I did:

  old=$(git rev-list --reverse HEAD | head -n1)
  seq --format="update refs/heads/branch%g $old" 10000 |
  git update-ref --stdin

Running "git for-each-ref --no-merged HEAD" takes ~650ms with stock Git.
But with your patch, it goes to ~830ms!

So what am I missing about your repo that it is so slow in the first
place?

>      * Hacking the array index into the decoration value as (void *)(i + 1)
>        instead of storing a proper pointer

The decoration API is not the most generic option here. There's an
oidmap type, but you have to embed the hashmap bits into your struct,
which is a lot of boilerplate if you're just storing an int. You can
define a khash with a custom value type, and I think the existing
oid_pos uses an int, which might be enough. All of those will store an
extra copy of the oid, though for the sizes we're talking about that's
not the end of the world.

Since we're always mapping commits, you could define a commit-slab (each
commit struct gets a unique id which we then index into a big array).
See commit-slab.h for an example.

I'm not very familiar with this code, but I wonder if we actually need
to map at all. It looks like we are mostly interested in set inclusion,
so perhaps an oidset() would work. Or even a bit in the object-flags.

-Peff

gitgitgadget · 2026-05-15T21:19:02Z

User Jeff King <peff@peff.net> has been added to the cc: list.

gitgitgadget · 2026-05-16T08:25:35Z

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

Thanks for testing this, Jeff! You're right, the patch as posted
regresses on your synthetic test case.

The issue is that when multiple refs point to the same commit,
add_decoration overwrites earlier entries,
so only one index gets stored. The marking itself is correct (the flag
is on the shared commit object,
so all duplicates get marked), but the j == min_generation_index check
never fires for the minimum tip,
so early termination breaks. The DFS walks the entire graph instead of
stopping when all tips are found.

I have a fix for the early-termination bug (checking the flag at
min_generation_index instead of comparing indices),
but your suggestions about the API are well taken, I don't think the
decoration hash is the right tool here.
Since we only need set membership ("is this commit a tip?"), not a
mapping, an object-flags bit or commit-slab would
indeed be simpler and avoid the (void *)(i + 1) hack entirely.

I fixed it locally now for the linux test case and got a 4x speedup
there too - the problem was failing the early termination.
Some numbers when running against the linux repo on my machine:

Command          │ Baseline │     V1 (broken)     │     V2 (fixed)      │
--no-merged HEAD │ 1.33s    │ 2.01s (1.5x slower) │ 0.31s (4.3x faster) │
--merged HEAD    │ 1.35s    │ 1.96s (1.5x slower) │ 0.31s (4.3x faster) │

However, I'll still need to rethink the decoration map - I will come
back with a better patch shortly.

- Kristofer

On Fri, 15 May 2026 at 23:15, Jeff King <peff@peff.net> wrote:
>
> On Fri, May 15, 2026 at 06:07:43PM +0000, Kristofer Karlsson via GitGitGadget wrote:
>
> > From: Kristofer Karlsson <krka@spotify.com>
> >
> > tips_reachable_from_bases() walks the commit graph from a set of base
> > commits to find which tip commits are reachable.  The inner loop does
> > a linear scan over the tips array to check whether each visited commit
> > is a tip, making the overall cost O(C * T) where C is commits walked
> > and T is the number of tips.
> >
> > Replace the linear scan with the decoration hash for lookups, reducing
> > the per-commit tip check from O(T) to O(1) and the overall cost from
> > O(C * T) to O(C + T).
> >
> > This function is called by `git for-each-ref --merged` and
> > `git branch/tag --contains/--no-contains` via reach_filter() in
> > ref-filter.c.
> >
> > Benchmark on a merge-heavy monorepo (2.3M commits, 10,000 refs):
> >
> >   Command                           Before    After   Speedup
> >   for-each-ref --merged HEAD        6.64s     1.66s     4.0x
> >   for-each-ref --no-merged HEAD     6.75s     1.74s     3.9x
> >   branch --merged HEAD              0.68s     0.61s      10%
> >   branch --no-merged HEAD           0.65s     0.61s       8%
> >   tag --merged HEAD                 0.12s     0.12s       -
> >
> > The large speedup for for-each-ref is because it checks all 10,000
> > refs as tips, making the O(T) inner loop expensive.  The branch
> > subcommand only checks local branches (fewer tips), so the improvement
> > is smaller.
>
> Hmm, I couldn't reproduce the speedup on something like linux.git (~1.4M
> commits) with a lot of synthetic branches. I'd think that old branches
> would be the most expensive, so I did:
>
>   old=$(git rev-list --reverse HEAD | head -n1)
>   seq --format="update refs/heads/branch%g $old" 10000 |
>   git update-ref --stdin
>
> Running "git for-each-ref --no-merged HEAD" takes ~650ms with stock Git.
> But with your patch, it goes to ~830ms!
>
> So what am I missing about your repo that it is so slow in the first
> place?
>
> >      * Hacking the array index into the decoration value as (void *)(i + 1)
> >        instead of storing a proper pointer
>
> The decoration API is not the most generic option here. There's an
> oidmap type, but you have to embed the hashmap bits into your struct,
> which is a lot of boilerplate if you're just storing an int. You can
> define a khash with a custom value type, and I think the existing
> oid_pos uses an int, which might be enough. All of those will store an
> extra copy of the oid, though for the sizes we're talking about that's
> not the end of the world.
>
> Since we're always mapping commits, you could define a commit-slab (each
> commit struct gets a unique id which we then index into a big array).
> See commit-slab.h for an example.
>
> I'm not very familiar with this code, but I wonder if we actually need
> to map at all. It looks like we are mostly interested in set inclusion,
> so perhaps an oidset() would work. Or even a bit in the object-flags.
>
> -Peff

gitgitgadget · 2026-05-16T08:25:37Z

User Kristofer Karlsson <krka@spotify.com> has been added to the cc: list.

tips_reachable_from_bases() walks the commit graph from a set of base commits to find which tip commits are reachable. The inner loop does a linear scan over the tips array to check whether each visited commit is a tip, making the overall cost O(C * T) where C is commits walked and T is the number of tips. Use the RESULT object flag to mark tip commits, replacing the linear scan with a single flag test per visited commit. This reduces the per-commit tip check from O(T) to O(1) and the overall cost from O(C * T) to O(C + T). When multiple refs point to the same commit, the shared object gets the flag once, so all duplicates are handled automatically. The early-termination advancement loop checks the flag on the sorted commits array directly, which naturally handles duplicates since the flag is on the shared commit object. This also removes the index field from struct commit_and_index, since the indirection through the original tips array is no longer needed. This function is called by `git for-each-ref --merged` and `git branch/tag --contains/--no-contains` via reach_filter() in ref-filter.c. Benchmark on a merge-heavy monorepo (2.3M commits, 10,000 refs): Command Before After Speedup for-each-ref --merged HEAD 6.57s 1.59s 4.1x for-each-ref --no-merged HEAD 6.67s 1.66s 4.0x branch --merged HEAD 0.68s 0.61s 10% branch --no-merged HEAD 0.65s 0.61s 8% tag --merged HEAD 0.12s 0.12s - On linux.git with 10,000 synthetic branches at the root commit (worst case for the DFS walk): Command Before After Speedup for-each-ref --merged HEAD 1.35s 0.35s 3.9x for-each-ref --no-merged HEAD 1.82s 0.31s 5.9x The large speedup for for-each-ref is because it checks all 10,000 refs as tips, making the O(T) inner loop expensive. The branch subcommand only checks local branches (fewer tips), so the improvement is smaller. Signed-off-by: Kristofer Karlsson <krka@spotify.com>

spkrka · 2026-05-16T09:15:31Z

/submit

gitgitgadget · 2026-05-16T09:16:42Z

Submitted as pull.2116.v2.git.1778922993480.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2116/spkrka/tips-reachable-minimal-v2

To fetch this version to local tag pr-2116/spkrka/tips-reachable-minimal-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2116/spkrka/tips-reachable-minimal-v2

gitgitgadget · 2026-05-16T13:52:44Z

Derrick Stolee wrote on the Git mailing list (how to reply to this email):

On 5/16/26 4:23 AM, Kristofer Karlsson wrote:
> Thanks for testing this, Jeff! You're right, the patch as posted
> regresses on your synthetic test case.
> > The issue is that when multiple refs point to the same commit,
> add_decoration overwrites earlier entries,
> so only one index gets stored. The marking itself is correct (the flag
> is on the shared commit object,
> so all duplicates get marked), but the j == min_generation_index check
> never fires for the minimum tip,
> so early termination breaks. The DFS walks the entire graph instead of
> stopping when all tips are found.
> > I have a fix for the early-termination bug (checking the flag at
> min_generation_index instead of comparing indices),
> but your suggestions about the API are well taken, I don't think the
> decoration hash is the right tool here.
> Since we only need set membership ("is this commit a tip?"), not a
> mapping, an object-flags bit or commit-slab would
> indeed be simpler and avoid the (void *)(i + 1) hack entirely.
> > I fixed it locally now for the linux test case and got a 4x speedup
> there too - the problem was failing the early termination.
> Some numbers when running against the linux repo on my machine:
> > Command          │ Baseline │     V1 (broken)     │     V2 (fixed)      │
> --no-merged HEAD │ 1.33s    │ 2.01s (1.5x slower) │ 0.31s (4.3x faster) │
> --merged HEAD    │ 1.35s    │ 1.96s (1.5x slower) │ 0.31s (4.3x faster) │
> > However, I'll still need to rethink the decoration map - I will come
> back with a better patch shortly.

This is indeed an interesting case (multiple decorations) that we should
make sure is covered by a test case so we don't fall into this mistake
again.

Thanks,
-Stolee

gitgitgadget · 2026-05-16T13:52:45Z

User Derrick Stolee <stolee@gmail.com> has been added to the cc: list.

When multiple refs point to the same commit, the reachability check must handle them correctly. Add three tests: - duplicate tips, all reachable - duplicate tips, none reachable - duplicate tips at the minimum generation (exercises the early-termination advancement logic) Suggested-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Kristofer Karlsson <krka@spotify.com>

spkrka · 2026-05-16T15:58:37Z

/submit

gitgitgadget · 2026-05-16T16:00:02Z

Submitted as pull.2116.v3.git.1778947182.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2116/spkrka/tips-reachable-minimal-v3

To fetch this version to local tag pr-2116/spkrka/tips-reachable-minimal-v3:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2116/spkrka/tips-reachable-minimal-v3

spkrka marked this pull request as ready for review May 15, 2026 18:06

spkrka force-pushed the tips-reachable-minimal branch from 992c0af to 7399a12 Compare May 16, 2026 08:45

spkrka changed the title ~~commit-reach: use the decoration hash for tips_reachable_from_bases()~~ commit-reach: use object flags for tips_reachable_from_bases() May 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

commit-reach: use object flags for tips_reachable_from_bases()#2116

commit-reach: use object flags for tips_reachable_from_bases()#2116
spkrka wants to merge 2 commits into
gitgitgadget:masterfrom
spkrka:tips-reachable-minimal

spkrka commented May 15, 2026 •

edited by gitgitgadget Bot

Loading

Uh oh!

spkrka commented May 15, 2026

Uh oh!

gitgitgadget Bot commented May 15, 2026

Uh oh!

gitgitgadget Bot commented May 15, 2026

Uh oh!

gitgitgadget Bot commented May 15, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

spkrka commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

spkrka commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

spkrka commented May 15, 2026 • edited by gitgitgadget Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spkrka commented May 15, 2026

Uh oh!

gitgitgadget Bot commented May 15, 2026

Uh oh!

gitgitgadget Bot commented May 15, 2026

Uh oh!

gitgitgadget Bot commented May 15, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

spkrka commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

spkrka commented May 16, 2026

Uh oh!

gitgitgadget Bot commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

spkrka commented May 15, 2026 •

edited by gitgitgadget Bot

Loading