Performance regression in "update" hooks

Performance regression in "update" hooks

[Patrick Steinhardt]

Hi,

Bencher has alerted me that there's been two performance regressions in
git-receive-pack(1) [1] and git-fetch(1) [2].

The first one is quite easy to reproduce with the benchmarks at [3] and
bisects to fc148b146a (receive-pack: convert update hooks to new API,
2026-01-28):

  $ cd receive-refs
  $ ./run --revisions /path/to/your/git/repo \
      fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \
      --parameter-list refformat reftable \
      --parameter-list refcount 10000

  Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)
    Time (mean ± σ):     182.0 ms ±   2.7 ms    [User: 91.5 ms, System: 89.3 ms]
    Range (min … max):   175.8 ms … 185.0 ms    15 runs

  Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)
    Time (mean ± σ):     484.6 ms ±  27.6 ms    [User: 176.2 ms, System: 376.1 ms]
    Range (min … max):   406.2 ms … 495.1 ms    10 runs

  Summary
    receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran
      2.66 ± 0.16 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)

I've Cc'd Adrian.

The other performance regression seems to be present in both
git-receive-pack(1) and git-fetch(1) and happens between e6e9f13364
(Sync with 'master', 2026-02-25) and ebd1da8b75 (Merge branch
'cx/fetch-display-ubfix' into next, 2026-02-26). It took me a while to
reproduce as my local Git configuration was hiding the regression, but I
have been able to bisect this to 452b12c2e0 (builtin/maintenance: use
"geometric" strategy by default, 2026-02-24).

The problem here is rather simple though. The benchmark fetches 10,000
refs into the repository, and before the commit we didn't do anything
about them. But after the commit we now have per-data-structure tasks,
and the result is that we thus end up packing refs. That's also why the
regression isn't present in the reftable backend, as it wouldn't need
any optimization.

So I'd consider this to be a bug in the benchmarking infrastructure
itself that I'll fix by disabling auto-maintenance.

Thanks!

Patrick

[1]: https://bencher.dev/perf/git?lower_value=false&upper_value=false&lower_boundary=false&upper_boundary=false&x_axis=date_time&branches=595859eb-071c-48e9-97cf-195e0a3d6ed1&testbeds=02dcb8ad-6873-494c-aabc-9a6237601308&benchmarks=e3553193-aefc-40a4-8816-9c1bdc1838a4%2Ccd00a2a1-0fd1-416a-9812-cfd3e9b4fdb8&measures=63dafffb-98c4-4c27-ba43-7112cae627fc&start_time=1765177145759&end_time=1772434745759&tab=plots&plot=6887f804-2bbc-4219-8211-55b6440fd5c0&plots_search=6887f804-2bbc-4219-8211-55b6440fd5c0&key=true&reports_per_page=4&branches_per_page=8&testbeds_per_page=8&benchmarks_per_page=8&plots_per_page=8&reports_page=1&branches_page=1&testbeds_page=1&benchmarks_page=1&plots_page=1
[2]: https://bencher.dev/perf/git?lower_value=false&upper_value=false&lower_boundary=false&upper_boundary=false&x_axis=date_time&branches=595859eb-071c-48e9-97cf-195e0a3d6ed1&testbeds=02dcb8ad-6873-494c-aabc-9a6237601308&benchmarks=196480c8-64d1-4768-a3e2-ac3c5f75a26e%2Cb422ed57-2b09-474b-a85f-2d71ba7ca46b&measures=63dafffb-98c4-4c27-ba43-7112cae627fc&start_time=1765177141331&end_time=1772434741331&tab=plots&plot=4134acc8-9194-454c-9d71-f41b44ab969d&plots_search=4134acc8-9194-454c-9d71-f41b44ab969d&key=true&reports_per_page=4&branches_per_page=8&testbeds_per_page=8&benchmarks_per_page=8&plots_per_page=8&reports_page=1&branches_page=1&testbeds_page=1&benchmarks_page=1&plots_page=1
[3]: https://gitlab.com/gitlab-org/data-access/git/benchmarks

Re: Performance regression in "update" hooks

[Adrian Ratiu]

On Mon, 02 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> Hi,
>
> Bencher has alerted me that there's been two performance regressions in
> git-receive-pack(1) [1] and git-fetch(1) [2].
>
> The first one is quite easy to reproduce with the benchmarks at [3] and
> bisects to fc148b146a (receive-pack: convert update hooks to new API,
> 2026-01-28):
>
>   $ cd receive-refs
>   $ ./run --revisions /path/to/your/git/repo \
>       fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \
>       --parameter-list refformat reftable \
>       --parameter-list refcount 10000
>
>   Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)
>     Time (mean ± σ):     182.0 ms ±   2.7 ms    [User: 91.5 ms, System: 89.3 ms]
>     Range (min … max):   175.8 ms … 185.0 ms    15 runs
>
>   Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)
>     Time (mean ± σ):     484.6 ms ±  27.6 ms    [User: 176.2 ms, System: 376.1 ms]
>     Range (min … max):   406.2 ms … 495.1 ms    10 runs
>
>   Summary
>     receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran
>       2.66 ± 0.16 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)
>
> I've Cc'd Adrian.

Hi Patrick,

I looked at the commits before and after the many-refs test regression
and it appears the regressions started after Junio landed v2 of the
config series in next [1], which might cause it.

v2 was not ready to land. I sent v3 yesterday addressing all the
feedback, didn't even realize v2 landed. :)

Does the regression go away if you revert [1] ?

I don't have the benchmark setup and it might be easier for you to
confirm?

Many thanks!

1:

commit 6a04cca28e210f0c51cfefcb52475c7ede6e99fb
Merge: d6ebc97cb1 4b12cd3ae3
Author:     Junio C Hamano <gitster@pobox.com>
AuthorDate: Fri Feb 27 15:16:30 2026 -0800
Commit:     Junio C Hamano <gitster@pobox.com>
CommitDate: Fri Feb 27 15:16:30 2026 -0800

    Merge branch 'ar/config-hooks' into next
    
    Allow hook commands to be defined (possibly centrally) in the
    configuration files, and run multiple of them for the same hook
    event.
    
    * ar/config-hooks:
      hook: add -z option to "git hook list"
      hook: allow out-of-repo 'git hook' invocations
      hook: allow event = "" to overwrite previous values
      hook: allow disabling config hooks
      hook: include hooks from the config
      hook: add "git hook list" command
      hook: run a list of hooks to prepare for multihook support
      hook: add internal state alloc/free callbacks

Re: Performance regression in "update" hooks

[Adrian Ratiu]

On Mon, 02 Mar 2026, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
> On Mon, 02 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
>> Hi,
>>
>> Bencher has alerted me that there's been two performance regressions in
>> git-receive-pack(1) [1] and git-fetch(1) [2].
>>
>> The first one is quite easy to reproduce with the benchmarks at [3] and
>> bisects to fc148b146a (receive-pack: convert update hooks to new API,
>> 2026-01-28):
>>
>>   $ cd receive-refs
>>   $ ./run --revisions /path/to/your/git/repo \
>>       fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \
>>       --parameter-list refformat reftable \
>>       --parameter-list refcount 10000
>>
>>   Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)
>>     Time (mean ± σ):     182.0 ms ±   2.7 ms    [User: 91.5 ms, System: 89.3 ms]
>>     Range (min … max):   175.8 ms … 185.0 ms    15 runs
>>
>>   Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)
>>     Time (mean ± σ):     484.6 ms ±  27.6 ms    [User: 176.2 ms, System: 376.1 ms]
>>     Range (min … max):   406.2 ms … 495.1 ms    10 runs
>>
>>   Summary
>>     receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran
>>       2.66 ± 0.16 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)
>>
>> I've Cc'd Adrian.
>
> Hi Patrick,
>
> I looked at the commits before and after the many-refs test regression
> and it appears the regressions started after Junio landed v2 of the
> config series in next [1], which might cause it.
>
> v2 was not ready to land. I sent v3 yesterday addressing all the
> feedback, didn't even realize v2 landed. :)
>
> Does the regression go away if you revert [1] ?
>
> I don't have the benchmark setup and it might be easier for you to
> confirm?
>
> Many thanks!
>
> 1:
>
> commit 6a04cca28e210f0c51cfefcb52475c7ede6e99fb
> Merge: d6ebc97cb1 4b12cd3ae3
> Author:     Junio C Hamano <gitster@pobox.com>
> AuthorDate: Fri Feb 27 15:16:30 2026 -0800
> Commit:     Junio C Hamano <gitster@pobox.com>
> CommitDate: Fri Feb 27 15:16:30 2026 -0800
>
>     Merge branch 'ar/config-hooks' into next
>     
>     Allow hook commands to be defined (possibly centrally) in the
>     configuration files, and run multiple of them for the same hook
>     event.
>     
>     * ar/config-hooks:
>       hook: add -z option to "git hook list"
>       hook: allow out-of-repo 'git hook' invocations
>       hook: allow event = "" to overwrite previous values
>       hook: allow disabling config hooks
>       hook: include hooks from the config
>       hook: add "git hook list" command
>       hook: run a list of hooks to prepare for multihook support
>       hook: add internal state alloc/free callbacks

Actually I think these are two separate issues.

I will reproduce and look into the regression which bisected to
c148b146a (receive-pack: convert update hooks to new API,  2026-01-28).

Re: Performance regression in "update" hooks

[Patrick Steinhardt]

On Mon, Mar 02, 2026 at 04:12:39PM +0200, Adrian Ratiu wrote:
> On Mon, 02 Mar 2026, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
> > On Mon, 02 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> >> Hi,
> >>
> >> Bencher has alerted me that there's been two performance regressions in
> >> git-receive-pack(1) [1] and git-fetch(1) [2].
> >>
> >> The first one is quite easy to reproduce with the benchmarks at [3] and
> >> bisects to fc148b146a (receive-pack: convert update hooks to new API,
> >> 2026-01-28):
> >>
> >>   $ cd receive-refs
> >>   $ ./run --revisions /path/to/your/git/repo \
> >>       fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \
> >>       --parameter-list refformat reftable \
> >>       --parameter-list refcount 10000
> >>
> >>   Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)
> >>     Time (mean ± σ):     182.0 ms ±   2.7 ms    [User: 91.5 ms, System: 89.3 ms]
> >>     Range (min … max):   175.8 ms … 185.0 ms    15 runs
> >>
> >>   Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)
> >>     Time (mean ± σ):     484.6 ms ±  27.6 ms    [User: 176.2 ms, System: 376.1 ms]
> >>     Range (min … max):   406.2 ms … 495.1 ms    10 runs
> >>
> >>   Summary
> >>     receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran
> >>       2.66 ± 0.16 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)
> >>
> >> I've Cc'd Adrian.
> >
> > Hi Patrick,
> >
> > I looked at the commits before and after the many-refs test regression
> > and it appears the regressions started after Junio landed v2 of the
> > config series in next [1], which might cause it.
> >
> > v2 was not ready to land. I sent v3 yesterday addressing all the
> > feedback, didn't even realize v2 landed. :)
> >
> > Does the regression go away if you revert [1] ?
> >
> > I don't have the benchmark setup and it might be easier for you to
> > confirm?

All you need is a normal development infra and hyperfine. The
benchmarking scripts in the repo I linked should then "just work" with
the above invocation.

[snip]
> Actually I think these are two separate issues.
> 
> I will reproduce and look into the regression which bisected to
> c148b146a (receive-pack: convert update hooks to new API,  2026-01-28).

Perfect, thanks!

Patrick

Re: Performance regression in "update" hooks

[Jeff King]

On Mon, Mar 02, 2026 at 03:27:22PM +0100, Patrick Steinhardt wrote:

> > > I don't have the benchmark setup and it might be easier for you to
> > > confirm?
> 
> All you need is a normal development infra and hyperfine. The
> benchmarking scripts in the repo I linked should then "just work" with
> the above invocation.

Thanks, these were very cool and easy to use.

Looking at the patch, my guess was that the problem is that we are now
setting up and tearing down the sideband muxer for each hook invocation.
This is expensive for the "update" hook, since it fires once per ref.

After running the benchmark I tried tweaking the "stdin" file to replace
"side-band-64k" with "not-side-band" (which conveniently is the same
length and thus you don't need to update the pkt-line header). And it
does make the slowdown go away. (Sadly that input is generated on the
fly by the benchmark, so you have to time with your own invocation).

I think it wouldn't be _quite_ so bad if we actually had an update hook,
because then we'd be paying the cost to exec the hook for each ref. So
the extra work to setup the sideband would be less noticeable.

But it looks like the sideband setup happens even if we aren't going to
run anything, so you get a large relative increase in time.

Doing this:

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index efc6e26fd4..a8d198ffd0 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -987,6 +987,9 @@ static int run_update_hook(struct command *cmd)
 	int saved_stderr = -1;
 	int code;
 
+	if (!find_hook(the_repository, "update"))
+		return 0;
+
 	strvec_pushl(&opt.args,
 		     cmd->ref_name,
 		     oid_to_hex(&cmd->old_oid),

restores the benchmark, but there might be a cleaner way to integrate it
with the rest of the hook infrastructure. And probably the same thing
should be done for other hooks, too.

-Peff

Re: Performance regression in "update" hooks

[Adrian Ratiu]

On Mon, 02 Mar 2026, Jeff King <peff@peff.net> wrote:
> On Mon, Mar 02, 2026 at 03:27:22PM +0100, Patrick Steinhardt wrote:
>
>> > > I don't have the benchmark setup and it might be easier for you to
>> > > confirm?
>> 
>> All you need is a normal development infra and hyperfine. The
>> benchmarking scripts in the repo I linked should then "just work" with
>> the above invocation.
>
> Thanks, these were very cool and easy to use.
>
> Looking at the patch, my guess was that the problem is that we are now
> setting up and tearing down the sideband muxer for each hook invocation.
> This is expensive for the "update" hook, since it fires once per ref.

I independently root caused it and came up with (mostly) the same fix,
so this is a very good confirmation, thanks!

Please wait for my patch because it needs fixing it 3 places, for 3
hooks which spin up/down no-op async threads. :)

Re: Performance regression in "update" hooks

[Junio C Hamano]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> On Mon, 02 Mar 2026, Jeff King <peff@peff.net> wrote:
>> On Mon, Mar 02, 2026 at 03:27:22PM +0100, Patrick Steinhardt wrote:
>>
>>> > > I don't have the benchmark setup and it might be easier for you to
>>> > > confirm?
>>> 
>>> All you need is a normal development infra and hyperfine. The
>>> benchmarking scripts in the repo I linked should then "just work" with
>>> the above invocation.
>>
>> Thanks, these were very cool and easy to use.
>>
>> Looking at the patch, my guess was that the problem is that we are now
>> setting up and tearing down the sideband muxer for each hook invocation.
>> This is expensive for the "update" hook, since it fires once per ref.
>
> I independently root caused it and came up with (mostly) the same fix,
> so this is a very good confirmation, thanks!
>
> Please wait for my patch because it needs fixing it 3 places, for 3
> hooks which spin up/down no-op async threads. :)

Thanks for working on the problem report and coming to a fix so
quickly.