Re: [PATCH 2/4] hook: allow parallel hook execution

[Patrick Steinhardt <ps@pks.im>]

On Wed, Feb 04, 2026 at 07:33:26PM +0200, Adrian Ratiu wrote:
> From: Emily Shaffer <emilyshaffer@google.com>
> 
> In many cases, there's no reason not to allow hooks to execute in
> parallel, if more than one was provided.
> 
> hook.c already calls run_processes_parallel() so all we need to do is
> allow its job count to be greater than 1.
> 
> Serial execution is achieved by setting .jobs == 1 at compile time via
> RUN_HOOKS_OPT_INIT_SERIAL or by setting the 'hook.jobs' config to 1.
> This matches the behavior prior to this commit.
> 
> The compile-time 'struct run_hooks_opt.jobs' parameter has the highest
> priority if non-zero, followed by the 'hook.jobs' user config, then the
> processor count from online_cpus() is the last fallback.

Wait, the compile-time parameter overrides the user configuration? That
doesn't seem right to me.

I'm also a bit sceptical whether we should really default to
`online_cpus()`. If so, we start to assume semantics of the hooks
themselves, and that they cannot conflict with one another. But this is
nothing we can really guarantee. It might be that multiple hooks want to
modify the same data structure, and if so running them in parallel would
lead to races.

So I wonder whether we should rather make this behaviour opt-in than
opt-out.

> The above ordering ensures hooks unsafe to run in parallel are always
> executed sequentially (RUN_HOOKS_OPT_INIT_SERIAL) while allowing users
> to control parallelism with an efficient default.

Ah, okay, we only let the compile-time parameter override the config in
case we know that hooks must run in serial. That makes a bit more sense.

> diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc
> index 49c7ffd82e..c394756328 100644
> --- a/Documentation/config/hook.adoc
> +++ b/Documentation/config/hook.adoc
> @@ -15,3 +15,8 @@ hook.<name>.event::
>  	On the specified event, the associated `hook.<name>.command` will be
>  	executed. More than one event can be specified if you wish for
>  	`hook.<name>` to execute on multiple events. See linkgit:git-hook[1].
> +
> +hook.jobs::
> +	Specifies how many hooks can be run simultaneously during parallelized
> +	hook execution. If unspecified, defaults to the number of processors on
> +	the current system.

We should probably note that some hooks will run sequentially regardless
of this setting. Maybe we should even document which ones? I expect it's
not going to be that many.

> diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
> index 5f339dc48b..72c6c6d1ee 100644
> --- a/Documentation/git-hook.adoc
> +++ b/Documentation/git-hook.adoc
> @@ -128,6 +129,16 @@ OPTIONS
>  	tools that want to do a blind one-shot run of a hook that may
>  	or may not be present.
>  
> +-j::
> +--jobs::
> +	Only valid for `run`.
> ++
> +Specify how many hooks to run simultaneously. If this flag is not specified,
> +the value of the `hook.jobs` config is used, see linkgit:git-config[1]. If the
> +config is not specified, the number of CPUs on the current system is used. Some
> +hooks may be ineligible for parallelization: for example, 'commit-msg' hooks
> +typically modify the commit message body and cannot be parallelized.

Yeah, this info is probably what I was searching for in the "hook.jobs"
description.

> diff --git a/builtin/hook.c b/builtin/hook.c
> index 4cc6dac45a..cd1f4ebe6a 100644
> --- a/builtin/hook.c
> +++ b/builtin/hook.c
> @@ -76,7 +77,7 @@ static int run(int argc, const char **argv, const char *prefix,
>  	       struct repository *repo UNUSED)
>  {
>  	int i;
> -	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
> +	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL;
>  	int ignore_missing = 0;
>  	const char *hook_name;
>  	struct option run_options[] = {

Hm. Assuming that the user executes `git hooks run prepare-commit-msg`
with "--jobs=2", should we really honor that request? We know that the
hook cannot run in parallel, so we might want to refuse such requests.

Taking a step back, I wonder whether it really is sensible to declare
complete classes of hooks as parallelizable or non-parallelizable. We
have to assume semantics of the hook scripts themselves to be able to
answer whether or not they can be parallelizable. For some classes of
hooks like "prepare-commit-msg" we can assume that it's almost never
correct to serialize them. But for others we cannot assume anything.

Which makes me wonder whether the design here is really the right one.
Shouldn't we stop worrying about classes of hooks, but rather worry
about the user's intent? The user will know whether two hooks can run in
parallel or not, so let them tell us that this is the case.

I think this could be achieved via the configuration:

    [hook "my-parallelizable-hook-a"]
    path = /some/script-a.sh
    parallel = true

    [hook "my-parallelizable-hook"]
    path = /some/script-b.sh
    parallel = true

    [hook "serial-hook"]
    path = /some/script-c.sh
    parallel = false

This would tell us that we can safely run two of the hooks in parallel,
but not the third one. So we'd then first execute all serial hooks in
serial, and then in a second phase we'd execute the other hooks in
parallel.

Sure, this puts more responsibility on the user. But I think this is a
more flexible approach as it also empowers the user and caters to more
use cases.

Please let me know what you think.

Thanks!

Patrick