threads / discuss / 65113

Performance regression in "update" hooks

Subject: Performance regression in "update" hooks

## tl;dr

7 messages between Mar 2, 2026 and Mar 2, 2026.

replies: 6people: 4as markdown or json

Patrick Steinhardt· Mar 2, 2026, 07:17 UTC · lore
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

Adrian Ratiu· Mar 2, 2026, 13:37 UTC · re: Patrick Steinhardt · lore

Re: Performance regression in "update" hooks

On Mon, 02 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
Show 28 quoted lines
> 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
Adrian Ratiu· Mar 2, 2026, 14:12 UTC · re: Adrian Ratiu · lore

Re: Performance regression in "update" hooks

On Mon, 02 Mar 2026, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
Show 70 quoted lines
> 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).

Patrick Steinhardt· Mar 2, 2026, 14:27 UTC · re: Adrian Ratiu · lore

Re: Performance regression in "update" hooks

On Mon, Mar 02, 2026 at 04:12:39PM +0200, Adrian Ratiu wrote:
Show 44 quoted lines
> 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
Jeff King· Mar 2, 2026, 17:50 UTC · re: Patrick Steinhardt · lore

Re: Performance regression in "update" hooks

On Mon, Mar 02, 2026 at 03:27:22PM +0100, Patrick Steinhardt wrote:
Show 6 quoted lines
> > > 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
Adrian Ratiu· Mar 2, 2026, 18:02 UTC · re: Jeff King · lore

Re: Performance regression in "update" hooks

On Mon, 02 Mar 2026, Jeff King <peff@peff.net> wrote:
Show 14 quoted lines
> 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. :)

Junio C Hamano· Mar 2, 2026, 18:54 UTC · re: Adrian Ratiu · lore

Re: Performance regression in "update" hooks

Adrian Ratiu <adrian.ratiu@collabora.com> writes:
Show 21 quoted lines
> 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.

← back to recent threads