Skip to content

Commit 495b7d5

Browse files
10ne1gitster
authored andcommitted
hook: allow hook.jobs=-1 to use all available CPU cores
Allow -1 as a value for hook.jobs, hook.<event>.jobs, and the -j CLI flag to mean "use as many jobs as there are CPU cores", matching the convention used by fetch.parallel and other Git subsystems. The value is resolved to online_cpus() at parse time so the rest of the code always works with a positive resolved count. Other non-positive values (0, -2, etc) are rejected with a warning (config) or die (CLI). Suggested-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent dcfb5af commit 495b7d5

File tree

4 files changed

+108
-20
lines changed

4 files changed

+108
-20
lines changed

Documentation/config/hook.adoc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ hook.<event>.jobs::
5858
hook event (e.g. `hook.post-receive.jobs = 4`). Overrides `hook.jobs`
5959
for this specific event. The same parallelism restrictions apply: this
6060
setting has no effect unless all configured hooks for the event have
61-
`hook.<friendly-name>.parallel` set to `true`. Must be a positive int,
61+
`hook.<friendly-name>.parallel` set to `true`. Set to `-1` to use the
62+
number of available CPU cores. Must be a positive integer or `-1`;
6263
zero is rejected with a warning. See linkgit:git-hook[1].
6364
+
6465
Note on naming: although this key resembles `hook.<friendly-name>.*`
@@ -74,6 +75,7 @@ valid event name when setting `hook.<event>.jobs`.
7475
hook.jobs::
7576
Specifies how many hooks can be run simultaneously during parallelized
7677
hook execution. If unspecified, defaults to 1 (serial execution).
78+
Set to `-1` to use the number of available CPU cores.
7779
Can be overridden on a per-event basis with `hook.<event>.jobs`.
7880
Some hooks always run sequentially regardless of this setting because
7981
they operate on shared data and cannot safely be parallelized:

builtin/hook.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "gettext.h"
66
#include "hook.h"
77
#include "parse-options.h"
8+
#include "thread-utils.h"
89

910
#define BUILTIN_HOOK_RUN_USAGE \
1011
N_("git hook run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] [(-j|--jobs) <n>]\n" \
@@ -123,6 +124,7 @@ static int run(int argc, const char **argv, const char *prefix,
123124
struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
124125
int ignore_missing = 0;
125126
int allow_unknown = 0;
127+
int jobs = 0;
126128
const char *hook_name;
127129
struct option run_options[] = {
128130
OPT_BOOL(0, "allow-unknown-hook-name", &allow_unknown,
@@ -131,8 +133,8 @@ static int run(int argc, const char **argv, const char *prefix,
131133
N_("silently ignore missing requested <hook-name>")),
132134
OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"),
133135
N_("file to read into hooks' stdin")),
134-
OPT_UNSIGNED('j', "jobs", &opt.jobs,
135-
N_("run up to <n> hooks simultaneously")),
136+
OPT_INTEGER('j', "jobs", &jobs,
137+
N_("run up to <n> hooks simultaneously (-1 for CPU count)")),
136138
OPT_END(),
137139
};
138140
int ret;
@@ -141,6 +143,15 @@ static int run(int argc, const char **argv, const char *prefix,
141143
builtin_hook_run_usage,
142144
PARSE_OPT_KEEP_DASHDASH);
143145

146+
if (jobs == -1)
147+
opt.jobs = online_cpus();
148+
else if (jobs < 0)
149+
die(_("invalid value for -j: %d"
150+
" (use -1 for CPU count or a"
151+
" positive integer)"), jobs);
152+
else
153+
opt.jobs = jobs;
154+
144155
if (!argc)
145156
goto usage;
146157

hook.c

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "setup.h"
1313
#include "strbuf.h"
1414
#include "strmap.h"
15+
#include "thread-utils.h"
1516

1617
bool is_known_hook(const char *name)
1718
{
@@ -165,13 +166,17 @@ static int hook_config_lookup_all(const char *key, const char *value,
165166
/* Handle plain hook.<key> entries that have no hook name component. */
166167
if (!name) {
167168
if (!strcmp(subkey, "jobs") && value) {
168-
unsigned int v;
169-
if (!git_parse_uint(value, &v))
170-
warning(_("hook.jobs must be a positive integer, ignoring: '%s'"), value);
171-
else if (!v)
172-
warning(_("hook.jobs must be positive, ignoring: 0"));
173-
else
169+
int v;
170+
if (!git_parse_int(value, &v))
171+
warning(_("hook.jobs must be an integer, ignoring: '%s'"), value);
172+
else if (v == -1)
173+
data->jobs = online_cpus();
174+
else if (v > 0)
174175
data->jobs = v;
176+
else
177+
warning(_("hook.jobs must be a positive integer"
178+
" or -1, ignoring: '%s'"),
179+
value);
175180
}
176181
return 0;
177182
}
@@ -259,17 +264,21 @@ static int hook_config_lookup_all(const char *key, const char *value,
259264
" ignoring: '%s'"),
260265
hook_name, value);
261266
} else if (!strcmp(subkey, "jobs")) {
262-
unsigned int v;
263-
if (!git_parse_uint(value, &v))
264-
warning(_("hook.%s.jobs must be a positive integer,"
267+
int v;
268+
if (!git_parse_int(value, &v))
269+
warning(_("hook.%s.jobs must be an integer,"
265270
" ignoring: '%s'"),
266271
hook_name, value);
267-
else if (!v)
268-
warning(_("hook.%s.jobs must be positive,"
269-
" ignoring: 0"), hook_name);
270-
else
272+
else if (v == -1)
273+
strmap_put(&data->event_jobs, hook_name,
274+
(void *)(uintptr_t)online_cpus());
275+
else if (v > 0)
271276
strmap_put(&data->event_jobs, hook_name,
272277
(void *)(uintptr_t)v);
278+
else
279+
warning(_("hook.%s.jobs must be a positive"
280+
" integer or -1, ignoring: '%s'"),
281+
hook_name, value);
273282
}
274283

275284
free(hook_name);
@@ -688,6 +697,25 @@ static void warn_non_parallel_hooks_override(unsigned int jobs,
688697
}
689698
}
690699

700+
/* Resolve a hook.jobs config key, handling -1 as online_cpus(). */
701+
static void resolve_hook_config_jobs(struct repository *r,
702+
const char *key,
703+
unsigned int *jobs)
704+
{
705+
int v;
706+
707+
if (repo_config_get_int(r, key, &v))
708+
return;
709+
710+
if (v == -1)
711+
*jobs = online_cpus();
712+
else if (v > 0)
713+
*jobs = v;
714+
else
715+
warning(_("%s must be a positive integer or -1,"
716+
" ignoring: %d"), key, v);
717+
}
718+
691719
/* Determine how many jobs to use for hook execution. */
692720
static unsigned int get_hook_jobs(struct repository *r,
693721
struct run_hooks_opt *options,
@@ -721,14 +749,12 @@ static unsigned int get_hook_jobs(struct repository *r,
721749
if (event_jobs)
722750
options->jobs = (unsigned int)(uintptr_t)event_jobs;
723751
} else {
724-
unsigned int event_jobs;
725752
char *key;
726753

727-
repo_config_get_uint(r, "hook.jobs", &options->jobs);
754+
resolve_hook_config_jobs(r, "hook.jobs", &options->jobs);
728755

729756
key = xstrfmt("hook.%s.jobs", hook_name);
730-
if (!repo_config_get_uint(r, key, &event_jobs) && event_jobs)
731-
options->jobs = event_jobs;
757+
resolve_hook_config_jobs(r, key, &options->jobs);
732758
free(key);
733759
}
734760
}

t/t1800-hook.sh

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,6 +1058,55 @@ test_expect_success 'hook.<event>.jobs does not warn for a real event name' '
10581058
test_grep ! "friendly-name" err
10591059
'
10601060

1061+
test_expect_success 'hook.jobs=-1 resolves to online_cpus()' '
1062+
test_config hook.hook-1.event test-hook &&
1063+
test_config hook.hook-1.command "true" &&
1064+
test_config hook.hook-1.parallel true &&
1065+
1066+
test_config hook.jobs -1 &&
1067+
1068+
cpus=$(test-tool online-cpus) &&
1069+
GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
1070+
git hook run --allow-unknown-hook-name test-hook >out 2>err &&
1071+
grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:$cpus\"" trace.txt
1072+
'
1073+
1074+
test_expect_success 'hook.<event>.jobs=-1 resolves to online_cpus()' '
1075+
test_config hook.hook-1.event test-hook &&
1076+
test_config hook.hook-1.command "true" &&
1077+
test_config hook.hook-1.parallel true &&
1078+
1079+
test_config hook.test-hook.jobs -1 &&
1080+
1081+
cpus=$(test-tool online-cpus) &&
1082+
GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
1083+
git hook run --allow-unknown-hook-name test-hook >out 2>err &&
1084+
grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:$cpus\"" trace.txt
1085+
'
1086+
1087+
test_expect_success 'git hook run -j-1 resolves to online_cpus()' '
1088+
test_config hook.hook-1.event test-hook &&
1089+
test_config hook.hook-1.command "true" &&
1090+
test_config hook.hook-1.parallel true &&
1091+
1092+
cpus=$(test-tool online-cpus) &&
1093+
GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
1094+
git hook run --allow-unknown-hook-name -j-1 test-hook >out 2>err &&
1095+
grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:$cpus\"" trace.txt
1096+
'
1097+
1098+
test_expect_success 'hook.jobs rejects values less than -1' '
1099+
test_config hook.jobs -2 &&
1100+
git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err &&
1101+
test_grep "hook.jobs must be a positive integer or -1" err
1102+
'
1103+
1104+
test_expect_success 'hook.<event>.jobs rejects values less than -1' '
1105+
test_config hook.test-hook.jobs -5 &&
1106+
git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err &&
1107+
test_grep "hook.test-hook.jobs must be a positive integer or -1" err
1108+
'
1109+
10611110
test_expect_success 'hook.<event>.enabled=false skips all hooks for event' '
10621111
test_config hook.hook-1.event test-hook &&
10631112
test_config hook.hook-1.command "echo ran" &&

0 commit comments

Comments
 (0)