Subject: Re: [PATCH] ls-refs: reuse buffer when sending refs

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 25 Aug 2021 17:43:39 +0200

On Wed, Aug 25, 2021 at 10:50:30AM -0400, Derrick Stolee wrote:
> On 8/25/2021 9:49 AM, Patrick Steinhardt wrote:
> > Improve this by passing in a buffer via the `ls_refs_data` struct which
> > is then reused on each reference. In a repository with about 2.3M refs,
> > this speeds up local mirror fetches by about 2%:
> > 
> >     Benchmark #1: HEAD~: git-fetch
> >       Time (mean ± σ):     25.415 s ±  0.131 s    [User: 22.722 s, System: 4.740 s]
> >       Range (min … max):   25.240 s … 25.543 s    5 runs
> > 
> >     Benchmark #2: HEAD: git-fetch
> >       Time (mean ± σ):     24.922 s ±  0.110 s    [User: 22.404 s, System: 4.476 s]
> >       Range (min … max):   24.825 s … 25.081 s    5 runs
> > 
> >     Summary
> >       'HEAD: git-fetch' ran
> >         1.02 ± 0.01 times faster than 'HEAD~: git-fetch'
> > 
> > Signed-off-by: Patrick Steinhardt <[email-masked]>
> > ---
> > 
> > Note that while this topic applies on top of "master", I've done the
> > benchmark on top of my other optimizations for fetches. It's cheating a
> > bit, but it's easier to see that the optimization does something when
> > the remaining constant part is lower.
> 
> I don't mind demonstrating an optimization using the other work.
> 
> Perhaps this would be better grouped with those other changes?
> I know that the text is independent and merges cleanly without it,
> but it can be helpful to think about the effort as one unified
> topic instead of juggling multiple, especially because I don't
> see the other one needing many revisions.

I don't know. I just happen to revisit this topic every few days, and
every time I stumble upon some more performance improvements. It would
feel wrong to shift the goalposts of the other series every time I find
something new, so I instead opt for separate patch series.

If this proves to be annoying for reviewers, then feel free to shout at
me and I'll change my approach.

Thanks for your review!

Patrick