Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history

[Patrick Steinhardt <ps@pks.im>]

On Tue, Nov 10, 2026 at 06:40:00PM -0600, Justin Tobler wrote:
> On 26/11/09 09:03AM, Miklos Vajna wrote:
> > Hi Patrick,
> > 
> > Thanks for the reviews of v4. Changes in v5:
> > 
> > - The fix of the old 6/6 is now squashed into the speedup patch (5/5),
> >  so no commit in the series loses history, and its test is moved to
> >  1/5. While at it, I simplified 5/5: instead of two passes of diffs
> >  against the parents, it first finds a parent where the path is
> >  unchanged, and then only diffs against the other parents if they
> >  still need renames to be followed. Because of the squash, the
> >  range-diff pairs the new 5/5 with the old 6/6.
> > 
> > - 5/5 now also follows renames when a parent has no merge base with
> >  the parent where the path is unchanged, e.g. a subtree merge of
> >  unrelated histories. Added a test for this to 1/5.
> > 
> > - 5/5 now also follows copies, not only renames, when the merge dropped
> >  the edit of the copy source. Added your test for this to 1/5. With
> >  this, the linux.git case takes about 42s, instead of 37s.
> 
> I'm a bit late to the party, but I decided to run some benchmarks
> against linux.git to showcase performance with 304812ed33de (log:
> improve --follow following renames for non-linear history, 2026-06-22)
> reverted, at the regression, and with v5 of this series applied:
> 
>   Benchmark 1: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted)
>     Time (mean ± σ):     23.493 s ±  0.645 s    [User: 23.001 s, System: 0.389 s]
>     Range (min … max):   22.748 s … 23.883 s    3 runs
> 
>   Benchmark 2: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
>     Time (mean ± σ):     335.883 s ±  4.866 s    [User: 333.773 s, System: 0.577 s]
>     Range (min … max):   332.296 s … 341.422 s    3 runs
> 
>   Benchmark 3: git log --stat --follow -- include/linux/rsc_table.h (rev = v5)
>     Time (mean ± σ):     38.688 s ±  0.379 s    [User: 38.081 s, System: 0.415 s]
>     Range (min … max):   38.250 s … 38.923 s    3 runs
> 
>   Summary
>     git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted) ran
>      14.30 ± 0.44 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
>       1.65 ± 0.05 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = v5)
> 
> While the fix in this series does improve the state of things, it is
> still overall a significant performance regression that I expect will
> result in increased file history timeouts on the server-side for some
> repositories we host at GitLab. Rename following for a file's history
> even prior to this regression can be expensive for large repositories.
> 
> While the original patch and this series do improve correctness of
> following a file through history, I wonder if the increased cost is
> ultimately worth it in most scenarios. Maybe this could be something
> opt-in with another flag instead of toggled on my default?

Yeah, by now I'm also leaning into this direction. The performance cost
is definitely way too high for us, so we need a way to disable this new
feature altogether at GitLab.

> With this in mind, maybe it would be worth it to revert 304812ed33de
> (log: improve --follow following renames for non-linear history,
> 2026-06-22) and restart with a fresh series. IMO, it might make the
> changes here a bit easier to validate also.

I don't have a strong sense on whether or not we should revert the whole
series. I find the patches quite hard to review, but that may not be
because we're fixing things incrementally and rather be because I myself
am not familiar with this subsystem.

But in case we decide to not revert I'd like to see a toggle to disable
this feature as a first step. As mentioned, this change has caused a
rather big regression for GitLab, so we'd ideally have a way to disable
this feature soon so that we can mitigate it.

I don't want to make you (Miklos) responsible for delivering that of
course, I just want us to align on a fast path forward. In case we don't
revert we can help out and contribute such a default-disabled toggle.
And if so, that would also give us more time to fix the issue properly.

Thanks!

Patrick