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

[PATCH v2 84/94] builtin/am: use apply api in run_apply()

From
Christian Couder <christian.couder@gmail.com>
Date
May 11, 2016, 13:17 UTC
Message-ID
<20160511131745.2914-85-chriscool@tuxfamily.org>
In-Reply-To
<20160511131745.2914-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 | 104 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 86 insertions(+), 18 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index d003939..cc66a48 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.
@@ -1522,39 +1523,106 @@ 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;
-
-	cp.git_cmd = 1;
-
-	if (index_file)
-		argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file);
+	struct argv_array apply_paths = ARGV_ARRAY_INIT;
+	struct argv_array apply_opts = ARGV_ARRAY_INIT;
+	struct apply_state apply_state;
+	int save_stdout_fd, save_stderr_fd;
+	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()
+	};
 
 	/*
 	 * 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;
+		save_stdout_fd = dup(1);
+		dup_devnull(1);
+		save_stderr_fd = dup(2);
+		dup_devnull(2);
 	}
 
-	argv_array_push(&cp.args, "apply");
+	if (index_file) {
+		save_index_file = get_index_file();
+		set_index_file((char *)index_file);
+	}
 
-	argv_array_pushv(&cp.args, state->git_apply_opts.argv);
+	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_push(&cp.args, "--cached");
+		apply_state.cached = 1;
 	else
-		argv_array_push(&cp.args, "--index");
+		apply_state.check_index = 1;
 
-	argv_array_push(&cp.args, am_path(state, "patch"));
+	if (check_apply_state(&apply_state, 0))
+		die("check_apply_state() failed");
 
-	if (run_command(&cp))
-		return -1;
+	argv_array_push(&apply_paths, am_path(state, "patch"));
 
-	/* Reload index as git-apply will have modified it. */
-	discard_cache();
-	read_cache_from(index_file ? index_file : get_index_file());
+	res = apply_all_patches(&apply_state, apply_paths.argc, apply_paths.argv, 0);
+
+	/* Restore stdout and stderr */
+	if (state->threeway && !index_file) {
+		dup2(save_stdout_fd, 1);
+		close(save_stdout_fd);
+		dup2(save_stderr_fd, 2);
+		close(save_stderr_fd);
+	}
+
+	if (index_file)
+		set_index_file(save_index_file);
+
+	argv_array_clear(&apply_paths);
+	argv_array_clear(&apply_opts);
+
+	if (res)
+		return res;
+
+	if (index_file) {
+		/* Reload index as apply_all_patches() will have modified it. */
+		discard_cache();
+		read_cache_from(index_file);
+	}
 
 	return 0;
 }
-- 
2.8.2.490.g3dabe57
Previous: Christian CouderNext: Christian Couder
Message 119 of 148 in “libify apply and use lib in am”
  1. 00/94 libify apply and use lib in amChristian Couder, May 11, 2016
  2. 01/94 builtin/apply: make gitdiff_verify_name() return voidChristian Couder, May 11, 2016
  3. Junio C HamanoMay 12, 2016
  4. 02/94 builtin/apply: avoid parameter shadowing 'p_value' globalChristian Couder, May 11, 2016
  5. Junio C HamanoMay 12, 2016
  6. 03/94 builtin/apply: avoid parameter shadowing 'linenr' globalChristian Couder, May 11, 2016
  7. Junio C HamanoMay 12, 2016
  8. 04/94 builtin/apply: avoid local variable shadowing 'len' parameterChristian Couder, May 11, 2016
  9. 05/94 builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()Christian Couder, May 11, 2016
  10. Junio C HamanoMay 12, 2016
  11. 06/94 builtin/apply: move 'options' variable into cmd_apply()Christian Couder, May 11, 2016
  12. 07/94 builtin/apply: move 'read_stdin' global into cmd_apply()Christian Couder, May 11, 2016
  13. 08/94 builtin/apply: introduce 'struct apply_state' to start libifyingChristian Couder, May 11, 2016
  14. 09/94 builtin/apply: move 'state' init into init_apply_state()Christian Couder, May 11, 2016
  15. Junio C HamanoMay 12, 2016
  16. 10/94 builtin/apply: move 'unidiff_zero' global into 'struct apply_state'Christian Couder, May 11, 2016
  17. Junio C HamanoMay 12, 2016
  18. Christian CouderMay 12, 2016
  19. 11/94 builtin/apply: move 'check' global into 'struct apply_state'Christian Couder, May 11, 2016
  20. 12/94 builtin/apply: move 'check_index' global into 'struct apply_state'Christian Couder, May 11, 2016
  21. 13/94 builtin/apply: move 'apply_in_reverse' global into 'struct apply_state'Christian Couder, May 11, 2016
  22. 14/94 builtin/apply: move 'apply_with_reject' global into 'struct apply_state'Christian Couder, May 11, 2016
  23. 15/94 builtin/apply: move 'apply_verbosely' global into 'struct apply_state'Christian Couder, May 11, 2016
  24. 16/94 builtin/apply: move 'update_index' global into 'struct apply_state'Christian Couder, May 11, 2016
  25. Junio C HamanoMay 12, 2016
  26. 17/94 builtin/apply: move 'allow_overlap' global into 'struct apply_state'Christian Couder, May 11, 2016
  27. 18/94 builtin/apply: move 'cached' global into 'struct apply_state'Christian Couder, May 11, 2016
  28. Junio C HamanoMay 12, 2016
  29. 19/94 builtin/apply: move 'diffstat' global into 'struct apply_state'Christian Couder, May 11, 2016
  30. 20/94 builtin/apply: move 'numstat' global into 'struct apply_state'Christian Couder, May 11, 2016
  31. 21/94 builtin/apply: move 'summary' global into 'struct apply_state'Christian Couder, May 11, 2016
  32. 22/94 builtin/apply: move 'threeway' global into 'struct apply_state'Christian Couder, May 11, 2016
  33. Junio C HamanoMay 12, 2016
  34. Christian CouderMay 12, 2016
  35. Junio C HamanoMay 12, 2016
  36. 23/94 builtin/apply: move 'no_add' global into 'struct apply_state'Christian Couder, May 11, 2016
  37. 24/94 builtin/apply: move 'unsafe_paths' global into 'struct apply_state'Christian Couder, May 11, 2016
  38. 25/94 builtin/apply: move 'line_termination' global into 'struct apply_state'Christian Couder, May 11, 2016
  39. 26/94 builtin/apply: move 'fake_ancestor' global into 'struct apply_state'Christian Couder, May 11, 2016
  40. 27/94 builtin/apply: move 'p_context' global into 'struct apply_state'Christian Couder, May 11, 2016
  41. 28/94 builtin/apply: move 'apply' global into 'struct apply_state'Christian Couder, May 11, 2016
  42. 29/94 builtin/apply: move 'patch_input_file' global into 'struct apply_state'Christian Couder, May 11, 2016
  43. 30/94 builtin/apply: move 'limit_by_name' global into 'struct apply_state'Christian Couder, May 11, 2016
  44. 31/94 builtin/apply: move 'has_include' global into 'struct apply_state'Christian Couder, May 11, 2016
  45. 32/94 builtin/apply: move 'p_value' global into 'struct apply_state'Christian Couder, May 11, 2016
  46. 33/94 builtin/apply: move 'p_value_known' global into 'struct apply_state'Christian Couder, May 11, 2016
  47. Junio C HamanoMay 12, 2016
  48. 34/94 builtin/apply: move 'root' global into 'struct apply_state'Christian Couder, May 11, 2016
  49. 35/94 builtin/apply: move 'whitespace_error' global into 'struct apply_state'Christian Couder, May 11, 2016
  50. 36/94 builtin/apply: move 'whitespace_option' into 'struct apply_state'Christian Couder, May 11, 2016
  51. 37/94 builtin/apply: remove whitespace_option arg from set_default_whitespace_mode()Christian Couder, May 11, 2016
  52. 38/94 builtin/apply: move 'squelch_whitespace_errors' into 'struct apply_state'Christian Couder, May 11, 2016
  53. 39/94 builtin/apply: move 'applied_after_fixing_ws' into 'struct apply_state'Christian Couder, May 11, 2016
  54. 40/94 builtin/apply: move 'ws_error_action' into 'struct apply_state'Christian Couder, May 11, 2016
  55. Junio C HamanoMay 12, 2016
  56. 41/94 builtin/apply: move 'ws_ignore_action' into 'struct apply_state'Christian Couder, May 11, 2016
  57. 42/94 builtin/apply: move 'max_change' and 'max_len' into 'struct apply_state'Christian Couder, May 11, 2016
  58. 43/94 builtin/apply: move 'state_linenr' global into 'struct apply_state'Christian Couder, May 11, 2016
  59. 44/94 builtin/apply: move 'fn_table' global into 'struct apply_state'Christian Couder, May 11, 2016
  60. 45/94 builtin/apply: move 'symlink_changes' global into 'struct apply_state'Christian Couder, May 11, 2016
  61. 46/94 builtin/apply: move 'state' check into check_apply_state()Christian Couder, May 11, 2016
  62. 47/94 builtin/apply: move applying patches into apply_all_patches()Christian Couder, May 11, 2016
  63. 48/94 builtin/apply: rename 'prefix_' parameter to 'prefix'Christian Couder, May 11, 2016
  64. Junio C HamanoMay 12, 2016
  65. Junio C HamanoMay 12, 2016
  66. Christian CouderMay 13, 2016
  67. Junio C HamanoMay 14, 2016
  68. Christian CouderMay 24, 2016
  69. Christian CouderMay 13, 2016
  70. Christian CouderMay 24, 2016
  71. 49/94 builtin/apply: move 'lock_file' global into 'struct apply_state'Christian Couder, May 11, 2016
  72. 50/94 builtin/apply: move 'newfd' global into 'struct apply_state'Christian Couder, May 11, 2016
  73. 51/94 builtin/apply: make apply_patch() return -1 instead of die()ingChristian Couder, May 11, 2016
  74. 52/94 builtin/apply: read_patch_file() return -1 instead of die()ingChristian Couder, May 11, 2016
  75. Eric SunshineMay 16, 2016
  76. Christian CouderMay 16, 2016
  77. 53/94 builtin/apply: make find_header() return -1 instead of die()ingChristian Couder, May 11, 2016
  78. 54/94 builtin/apply: make parse_chunk() return a negative integer on errorChristian Couder, May 11, 2016
  79. Eric SunshineMay 16, 2016
  80. Christian CouderMay 16, 2016
  81. Christian CouderJun 8, 2016
  82. 55/94 builtin/apply: make parse_single_patch() return -1 on errorChristian Couder, May 11, 2016
  83. 56/94 apply: move 'struct apply_state' to apply.hChristian Couder, May 11, 2016
  84. Eric SunshineMay 16, 2016
  85. Junio C HamanoMay 16, 2016
  86. Christian CouderJun 8, 2016
  87. 57/94 builtin/apply: make parse_whitespace_option() return -1 instead of die()ingChristian Couder, May 11, 2016
  88. 58/94 builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ingChristian Couder, May 11, 2016
  89. 59/94 builtin/apply: move init_apply_state() to apply.cChristian Couder, May 11, 2016
  90. Eric SunshineMay 16, 2016
  91. 60/94 apply: make init_apply_state() return -1 instead of exit()ingChristian Couder, May 11, 2016
  92. Eric SunshineMay 16, 2016
  93. 61/94 builtin/apply: make check_apply_state() return -1 instead of die()ingChristian Couder, May 11, 2016
  94. 62/94 builtin/apply: move check_apply_state() to apply.cChristian Couder, May 11, 2016
  95. 63/94 builtin/apply: make apply_all_patches() return -1 on errorChristian Couder, May 11, 2016
  96. Eric SunshineMay 16, 2016
  97. Christian CouderJun 8, 2016
  98. Eric SunshineJun 8, 2016
  99. Christian CouderJun 9, 2016
  100. 64/94 builtin/apply: make parse_traditional_patch() return -1 on errorChristian Couder, May 11, 2016
  101. 65/94 builtin/apply: make gitdiff_*() return 1 at end of headerChristian Couder, May 11, 2016
  102. 66/94 builtin/apply: make gitdiff_*() return -1 on errorChristian Couder, May 11, 2016
  103. 67/94 builtin/apply: change die_on_unsafe_path() to check_unsafe_path()Christian Couder, May 11, 2016
  104. 68/94 builtin/apply: make build_fake_ancestor() return -1 on errorChristian Couder, May 11, 2016
  105. 69/94 builtin/apply: make remove_file() return -1 on errorChristian Couder, May 11, 2016
  106. 70/94 builtin/apply: make add_conflicted_stages_file() return -1 on errorChristian Couder, May 11, 2016
  107. 71/94 builtin/apply: make add_index_file() return -1 on errorChristian Couder, May 11, 2016
  108. 72/94 builtin/apply: make create_file() return -1 on errorChristian Couder, May 11, 2016
  109. 73/94 builtin/apply: make write_out_one_result() return -1 on errorChristian Couder, May 11, 2016
  110. 74/94 builtin/apply: make write_out_results() return -1 on errorChristian Couder, May 11, 2016
  111. 75/94 builtin/apply: make try_create_file() return -1 on errorChristian Couder, May 11, 2016
  112. 76/94 builtin/apply: make create_one_file() return -1 on errorChristian Couder, May 11, 2016
  113. 77/94 builtin/apply: rename option parsing functionsChristian Couder, May 11, 2016
  114. 78/94 apply: rename and move opt constants to apply.hChristian Couder, May 11, 2016
  115. 80/94 apply: make some parsing functions static againChristian Couder, May 11, 2016
  116. 81/94 run-command: make dup_devnull() non staticChristian Couder, May 11, 2016
  117. 82/94 apply: roll back index lock file in case of errorChristian Couder, May 11, 2016
  118. 83/94 environment: add set_index_file()Christian Couder, May 11, 2016
  119. 84/94 builtin/am: use apply api in run_apply()Christian Couder, May 11, 2016
  120. 85/94 write_or_die: use warning() instead of fprintf(stderr, ...)Christian Couder, May 11, 2016
  121. 86/94 apply: add 'be_silent' variable to 'struct apply_state'Christian Couder, May 11, 2016
  122. 87/94 apply: make 'be_silent' incomatible with 'apply_verbosely'Christian Couder, May 11, 2016
  123. 88/94 apply: don't print on stdout when be_silent is setChristian Couder, May 11, 2016
  124. 89/94 usage: add set_warn_routine()Christian Couder, May 11, 2016
  125. 90/94 usage: add get_error_routine() and get_warn_routine()Christian Couder, May 11, 2016
  126. 91/94 apply: change error_routine when be_silent is setChristian Couder, May 11, 2016
  127. 92/94 am: use be_silent in 'struct apply_state' to shut up applying patchesChristian Couder, May 11, 2016
  128. 93/94 run-command: make dup_devnull() static againChristian Couder, May 11, 2016
  129. 94/94 builtin/apply: add a cli option for be_silentChristian Couder, May 11, 2016
  130. Johannes SixtMay 12, 2016
  131. Christian CouderMay 12, 2016
  132. Johannes SixtJun 9, 2016
  133. Christian CouderJun 10, 2016
  134. Johannes SchindelinJun 10, 2016
  135. Christian CouderJun 10, 2016
  136. Johannes SchindelinJun 10, 2016
  137. Johannes SixtJun 10, 2016
  138. Christian CouderJun 10, 2016
  139. Johannes SchindelinJun 11, 2016
  140. Junio C HamanoMay 12, 2016
  141. Christian CouderMay 12, 2016
  142. Johannes SchindelinMay 13, 2016
  143. Christian CouderMay 13, 2016
  144. Johannes SchindelinMay 14, 2016
  145. Christian CouderMay 14, 2016
  146. Junio C HamanoMay 14, 2016
  147. Christian CouderMay 14, 2016
  148. Junio C HamanoMay 15, 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.