Re: Performance regression in "update" hooks
- From
Jeff King <peff@peff.net>
- Date
- Mar 2, 2026, 17:50 UTC
- Message-ID
- <20260302175052.GA28275@coredump.intra.peff.net>
- In-Reply-To
- <aaWeSu-d1FMz_sW8@pks.im>
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