public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Adrian Ratiu <adrian.ratiu@collabora.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Emily Shaffer <emilyshaffer@google.com>,
	Jeff King <peff@peff.net>
Subject: Re: [PATCH 1/1] builtin/receive-pack: avoid spinning no-op sideband async threads
Date: Tue, 3 Mar 2026 07:11:05 +0100	[thread overview]
Message-ID: <aaZ7eXtUSWSS_igX@pks.im> (raw)
In-Reply-To: <20260302191704.1814567-2-adrian.ratiu@collabora.com>

On Mon, Mar 02, 2026 at 09:17:04PM +0200, Adrian Ratiu wrote:
> 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.

> 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

  parent reply	other threads:[~2026-03-03  6:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-02 19:17 [PATCH 0/1] Fix update hook perf regression in next Adrian Ratiu
2026-03-02 19:17 ` [PATCH 1/1] builtin/receive-pack: avoid spinning no-op sideband async threads Adrian Ratiu
2026-03-02 21:40   ` Junio C Hamano
2026-03-03 12:47     ` Adrian Ratiu
2026-03-03  6:11   ` Patrick Steinhardt [this message]
2026-03-03 12:45     ` Adrian Ratiu
2026-03-03 13:28   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aaZ7eXtUSWSS_igX@pks.im \
    --to=ps@pks.im \
    --cc=adrian.ratiu@collabora.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox