git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v7 12/17] hook: allow parallel hook execution

From
Emily Shaffer <emilyshaffer@google.com>
Date
Dec 22, 2020, 00:02 UTC
Message-ID
<20201222000220.1491091-13-emilyshaffer@google.com>
In-Reply-To
<20201222000220.1491091-1-emilyshaffer@google.com>

In many cases, there's no reason not to allow hooks to execute in parallel. run_processes_parallel() is well-suited - it's a task queue that runs its housekeeping in series, which means users don't need to worry about thread safety on their callback data. True multithreaded execution with the async_* functions isn't necessary here. Synchronous hook execution can be achieved by only allowing 1 job to run at a time.

Teach run_hooks() to use that function for simple hooks which don't require stdin or capture of stderr.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
Notes:
    Per AEvar's request - parallel hook execution on day zero.
    
    In most ways run_processes_parallel() worked great for me - but it didn't
    have great support for hooks where we pipe to and from. I had to add this
    support later in the series.
    
    Since I modified an existing and in-use library I'd appreciate a keen look on
    these patches.
    
     - Emily
 Documentation/config/hook.txt |   5 ++
 Documentation/git-hook.txt    |  14 +++-
 builtin/hook.c                |   6 +-
 hook.c                        | 142 ++++++++++++++++++++++++++--------
 hook.h                        |  28 ++++++-
 5 files changed, 157 insertions(+), 38 deletions(-)
diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt
index 75312754ae..a423d13781 100644
--- a/Documentation/config/hook.txt
+++ b/Documentation/config/hook.txt
@@ -12,3 +12,8 @@ hook.runHookDir::
 	Controls how hooks contained in your hookdir are executed. Can be any of
 	"yes", "warn", "interactive", or "no". Defaults to "yes". See
 	linkgit:git-hook[1] and linkgit:git-config[1] "core.hooksPath").
+
+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.
diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
index cce30a80d0..01cee4ad81 100644
--- a/Documentation/git-hook.txt
+++ b/Documentation/git-hook.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git hook' list <hook-name>
 'git hook' run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] [--to-stdin=<path>]
-	<hook-name>
+	[(-j|--jobs) <n>] <hook-name>
 
 DESCRIPTION
 -----------
@@ -66,7 +66,7 @@ in the order they should be run, and print the config scope where the relevant
 `hook.<hook-name>.command` was specified, not the `hookcmd` (if applicable).
 This output is human-readable and the format is subject to change over time.
 
-run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] [--to-stdin=<path>] `<hook-name>`::
+run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] [--to-stdin=<path>] [(-j|--jobs)<n>] `<hook-name>`::
 
 Runs hooks configured for `<hook-name>`, in the same order displayed by `git
 hook list`. Hooks configured this way are run prepended with `sh -c`, so paths
@@ -98,6 +98,16 @@ Specify environment variables to set for every hook that is run.
 Specify a file which will be streamed into stdin for every hook that is run.
 Each hook will receive the entire file from beginning to EOF.
 
+-j::
+--jobs::
+	Only valid for `run`.
++
+Specify how many hooks to run simultaneously. If this flag is not specified, use
+the value of the `hook.jobs` config. If the config is not specified, use the
+number of CPUs on the current system. Some hooks may be ineligible for
+parallelization: for example, 'commit-msg' intends hooks modify the commit
+message body and cannot be parallelized.
+
 CONFIGURATION
 -------------
 include::config/hook.txt[]
diff --git a/builtin/hook.c b/builtin/hook.c
index be104f2938..7fbc84ab64 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -10,7 +10,7 @@
 static const char * const builtin_hook_usage[] = {
 	N_("git hook list <hookname>"),
 	N_("git hook run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...]"
-	   "[--to-stdin=<path>] <hookname>"),
+	   "[--to-stdin=<path>] [(-j|--jobs) <count>] <hookname>"),
 	NULL
 };
 
@@ -90,7 +90,7 @@ static int list(int argc, const char **argv, const char *prefix)
 static int run(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf hookname = STRBUF_INIT;
-	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_ASYNC;
 	int rc = 0;
 
 	struct option run_options[] = {
@@ -100,6 +100,8 @@ static int run(int argc, const char **argv, const char *prefix)
 			   N_("argument to pass to hook")),
 		OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"),
 			   N_("file to read into hooks' stdin")),
+		OPT_INTEGER('j', "jobs", &opt.jobs,
+			    N_("run up to <n> hooks simultaneously")),
 		OPT_END(),
 	};
 
diff --git a/hook.c b/hook.c
index ce5c443206..b190afa33b 100644
--- a/hook.c
+++ b/hook.c
@@ -136,6 +136,14 @@ enum hookdir_opt configured_hookdir_opt(void)
 	return HOOKDIR_UNKNOWN;
 }
 
+int configured_hook_jobs(void)
+{
+	int n = online_cpus();
+	git_config_get_int("hook.jobs", &n);
+
+	return n;
+}
+
 static int should_include_hookdir(const char *path, enum hookdir_opt cfg)
 {
 	struct strbuf prompt = STRBUF_INIT;
@@ -223,6 +231,7 @@ void run_hooks_opt_init(struct run_hooks_opt *o)
 	strvec_init(&o->env);
 	strvec_init(&o->args);
 	o->run_hookdir = configured_hookdir_opt();
+	o->jobs = configured_hook_jobs();
 }
 
 int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir)
@@ -246,11 +255,96 @@ void run_hooks_opt_clear(struct run_hooks_opt *o)
 	strvec_clear(&o->args);
 }
 
+
+static int pick_next_hook(struct child_process *cp,
+			  struct strbuf *out,
+			  void *pp_cb,
+			  void **pp_task_cb)
+{
+	struct hook_cb_data *hook_cb = pp_cb;
+
+	struct hook *hook = list_entry(hook_cb->run_me, struct hook, list);
+
+	if (hook_cb->head == hook_cb->run_me)
+		return 0;
+
+	cp->env = hook_cb->options->env.v;
+	cp->stdout_to_stderr = 1;
+	cp->trace2_hook_name = hook->command.buf;
+
+	/* reopen the file for stdin; run_command closes it. */
+	if (hook_cb->options->path_to_stdin) {
+		cp->no_stdin = 0;
+		cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY);
+	} else {
+		cp->no_stdin = 1;
+	}
+
+	/*
+	 * Commands from the config could be oneliners, but we know
+	 * for certain that hookdir commands are not.
+	 */
+	if (hook->from_hookdir)
+		cp->use_shell = 0;
+	else
+		cp->use_shell = 1;
+
+	/* add command */
+	strvec_push(&cp->args, hook->command.buf);
+
+	/*
+	 * add passed-in argv, without expanding - let the user get back
+	 * exactly what they put in
+	 */
+	strvec_pushv(&cp->args, hook_cb->options->args.v);
+
+	/* Provide context for errors if necessary */
+	*pp_task_cb = hook;
+
+	/* Get the next entry ready */
+	hook_cb->run_me = hook_cb->run_me->next;
+
+	return 1;
+}
+
+static int notify_start_failure(struct strbuf *out,
+				void *pp_cb,
+				void *pp_task_cp)
+{
+	struct hook_cb_data *hook_cb = pp_cb;
+	struct hook *attempted = pp_task_cp;
+
+	/* |= rc in cb */
+	hook_cb->rc |= 1;
+
+	strbuf_addf(out, _("Couldn't start '%s', configured in '%s'\n"),
+		    attempted->command.buf,
+		    attempted->from_hookdir ? "hookdir"
+		    	: config_scope_name(attempted->origin));
+
+	/* NEEDSWORK: if halt_on_error is desired, do it here. */
+	return 0;
+}
+
+static int notify_hook_finished(int result,
+				struct strbuf *out,
+				void *pp_cb,
+				void *pp_task_cb)
+{
+	struct hook_cb_data *hook_cb = pp_cb;
+
+	/* |= rc in cb */
+	hook_cb->rc |= result;
+
+	/* NEEDSWORK: if halt_on_error is desired, do it here. */
+	return 0;
+}
+
 int run_hooks(const char *hookname, struct run_hooks_opt *options)
 {
 	struct strbuf hookname_str = STRBUF_INIT;
 	struct list_head *to_run, *pos = NULL, *tmp = NULL;
-	int rc = 0;
+	struct hook_cb_data cb_data = { 0, NULL, NULL, options };
 
 	if (!options)
 		BUG("a struct run_hooks_opt must be provided to run_hooks");
@@ -260,41 +354,23 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options)
 	to_run = hook_list(&hookname_str);
 
 	list_for_each_safe(pos, tmp, to_run) {
-		struct child_process hook_proc = CHILD_PROCESS_INIT;
 		struct hook *hook = list_entry(pos, struct hook, list);
 
-		/* reopen the file for stdin; run_command closes it. */
-		if (options->path_to_stdin)
-			hook_proc.in = xopen(options->path_to_stdin, O_RDONLY);
-		else
-			hook_proc.no_stdin = 1;
-
-		hook_proc.env = options->env.v;
-		hook_proc.stdout_to_stderr = 1;
-		hook_proc.trace2_hook_name = hook->command.buf;
-		hook_proc.use_shell = 1;
-
-		if (hook->from_hookdir) {
-		    if (!should_include_hookdir(hook->command.buf, options->run_hookdir))
-			continue;
-		    /*
-		     * Commands from the config could be oneliners, but we know
-		     * for certain that hookdir commands are not.
-		     */
-		    hook_proc.use_shell = 0;
-		}
-
-		/* add command */
-		strvec_push(&hook_proc.args, hook->command.buf);
+		if (hook->from_hookdir &&
+		    !should_include_hookdir(hook->command.buf, options->run_hookdir))
+			    list_del(pos);
+	}
 
-		/*
-		 * add passed-in argv, without expanding - let the user get back
-		 * exactly what they put in
-		 */
-		strvec_pushv(&hook_proc.args, options->args.v);
+	cb_data.head = to_run;
+	cb_data.run_me = to_run->next;
 
-		rc |= run_command(&hook_proc);
-	}
+	run_processes_parallel_tr2(options->jobs,
+				   pick_next_hook,
+				   notify_start_failure,
+				   notify_hook_finished,
+				   &cb_data,
+				   "hook",
+				   hookname);
 
-	return rc;
+	return cb_data.rc;
 }
diff --git a/hook.h b/hook.h
index e22a6db832..0d973d090f 100644
--- a/hook.h
+++ b/hook.h
@@ -37,6 +37,9 @@ enum hookdir_opt
  */
 enum hookdir_opt configured_hookdir_opt(void);
 
+/* Provides the number of threads to use for parallel hook execution. */
+int configured_hook_jobs(void);
+
 struct run_hooks_opt
 {
 	/* Environment vars to be set for each hook */
@@ -54,15 +57,38 @@ struct run_hooks_opt
 
 	/* Path to file which should be piped to stdin for each hook */
 	const char *path_to_stdin;
+
+	/* Number of threads to parallelize across */
+	int jobs;
 };
 
-#define RUN_HOOKS_OPT_INIT  {   		\
+/*
+ * Callback provided to feed_pipe_fn and consume_sideband_fn.
+ */
+struct hook_cb_data {
+	int rc;
+	struct list_head *head;
+	struct list_head *run_me;
+	struct run_hooks_opt *options;
+};
+
+#define RUN_HOOKS_OPT_INIT_SYNC  {   		\
 	.env = STRVEC_INIT, 			\
 	.args = STRVEC_INIT, 			\
 	.path_to_stdin = NULL,			\
+	.jobs = 1,				\
 	.run_hookdir = configured_hookdir_opt()	\
 }
 
+#define RUN_HOOKS_OPT_INIT_ASYNC {		\
+	.env = STRVEC_INIT, 			\
+	.args = STRVEC_INIT, 			\
+	.path_to_stdin = NULL,			\
+	.jobs = configured_hook_jobs(),		\
+	.run_hookdir = configured_hookdir_opt()	\
+}
+
+
 void run_hooks_opt_init(struct run_hooks_opt *o);
 void run_hooks_opt_clear(struct run_hooks_opt *o);
 

-- 
2.28.0.rc0.142.g3c755180ce-goog
Previous: Emily ShafferNext: Jonathan Tan
Message 57 of 89 in “propose config-based hooks (part I)”
  1. 00/17 propose config-based hooks (part I)Emily Shaffer, Dec 5, 2020
  2. 01/17 doc: propose hooks managed by the configEmily Shaffer, Dec 5, 2020
  3. 02/17 hook: scaffolding for git-hook subcommandEmily Shaffer, Dec 5, 2020
  4. 03/17 hook: add list commandEmily Shaffer, Dec 5, 2020
  5. 04/17 hook: include hookdir hook in listEmily Shaffer, Dec 5, 2020
  6. 06/17 hook: implement hookcmd.<name>.skipEmily Shaffer, Dec 5, 2020
  7. 05/17 hook: respect hook.runHookDirEmily Shaffer, Dec 5, 2020
  8. 08/17 hook: add 'run' subcommandEmily Shaffer, Dec 5, 2020
  9. Phillip WoodDec 11, 2020
  10. Emily ShafferDec 15, 2020
  11. 09/17 hook: replace find_hook() with hook_exists()Emily Shaffer, Dec 5, 2020
  12. 07/17 parse-options: parse into strvecEmily Shaffer, Dec 5, 2020
  13. 11/17 run-command: allow stdin for run_processes_parallelEmily Shaffer, Dec 5, 2020
  14. 10/17 hook: support passing stdin to hooksEmily Shaffer, Dec 5, 2020
  15. 12/17 hook: allow parallel hook executionEmily Shaffer, Dec 5, 2020
  16. 14/17 run-command: add stdin callback for parallelizationEmily Shaffer, Dec 5, 2020
  17. 13/17 hook: allow specifying working directory for hooksEmily Shaffer, Dec 5, 2020
  18. 17/17 hooks: allow callers to capture outputEmily Shaffer, Dec 5, 2020
  19. 15/17 hook: provide stdin by string_list or callbackEmily Shaffer, Dec 5, 2020
  20. SZEDER GáborDec 8, 2020
  21. Emily ShafferDec 8, 2020
  22. 16/17 run-command: allow capturing of collated outputEmily Shaffer, Dec 5, 2020
  23. Josh SteadmonDec 16, 2020
  24. Junio C HamanoDec 16, 2020
  25. Emily ShafferDec 16, 2020
  26. Junio C HamanoDec 16, 2020
  27. Emily ShafferDec 18, 2020
  28. Junio C HamanoDec 18, 2020
  29. 00/17 propose config-based hooks (part I)Emily Shaffer, Dec 22, 2020
  30. 01/17 doc: propose hooks managed by the configEmily Shaffer, Dec 22, 2020
  31. Ævar Arnfjörð BjarmasonJan 23, 2021
  32. Emily ShafferJan 29, 2021
  33. Junio C HamanoFeb 1, 2021
  34. Emily ShafferMar 10, 2021
  35. 02/17 hook: scaffolding for git-hook subcommandEmily Shaffer, Dec 22, 2020
  36. 03/17 hook: add list commandEmily Shaffer, Dec 22, 2020
  37. Jonathan TanJan 31, 2021
  38. Emily ShafferFeb 9, 2021
  39. 04/17 hook: include hookdir hook in listEmily Shaffer, Dec 22, 2020
  40. Jonathan TanJan 31, 2021
  41. Emily ShafferFeb 9, 2021
  42. 05/17 hook: respect hook.runHookDirEmily Shaffer, Dec 22, 2020
  43. Jonathan TanJan 31, 2021
  44. Emily ShafferFeb 9, 2021
  45. 06/17 hook: implement hookcmd.<name>.skipEmily Shaffer, Dec 22, 2020
  46. Jonathan TanJan 31, 2021
  47. Emily ShafferFeb 9, 2021
  48. 08/17 hook: add 'run' subcommandEmily Shaffer, Dec 22, 2020
  49. Jonathan TanJan 31, 2021
  50. Emily ShafferFeb 11, 2021
  51. 07/17 parse-options: parse into strvecEmily Shaffer, Dec 22, 2020
  52. 09/17 hook: replace find_hook() with hook_exists()Emily Shaffer, Dec 22, 2020
  53. Jonathan TanJan 31, 2021
  54. Emily ShafferFeb 12, 2021
  55. Emily ShafferFeb 18, 2021
  56. 10/17 hook: support passing stdin to hooksEmily Shaffer, Dec 22, 2020
  57. 12/17 hook: allow parallel hook executionEmily Shaffer, Dec 22, 2020
  58. Jonathan TanFeb 1, 2021
  59. Emily ShafferFeb 22, 2021
  60. 11/17 run-command: allow stdin for run_processes_parallelEmily Shaffer, Dec 22, 2020
  61. Jonathan TanFeb 1, 2021
  62. Emily ShafferFeb 19, 2021
  63. 14/17 run-command: add stdin callback for parallelizationEmily Shaffer, Dec 22, 2020
  64. Jonathan TanFeb 1, 2021
  65. Emily ShafferFeb 22, 2021
  66. Jonathan TanFeb 23, 2021
  67. Emily ShafferMar 10, 2021
  68. 13/17 hook: allow specifying working directory for hooksEmily Shaffer, Dec 22, 2020
  69. 15/17 hook: provide stdin by string_list or callbackEmily Shaffer, Dec 22, 2020
  70. Jonathan TanFeb 1, 2021
  71. Emily ShafferFeb 23, 2021
  72. Jonathan TanFeb 25, 2021
  73. Emily ShafferMar 2, 2021
  74. Jonathan TanMar 2, 2021
  75. 16/17 run-command: allow capturing of collated outputEmily Shaffer, Dec 22, 2020
  76. 17/17 hooks: allow callers to capture outputEmily Shaffer, Dec 22, 2020
  77. Junio C HamanoDec 22, 2020
  78. Emily ShafferDec 28, 2020
  79. Junio C HamanoDec 28, 2020
  80. 18/17 doc: make git-hook.txt point of truthEmily Shaffer, Dec 28, 2020
  81. Emily ShafferDec 28, 2020
  82. Emily ShafferJan 29, 2021
  83. Josh SteadmonFeb 16, 2021
  84. Junio C HamanoFeb 16, 2021
  85. Josh SteadmonFeb 17, 2021
  86. Junio C HamanoFeb 17, 2021
  87. Junio C HamanoFeb 25, 2021
  88. Emily ShafferMar 1, 2021
  89. Junio C HamanoMar 1, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.