Re: [PATCH 1/1] builtin/receive-pack: avoid spinning no-op sideband async threads

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 3, 2026, 06:11 UTC
Message-ID: <aaZ7eXtUSWSS_igX@pks.im>
In-Reply-To: <20260302191704.1814567-2-adrian.ratiu@collabora.com>

On Mon, Mar 02, 2026 at 09:17:04PM +0200, Adrian Ratiu wrote:

Show 6 quoted lines

> Exit early if the hooks do not exist, to avoid spinning up/down
> sideband async threads which no-op.
> 
> It is important to call the hook_exists() API provided by hook.[ch]
> because it covers both config-defined hooks and the "traditional"
> hooks from the hookdir. find_hook() only covers the hookdir hooks.

Just out of curiosity: will `find_hook()` eventually be removed? I saw that we still use it for the "proc-receive" hook in git-receive-pack(1) for example, which feels a bit fishy to me.

In any case, if this is an oversight then this can be handled in a subsequent patch series, if you ask me.

Show 14 quoted lines

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 139a227e71..6376c191c7 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -934,6 +934,9 @@ static int run_receive_hook(struct command *commands,
>  	int saved_stderr = -1;
>  	int ret;
>  
> +	if (!hook_exists(the_repository, hook_name))
> +		return 0;
> +
>  	/* if there are no valid commands, don't invoke the hook at all. */
>  	while (iter && skip_broken && (iter->error_string || iter->did_not_exist))
>  		iter = iter->next;

That fix is delightfully simple -- I was fearing for a deeper issue. I can confirm that this restores original performance:

  Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~)
    Time (mean ± σ):     177.4 ms ±   3.0 ms    [User: 92.0 ms, System: 84.2 ms]
    Range (min … max):   172.1 ms … 182.6 ms    15 runs

  Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)
    Time (mean ± σ):     485.0 ms ±   7.1 ms    [User: 180.0 ms, System: 375.0 ms]
    Range (min … max):   466.9 ms … 491.0 ms    10 runs

  Benchmark 3: receive: many refs (refformat = reftable, refcount = 10000, revision = 005f3fbe07a20dd5f7dea57f6f46cd797387e56a)
    Time (mean ± σ):     178.1 ms ±   2.4 ms    [User: 91.8 ms, System: 85.1 ms]
    Range (min … max):   172.2 ms … 181.3 ms    15 runs

  Summary
    receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9~) ran
      1.00 ± 0.02 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = 005f3fbe07a20dd5f7dea57f6f46cd797387e56a)
      2.73 ± 0.06 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = fc148b146ad41be71a7852c4867f0773cbfe1ff9)

And Bencher has already picked up those changes, too, and graphs have dropped back to previous levels. Awesome.

Thanks a lot for the quick turnaround!

Patrick