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

[PATCH v8 40/41] builtin/am: use apply api in run_apply()

From
Christian Couder <christian.couder@gmail.com>
Date
Jun 27, 2016, 18:24 UTC
Message-ID
<20160627182429.31550-41-chriscool@tuxfamily.org>
In-Reply-To
<20160627182429.31550-1-chriscool@tuxfamily.org>

This replaces run_apply() implementation with a new one that uses the apply api that has been previously prepared in apply.c and apply.h.

This shoud improve performance a lot in certain cases.

As the previous implementation was creating a new `git apply` process to apply each patch, it could be slow on systems like Windows where it is costly to create new processes.

Also the new `git apply` process had to read the index from disk, and when the process was done the calling process discarded its own index and read back from disk the new index that had been created by the `git apply` process.

This could be very inefficient with big repositories that have big index files, especially when the system decided that it was a good idea to run the `git apply` processes on a different processor core.

Also eliminating index reads enables further performance improvements by using:

`git update-index --split-index`

For example here is a benchmark of a multi hundred commit rebase on the Linux kernel on a Debian laptop with SSD:

command: git rebase --onto 1993b17 52bef0c 29dde7c

Vanilla "next" without split index: 1m54.953s Vanilla "next" with split index: 1m22.476s This series on top of "next" without split index: 1m12.034s This series on top of "next" with split index: 0m15.678s

(using branch "next" from mid April 2016.)
Benchmarked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/am.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 73 insertions(+), 18 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 3dfe70b..8647298 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -28,6 +28,7 @@
 #include "rerere.h"
 #include "prompt.h"
 #include "mailinfo.h"
+#include "apply.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -1521,39 +1522,93 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
  */
 static int run_apply(const struct am_state *state, const char *index_file)
 {
-	struct child_process cp = CHILD_PROCESS_INIT;
+	struct argv_array apply_paths = ARGV_ARRAY_INIT;
+	struct argv_array apply_opts = ARGV_ARRAY_INIT;
+	struct apply_state apply_state;
+	int res, opts_left;
+	char *save_index_file;
+	static struct lock_file lock_file;
+
+	struct option am_apply_options[] = {
+		{ OPTION_CALLBACK, 0, "whitespace", &apply_state, N_("action"),
+			N_("detect new or modified lines that have whitespace errors"),
+			0, apply_option_parse_whitespace },
+		{ OPTION_CALLBACK, 0, "ignore-space-change", &apply_state, NULL,
+			N_("ignore changes in whitespace when finding context"),
+			PARSE_OPT_NOARG, apply_option_parse_space_change },
+		{ OPTION_CALLBACK, 0, "ignore-whitespace", &apply_state, NULL,
+			N_("ignore changes in whitespace when finding context"),
+			PARSE_OPT_NOARG, apply_option_parse_space_change },
+		{ OPTION_CALLBACK, 0, "directory", &apply_state, N_("root"),
+			N_("prepend <root> to all filenames"),
+			0, apply_option_parse_directory },
+		{ OPTION_CALLBACK, 0, "exclude", &apply_state, N_("path"),
+			N_("don't apply changes matching the given path"),
+			0, apply_option_parse_exclude },
+		{ OPTION_CALLBACK, 0, "include", &apply_state, N_("path"),
+			N_("apply changes matching the given path"),
+			0, apply_option_parse_include },
+		OPT_INTEGER('C', NULL, &apply_state.p_context,
+				N_("ensure at least <n> lines of context match")),
+		{ OPTION_CALLBACK, 'p', NULL, &apply_state, N_("num"),
+			N_("remove <num> leading slashes from traditional diff paths"),
+			0, apply_option_parse_p },
+		OPT_BOOL(0, "reject", &apply_state.apply_with_reject,
+			N_("leave the rejected hunks in corresponding *.rej files")),
+		OPT_END()
+	};
 
-	cp.git_cmd = 1;
+	if (index_file) {
+		save_index_file = get_index_file();
+		set_index_file((char *)index_file);
+	}
+
+	if (init_apply_state(&apply_state, NULL, &lock_file))
+		die("init_apply_state() failed");
+
+	argv_array_push(&apply_opts, "apply");
+	argv_array_pushv(&apply_opts, state->git_apply_opts.argv);
+
+	opts_left = parse_options(apply_opts.argc, apply_opts.argv,
+				  NULL, am_apply_options, NULL, 0);
+
+	if (opts_left != 0)
+		die("unknown option passed thru to git apply");
 
 	if (index_file)
-		argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file);
+		apply_state.cached = 1;
+	else
+		apply_state.check_index = 1;
 
 	/*
 	 * If we are allowed to fall back on 3-way merge, don't give false
 	 * errors during the initial attempt.
 	 */
-	if (state->threeway && !index_file) {
-		cp.no_stdout = 1;
-		cp.no_stderr = 1;
-	}
+	if (state->threeway && !index_file)
+		apply_state.be_silent = 1;
 
-	argv_array_push(&cp.args, "apply");
+	if (check_apply_state(&apply_state, 0))
+		die("check_apply_state() failed");
 
-	argv_array_pushv(&cp.args, state->git_apply_opts.argv);
+	argv_array_push(&apply_paths, am_path(state, "patch"));
+
+	res = apply_all_patches(&apply_state, apply_paths.argc, apply_paths.argv, 0);
 
 	if (index_file)
-		argv_array_push(&cp.args, "--cached");
-	else
-		argv_array_push(&cp.args, "--index");
+		set_index_file(save_index_file);
 
-	argv_array_push(&cp.args, am_path(state, "patch"));
+	argv_array_clear(&apply_paths);
+	argv_array_clear(&apply_opts);
+	clear_apply_state(&apply_state);
 
-	if (run_command(&cp))
-		return -1;
+	if (res)
+		return res;
 
-	/* Reload index as git-apply will have modified it. */
-	discard_cache();
-	read_cache_from(index_file ? index_file : get_index_file());
+	if (index_file) {
+		/* Reload index as apply_all_patches() will have modified it. */
+		discard_cache();
+		read_cache_from(index_file);
+	}
 
 	return 0;
 }

-- 
2.9.0.172.gfb57a78
Previous: Christian CouderNext: Junio C Hamano
Message 33 of 59 in “libify apply and use lib in am, part 2”
  1. 00/41 libify apply and use lib in am, part 2Christian Couder, Jun 27, 2016
  2. 03/41 builtin/apply: make apply_patch() return -1 or -128 instead of die()ingChristian Couder, Jun 27, 2016
  3. 06/41 builtin/apply: make parse_chunk() return a negative integer on errorChristian Couder, Jun 27, 2016
  4. 07/41 builtin/apply: make parse_single_patch() return -1 on errorChristian Couder, Jun 27, 2016
  5. 01/41 apply: make some names more specificChristian Couder, Jun 27, 2016
  6. 12/41 builtin/apply: make check_apply_state() return -1 instead of die()ingChristian Couder, Jun 27, 2016
  7. 19/41 builtin/apply: make build_fake_ancestor() return -1 on errorChristian Couder, Jun 27, 2016
  8. 14/41 builtin/apply: make apply_all_patches() return 128 or 1 on errorChristian Couder, Jun 27, 2016
  9. 16/41 builtin/apply: make gitdiff_*() return 1 at end of headerChristian Couder, Jun 27, 2016
  10. 28/41 builtin/apply: rename option parsing functionsChristian Couder, Jun 27, 2016
  11. 29/41 apply: rename and move opt constants to apply.hChristian Couder, Jun 27, 2016
  12. 23/41 builtin/apply: make create_file() return -1 on errorChristian Couder, Jun 27, 2016
  13. 24/41 builtin/apply: make write_out_one_result() return -1 on errorChristian Couder, Jun 27, 2016
  14. 21/41 builtin/apply: make add_conflicted_stages_file() return -1 on errorChristian Couder, Jun 27, 2016
  15. 20/41 builtin/apply: make remove_file() return -1 on errorChristian Couder, Jun 27, 2016
  16. 26/41 builtin/apply: make try_create_file() return -1 on errorChristian Couder, Jun 27, 2016
  17. 32/41 environment: add set_index_file()Christian Couder, Jun 27, 2016
  18. Junio C HamanoJul 26, 2016
  19. Duy NguyenJul 27, 2016
  20. Christian CouderJul 29, 2016
  21. Duy NguyenJul 29, 2016
  22. Christian CouderJul 29, 2016
  23. Duy NguyenJul 29, 2016
  24. Junio C HamanoJul 29, 2016
  25. 31/41 apply: make some parsing functions static againChristian Couder, Jun 27, 2016
  26. 38/41 usage: add get_error_routine() and get_warn_routine()Christian Couder, Jun 27, 2016
  27. 35/41 apply: make 'be_silent' incompatible with 'apply_verbosely'Christian Couder, Jun 27, 2016
  28. Junio C HamanoJul 26, 2016
  29. 39/41 apply: change error_routine when be_silent is setChristian Couder, Jun 27, 2016
  30. 33/41 write_or_die: use warning() instead of fprintf(stderr, ...)Christian Couder, Jun 27, 2016
  31. Junio C HamanoJun 28, 2016
  32. Christian CouderJun 30, 2016
  33. 40/41 builtin/am: use apply api in run_apply()Christian Couder, Jun 27, 2016
  34. Junio C HamanoJul 26, 2016
  35. 34/41 apply: add 'be_silent' variable to 'struct apply_state'Christian Couder, Jun 27, 2016
  36. Junio C HamanoJul 26, 2016
  37. 41/41 apply: use error_errno() where possibleChristian Couder, Jun 27, 2016
  38. 37/41 usage: add set_warn_routine()Christian Couder, Jun 27, 2016
  39. 36/41 apply: don't print on stdout when be_silent is setChristian Couder, Jun 27, 2016
  40. Junio C HamanoJul 26, 2016
  41. 22/41 builtin/apply: make add_index_file() return -1 on errorChristian Couder, Jun 27, 2016
  42. 13/41 builtin/apply: move check_apply_state() to apply.cChristian Couder, Jun 27, 2016
  43. 27/41 builtin/apply: make create_one_file() return -1 on errorChristian Couder, Jun 27, 2016
  44. 25/41 builtin/apply: make write_out_results() return -1 on errorChristian Couder, Jun 27, 2016
  45. 15/41 builtin/apply: make parse_traditional_patch() return -1 on errorChristian Couder, Jun 27, 2016
  46. 11/41 apply: make init_apply_state() return -1 instead of exit()ingChristian Couder, Jun 27, 2016
  47. 02/41 apply: move 'struct apply_state' to apply.hChristian Couder, Jun 27, 2016
  48. 04/41 builtin/apply: read_patch_file() return -1 instead of die()ingChristian Couder, Jun 27, 2016
  49. 08/41 builtin/apply: make parse_whitespace_option() return -1 instead of die()ingChristian Couder, Jun 27, 2016
  50. 09/41 builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ingChristian Couder, Jun 27, 2016
  51. 05/41 builtin/apply: make find_header() return -128 instead of die()ingChristian Couder, Jun 27, 2016
  52. 18/41 builtin/apply: change die_on_unsafe_path() to check_unsafe_path()Christian Couder, Jun 27, 2016
  53. 17/41 builtin/apply: make gitdiff_*() return -1 on errorChristian Couder, Jun 27, 2016
  54. 10/41 builtin/apply: move init_apply_state() to apply.cChristian Couder, Jun 27, 2016
  55. Christian CouderJun 27, 2016
  56. Junio C HamanoJul 26, 2016
  57. Christian CouderJul 27, 2016
  58. Junio C HamanoJul 27, 2016
  59. Christian CouderJul 30, 2016

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.