{"thread":{"id":"65113","subject":"Performance regression in \"update\" hooks","startedAt":"2026-03-02T07:17:52Z","lastAt":"2026-03-02T18:54:21Z","messageCount":7,"participants":["Patrick Steinhardt","Adrian Ratiu","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"537496","messageId":"aaU5lZwEuR4OrxCl@pks.im","threadId":"65113","inReplyTo":null,"subject":"Performance regression in \"update\" hooks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-02T07:17:41Z","receivedAt":"2026-03-02T07:17:52Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nBencher has alerted me that there's been two performance regressions in\ngit-receive-pack(1) [1] and git-fetch(1) [2].\n\nThe first one is quite easy to reproduce with the benchmarks at [3] and\nbisects to fc148b146a (receive-pack: convert update hooks to new API,\n2026-01-28):\n\n $ cd receive-refs\n $ ./run --revisions /path/to/your/git/repo \\\n fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \\\n --parameter-list refformat reftable \\\n --parameter-list refcount 10000\n\n Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)\n Time (mean ± σ): 182.0 ms ± 2.7 ms [User: 91.5 ms, System: 89.3 ms]\n Range (min … max): 175.8 ms … 185.0 ms 15 runs\n\n Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)\n Time (mean ± σ): 484.6 ms ± 27.6 ms [User: 176.2 ms, System: 376.1 ms]\n Range (min … max): 406.2 ms … 495.1 ms 10 runs\n\n Summary\n receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran\n 2.66 ± 0.16 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)\n\nI've Cc'd Adrian.\n\nThe other performance regression seems to be present in both\ngit-receive-pack(1) and git-fetch(1) and happens between e6e9f13364\n(Sync with 'master', 2026-02-25) and ebd1da8b75 (Merge branch\n'cx/fetch-display-ubfix' into next, 2026-02-26). It took me a while to\nreproduce as my local Git configuration was hiding the regression, but I\nhave been able to bisect this to 452b12c2e0 (builtin/maintenance: use\n\"geometric\" strategy by default, 2026-02-24).\n\nThe problem here is rather simple though. The benchmark fetches 10,000\nrefs into the repository, and before the commit we didn't do anything\nabout them. But after the commit we now have per-data-structure tasks,\nand the result is that we thus end up packing refs. That's also why the\nregression isn't present in the reftable backend, as it wouldn't need\nany optimization.\n\nSo I'd consider this to be a bug in the benchmarking infrastructure\nitself that I'll fix by disabling auto-maintenance.\n\nThanks!\n\nPatrick\n\n[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\n[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\n[3]: https://gitlab.com/gitlab-org/data-access/git/benchmarks\n"},{"id":"537519","messageId":"87bjh673o0.fsf@gentoo.mail-host-address-is-not-set","threadId":"65113","inReplyTo":"aaU5lZwEuR4OrxCl@pks.im","subject":"Re: Performance regression in \"update\" hooks","fromName":"Adrian Ratiu","fromEmail":"adrian.ratiu@collabora.com","sentAt":"2026-03-02T13:37:51Z","receivedAt":"2026-03-02T13:38:02Z","isPatch":false,"sender":{"key":"adrian.ratiu@collabora.com","avatar":"https://avatars.githubusercontent.com/u/12472556?v=4"},"body":"On Mon, 02 Mar 2026, Patrick Steinhardt wrote:\n> Hi,\n>\n> Bencher has alerted me that there's been two performance regressions in\n> git-receive-pack(1) [1] and git-fetch(1) [2].\n>\n> The first one is quite easy to reproduce with the benchmarks at [3] and\n> bisects to fc148b146a (receive-pack: convert update hooks to new API,\n> 2026-01-28):\n>\n> $ cd receive-refs\n> $ ./run --revisions /path/to/your/git/repo \\\n> fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \\\n> --parameter-list refformat reftable \\\n> --parameter-list refcount 10000\n>\n> Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)\n> Time (mean ± σ): 182.0 ms ± 2.7 ms [User: 91.5 ms, System: 89.3 ms]\n> Range (min … max): 175.8 ms … 185.0 ms 15 runs\n>\n> Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)\n> Time (mean ± σ): 484.6 ms ± 27.6 ms [User: 176.2 ms, System: 376.1 ms]\n> Range (min … max): 406.2 ms … 495.1 ms 10 runs\n>\n> Summary\n> receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran\n> 2.66 ± 0.16 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)\n>\n> I've Cc'd Adrian.\n\nHi Patrick,\n\nI looked at the commits before and after the many-refs test regression\nand it appears the regressions started after Junio landed v2 of the\nconfig series in next [1], which might cause it.\n\nv2 was not ready to land. I sent v3 yesterday addressing all the\nfeedback, didn't even realize v2 landed. :)\n\nDoes the regression go away if you revert [1] ?\n\nI don't have the benchmark setup and it might be easier for you to\nconfirm?\n\nMany thanks!\n\n1:\n\ncommit 6a04cca28e210f0c51cfefcb52475c7ede6e99fb\nMerge: d6ebc97cb1 4b12cd3ae3\nAuthor: Junio C Hamano \nAuthorDate: Fri Feb 27 15:16:30 2026 -0800\nCommit: Junio C Hamano \nCommitDate: Fri Feb 27 15:16:30 2026 -0800\n\n Merge branch 'ar/config-hooks' into next\n \n Allow hook commands to be defined (possibly centrally) in the\n configuration files, and run multiple of them for the same hook\n event.\n \n * ar/config-hooks:\n hook: add -z option to \"git hook list\"\n hook: allow out-of-repo 'git hook' invocations\n hook: allow event = \"\" to overwrite previous values\n hook: allow disabling config hooks\n hook: include hooks from the config\n hook: add \"git hook list\" command\n hook: run a list of hooks to prepare for multihook support\n hook: add internal state alloc/free callbacks\n"},{"id":"537521","messageId":"874imy7220.fsf@collabora.com","threadId":"65113","inReplyTo":"87bjh673o0.fsf@gentoo.mail-host-address-is-not-set","subject":"Re: Performance regression in \"update\" hooks","fromName":"Adrian Ratiu","fromEmail":"adrian.ratiu@collabora.com","sentAt":"2026-03-02T14:12:39Z","receivedAt":"2026-03-02T14:12:48Z","isPatch":false,"sender":{"key":"adrian.ratiu@collabora.com","avatar":"https://avatars.githubusercontent.com/u/12472556?v=4"},"body":"On Mon, 02 Mar 2026, Adrian Ratiu wrote:\n> On Mon, 02 Mar 2026, Patrick Steinhardt wrote:\n>> Hi,\n>>\n>> Bencher has alerted me that there's been two performance regressions in\n>> git-receive-pack(1) [1] and git-fetch(1) [2].\n>>\n>> The first one is quite easy to reproduce with the benchmarks at [3] and\n>> bisects to fc148b146a (receive-pack: convert update hooks to new API,\n>> 2026-01-28):\n>>\n>> $ cd receive-refs\n>> $ ./run --revisions /path/to/your/git/repo \\\n>> fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \\\n>> --parameter-list refformat reftable \\\n>> --parameter-list refcount 10000\n>>\n>> Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)\n>> Time (mean ± σ): 182.0 ms ± 2.7 ms [User: 91.5 ms, System: 89.3 ms]\n>> Range (min … max): 175.8 ms … 185.0 ms 15 runs\n>>\n>> Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)\n>> Time (mean ± σ): 484.6 ms ± 27.6 ms [User: 176.2 ms, System: 376.1 ms]\n>> Range (min … max): 406.2 ms … 495.1 ms 10 runs\n>>\n>> Summary\n>> receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran\n>> 2.66 ± 0.16 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)\n>>\n>> I've Cc'd Adrian.\n>\n> Hi Patrick,\n>\n> I looked at the commits before and after the many-refs test regression\n> and it appears the regressions started after Junio landed v2 of the\n> config series in next [1], which might cause it.\n>\n> v2 was not ready to land. I sent v3 yesterday addressing all the\n> feedback, didn't even realize v2 landed. :)\n>\n> Does the regression go away if you revert [1] ?\n>\n> I don't have the benchmark setup and it might be easier for you to\n> confirm?\n>\n> Many thanks!\n>\n> 1:\n>\n> commit 6a04cca28e210f0c51cfefcb52475c7ede6e99fb\n> Merge: d6ebc97cb1 4b12cd3ae3\n> Author: Junio C Hamano \n> AuthorDate: Fri Feb 27 15:16:30 2026 -0800\n> Commit: Junio C Hamano \n> CommitDate: Fri Feb 27 15:16:30 2026 -0800\n>\n> Merge branch 'ar/config-hooks' into next\n> \n> Allow hook commands to be defined (possibly centrally) in the\n> configuration files, and run multiple of them for the same hook\n> event.\n> \n> * ar/config-hooks:\n> hook: add -z option to \"git hook list\"\n> hook: allow out-of-repo 'git hook' invocations\n> hook: allow event = \"\" to overwrite previous values\n> hook: allow disabling config hooks\n> hook: include hooks from the config\n> hook: add \"git hook list\" command\n> hook: run a list of hooks to prepare for multihook support\n> hook: add internal state alloc/free callbacks\n\nActually I think these are two separate issues.\n\nI will reproduce and look into the regression which bisected to\nc148b146a (receive-pack: convert update hooks to new API, 2026-01-28).\n"},{"id":"537526","messageId":"aaWeSu-d1FMz_sW8@pks.im","threadId":"65113","inReplyTo":"874imy7220.fsf@collabora.com","subject":"Re: Performance regression in \"update\" hooks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-02T14:27:22Z","receivedAt":"2026-03-02T14:27:28Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Mar 02, 2026 at 04:12:39PM +0200, Adrian Ratiu wrote:\n> On Mon, 02 Mar 2026, Adrian Ratiu wrote:\n> > On Mon, 02 Mar 2026, Patrick Steinhardt wrote:\n> >> Hi,\n> >>\n> >> Bencher has alerted me that there's been two performance regressions in\n> >> git-receive-pack(1) [1] and git-fetch(1) [2].\n> >>\n> >> The first one is quite easy to reproduce with the benchmarks at [3] and\n> >> bisects to fc148b146a (receive-pack: convert update hooks to new API,\n> >> 2026-01-28):\n> >>\n> >> $ cd receive-refs\n> >> $ ./run --revisions /path/to/your/git/repo \\\n> >> fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \\\n> >> --parameter-list refformat reftable \\\n> >> --parameter-list refcount 10000\n> >>\n> >> Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)\n> >> Time (mean ± σ): 182.0 ms ± 2.7 ms [User: 91.5 ms, System: 89.3 ms]\n> >> Range (min … max): 175.8 ms … 185.0 ms 15 runs\n> >>\n> >> Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)\n> >> Time (mean ± σ): 484.6 ms ± 27.6 ms [User: 176.2 ms, System: 376.1 ms]\n> >> Range (min … max): 406.2 ms … 495.1 ms 10 runs\n> >>\n> >> Summary\n> >> receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran\n> >> 2.66 ± 0.16 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)\n> >>\n> >> I've Cc'd Adrian.\n> >\n> > Hi Patrick,\n> >\n> > I looked at the commits before and after the many-refs test regression\n> > and it appears the regressions started after Junio landed v2 of the\n> > config series in next [1], which might cause it.\n> >\n> > v2 was not ready to land. I sent v3 yesterday addressing all the\n> > feedback, didn't even realize v2 landed. :)\n> >\n> > Does the regression go away if you revert [1] ?\n> >\n> > I don't have the benchmark setup and it might be easier for you to\n> > confirm?\n\nAll you need is a normal development infra and hyperfine. The\nbenchmarking scripts in the repo I linked should then \"just work\" with\nthe above invocation.\n\n[snip]\n> Actually I think these are two separate issues.\n> \n> I will reproduce and look into the regression which bisected to\n> c148b146a (receive-pack: convert update hooks to new API, 2026-01-28).\n\nPerfect, thanks!\n\nPatrick\n"},{"id":"537562","messageId":"20260302175052.GA28275@coredump.intra.peff.net","threadId":"65113","inReplyTo":"aaWeSu-d1FMz_sW8@pks.im","subject":"Re: Performance regression in \"update\" hooks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-02T17:50:52Z","receivedAt":"2026-03-02T17:57:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2026 at 03:27:22PM +0100, Patrick Steinhardt wrote:\n\n> > > I don't have the benchmark setup and it might be easier for you to\n> > > confirm?\n> \n> All you need is a normal development infra and hyperfine. The\n> benchmarking scripts in the repo I linked should then \"just work\" with\n> the above invocation.\n\nThanks, these were very cool and easy to use.\n\nLooking at the patch, my guess was that the problem is that we are now\nsetting up and tearing down the sideband muxer for each hook invocation.\nThis is expensive for the \"update\" hook, since it fires once per ref.\n\nAfter running the benchmark I tried tweaking the \"stdin\" file to replace\n\"side-band-64k\" with \"not-side-band\" (which conveniently is the same\nlength and thus you don't need to update the pkt-line header). And it\ndoes make the slowdown go away. (Sadly that input is generated on the\nfly by the benchmark, so you have to time with your own invocation).\n\nI think it wouldn't be _quite_ so bad if we actually had an update hook,\nbecause then we'd be paying the cost to exec the hook for each ref. So\nthe extra work to setup the sideband would be less noticeable.\n\nBut it looks like the sideband setup happens even if we aren't going to\nrun anything, so you get a large relative increase in time.\n\nDoing this:\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex efc6e26fd4..a8d198ffd0 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -987,6 +987,9 @@ static int run_update_hook(struct command *cmd)\n \tint saved_stderr = -1;\n \tint code;\n \n+\tif (!find_hook(the_repository, \"update\"))\n+\t\treturn 0;\n+\n \tstrvec_pushl(&opt.args,\n \t\t cmd->ref_name,\n \t\t oid_to_hex(&cmd->old_oid),\n\nrestores the benchmark, but there might be a cleaner way to integrate it\nwith the rest of the hook infrastructure. And probably the same thing\nshould be done for other hooks, too.\n\n-Peff\n"},{"id":"537563","messageId":"87wlzu5cug.fsf@collabora.com","threadId":"65113","inReplyTo":"20260302175052.GA28275@coredump.intra.peff.net","subject":"Re: Performance regression in \"update\" hooks","fromName":"Adrian Ratiu","fromEmail":"adrian.ratiu@collabora.com","sentAt":"2026-03-02T18:02:31Z","receivedAt":"2026-03-02T18:02:44Z","isPatch":false,"sender":{"key":"adrian.ratiu@collabora.com","avatar":"https://avatars.githubusercontent.com/u/12472556?v=4"},"body":"On Mon, 02 Mar 2026, Jeff King wrote:\n> On Mon, Mar 02, 2026 at 03:27:22PM +0100, Patrick Steinhardt wrote:\n>\n>> > > I don't have the benchmark setup and it might be easier for you to\n>> > > confirm?\n>> \n>> All you need is a normal development infra and hyperfine. The\n>> benchmarking scripts in the repo I linked should then \"just work\" with\n>> the above invocation.\n>\n> Thanks, these were very cool and easy to use.\n>\n> Looking at the patch, my guess was that the problem is that we are now\n> setting up and tearing down the sideband muxer for each hook invocation.\n> This is expensive for the \"update\" hook, since it fires once per ref.\n\nI independently root caused it and came up with (mostly) the same fix,\nso this is a very good confirmation, thanks!\n\nPlease wait for my patch because it needs fixing it 3 places, for 3\nhooks which spin up/down no-op async threads. :)\n"},{"id":"537585","messageId":"xmqq7bru12qs.fsf@gitster.g","threadId":"65113","inReplyTo":"87wlzu5cug.fsf@collabora.com","subject":"Re: Performance regression in \"update\" hooks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T18:54:19Z","receivedAt":"2026-03-02T18:54:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adrian Ratiu writes:\n\n> On Mon, 02 Mar 2026, Jeff King wrote:\n>> On Mon, Mar 02, 2026 at 03:27:22PM +0100, Patrick Steinhardt wrote:\n>>\n>>> > > I don't have the benchmark setup and it might be easier for you to\n>>> > > confirm?\n>>> \n>>> All you need is a normal development infra and hyperfine. The\n>>> benchmarking scripts in the repo I linked should then \"just work\" with\n>>> the above invocation.\n>>\n>> Thanks, these were very cool and easy to use.\n>>\n>> Looking at the patch, my guess was that the problem is that we are now\n>> setting up and tearing down the sideband muxer for each hook invocation.\n>> This is expensive for the \"update\" hook, since it fires once per ref.\n>\n> I independently root caused it and came up with (mostly) the same fix,\n> so this is a very good confirmation, thanks!\n>\n> Please wait for my patch because it needs fixing it 3 places, for 3\n> hooks which spin up/down no-op async threads. :)\n\nThanks for working on the problem report and coming to a fix so\nquickly.\n\n"}]}