All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Nguyen Thai Ngoc Duy" <pclouds@gmail.com>,
	"Stefan Beller" <sbeller@google.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ramsay Jones" <ramsay@ramsayjones.plus.com>,
	"Jeff King" <peff@peff.net>,
	"Karsten Blees" <karsten.blees@gmail.com>,
	"Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: [PATCH v2 84/94] builtin/am: use apply api in run_apply()
Date: Wed, 11 May 2016 15:17:35 +0200	[thread overview]
Message-ID: <20160511131745.2914-85-chriscool@tuxfamily.org> (raw)
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

  parent reply	other threads:[~2016-05-11 13:21 UTC|newest]

Thread overview: 148+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-11 13:16 [PATCH v2 00/94] libify apply and use lib in am Christian Couder
2016-05-11 13:16 ` [PATCH v2 01/94] builtin/apply: make gitdiff_verify_name() return void Christian Couder
2016-05-12 19:06   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 02/94] builtin/apply: avoid parameter shadowing 'p_value' global Christian Couder
2016-05-12 19:09   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 03/94] builtin/apply: avoid parameter shadowing 'linenr' global Christian Couder
2016-05-12 19:11   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 04/94] builtin/apply: avoid local variable shadowing 'len' parameter Christian Couder
2016-05-11 13:16 ` [PATCH v2 05/94] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment() Christian Couder
2016-05-12 19:20   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 06/94] builtin/apply: move 'options' variable into cmd_apply() Christian Couder
2016-05-11 13:16 ` [PATCH v2 07/94] builtin/apply: move 'read_stdin' global " Christian Couder
2016-05-11 13:16 ` [PATCH v2 08/94] builtin/apply: introduce 'struct apply_state' to start libifying Christian Couder
2016-05-11 13:16 ` [PATCH v2 09/94] builtin/apply: move 'state' init into init_apply_state() Christian Couder
2016-05-12 19:25   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 10/94] builtin/apply: move 'unidiff_zero' global into 'struct apply_state' Christian Couder
2016-05-12 19:28   ` Junio C Hamano
2016-05-12 20:18     ` Christian Couder
2016-05-11 13:16 ` [PATCH v2 11/94] builtin/apply: move 'check' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 12/94] builtin/apply: move 'check_index' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 13/94] builtin/apply: move 'apply_in_reverse' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 14/94] builtin/apply: move 'apply_with_reject' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 15/94] builtin/apply: move 'apply_verbosely' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 16/94] builtin/apply: move 'update_index' " Christian Couder
2016-05-12 19:31   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 17/94] builtin/apply: move 'allow_overlap' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 18/94] builtin/apply: move 'cached' " Christian Couder
2016-05-12 19:33   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 19/94] builtin/apply: move 'diffstat' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 20/94] builtin/apply: move 'numstat' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 21/94] builtin/apply: move 'summary' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 22/94] builtin/apply: move 'threeway' " Christian Couder
2016-05-12 19:41   ` Junio C Hamano
2016-05-12 20:26     ` Christian Couder
2016-05-12 21:21       ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 23/94] builtin/apply: move 'no_add' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 24/94] builtin/apply: move 'unsafe_paths' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 25/94] builtin/apply: move 'line_termination' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 26/94] builtin/apply: move 'fake_ancestor' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 27/94] builtin/apply: move 'p_context' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 28/94] builtin/apply: move 'apply' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 29/94] builtin/apply: move 'patch_input_file' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 30/94] builtin/apply: move 'limit_by_name' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 31/94] builtin/apply: move 'has_include' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 32/94] builtin/apply: move 'p_value' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 33/94] builtin/apply: move 'p_value_known' " Christian Couder
2016-05-12 19:43   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 34/94] builtin/apply: move 'root' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 35/94] builtin/apply: move 'whitespace_error' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 36/94] builtin/apply: move 'whitespace_option' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 37/94] builtin/apply: remove whitespace_option arg from set_default_whitespace_mode() Christian Couder
2016-05-11 13:16 ` [PATCH v2 38/94] builtin/apply: move 'squelch_whitespace_errors' into 'struct apply_state' Christian Couder
2016-05-11 13:16 ` [PATCH v2 39/94] builtin/apply: move 'applied_after_fixing_ws' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 40/94] builtin/apply: move 'ws_error_action' " Christian Couder
2016-05-12 19:48   ` Junio C Hamano
2016-05-11 13:16 ` [PATCH v2 41/94] builtin/apply: move 'ws_ignore_action' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 42/94] builtin/apply: move 'max_change' and 'max_len' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 43/94] builtin/apply: move 'state_linenr' global " Christian Couder
2016-05-11 13:16 ` [PATCH v2 44/94] builtin/apply: move 'fn_table' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 45/94] builtin/apply: move 'symlink_changes' " Christian Couder
2016-05-11 13:16 ` [PATCH v2 46/94] builtin/apply: move 'state' check into check_apply_state() Christian Couder
2016-05-11 13:16 ` [PATCH v2 47/94] builtin/apply: move applying patches into apply_all_patches() Christian Couder
2016-05-11 13:16 ` [PATCH v2 48/94] builtin/apply: rename 'prefix_' parameter to 'prefix' Christian Couder
2016-05-12 19:56   ` Junio C Hamano
2016-05-12 20:43     ` Junio C Hamano
2016-05-13 19:45       ` Christian Couder
2016-05-14 18:27         ` Junio C Hamano
2016-05-24  8:24           ` Christian Couder
2016-05-13 19:42     ` Christian Couder
2016-05-24  8:15       ` Christian Couder
2016-05-11 13:17 ` [PATCH v2 49/94] builtin/apply: move 'lock_file' global into 'struct apply_state' Christian Couder
2016-05-11 13:17 ` [PATCH v2 50/94] builtin/apply: move 'newfd' " Christian Couder
2016-05-11 13:17 ` [PATCH v2 51/94] builtin/apply: make apply_patch() return -1 instead of die()ing Christian Couder
2016-05-11 13:17 ` [PATCH v2 52/94] builtin/apply: read_patch_file() " Christian Couder
2016-05-16  1:56   ` Eric Sunshine
2016-05-16 17:19     ` Christian Couder
2016-05-11 13:17 ` [PATCH v2 53/94] builtin/apply: make find_header() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 54/94] builtin/apply: make parse_chunk() return a negative integer on error Christian Couder
2016-05-16  3:04   ` Eric Sunshine
2016-05-16 18:19     ` Christian Couder
2016-06-08 15:14     ` Christian Couder
2016-05-11 13:17 ` [PATCH v2 55/94] builtin/apply: make parse_single_patch() return -1 " Christian Couder
2016-05-11 13:17 ` [PATCH v2 56/94] apply: move 'struct apply_state' to apply.h Christian Couder
2016-05-16  3:10   ` Eric Sunshine
2016-05-16 16:03     ` Junio C Hamano
2016-06-08 15:25       ` Christian Couder
2016-05-11 13:17 ` [PATCH v2 57/94] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing Christian Couder
2016-05-11 13:17 ` [PATCH v2 58/94] builtin/apply: make parse_ignorewhitespace_option() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 59/94] builtin/apply: move init_apply_state() to apply.c Christian Couder
2016-05-16  3:16   ` Eric Sunshine
2016-05-11 13:17 ` [PATCH v2 60/94] apply: make init_apply_state() return -1 instead of exit()ing Christian Couder
2016-05-16  3:37   ` Eric Sunshine
2016-05-11 13:17 ` [PATCH v2 61/94] builtin/apply: make check_apply_state() return -1 instead of die()ing Christian Couder
2016-05-11 13:17 ` [PATCH v2 62/94] builtin/apply: move check_apply_state() to apply.c Christian Couder
2016-05-11 13:17 ` [PATCH v2 63/94] builtin/apply: make apply_all_patches() return -1 on error Christian Couder
2016-05-16  3:44   ` Eric Sunshine
2016-06-08 16:37     ` Christian Couder
2016-06-08 17:44       ` Eric Sunshine
2016-06-09 22:01         ` Christian Couder
2016-05-11 13:17 ` [PATCH v2 64/94] builtin/apply: make parse_traditional_patch() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 65/94] builtin/apply: make gitdiff_*() return 1 at end of header Christian Couder
2016-05-11 13:17 ` [PATCH v2 66/94] builtin/apply: make gitdiff_*() return -1 on error Christian Couder
2016-05-11 13:17 ` [PATCH v2 67/94] builtin/apply: change die_on_unsafe_path() to check_unsafe_path() Christian Couder
2016-05-11 13:17 ` [PATCH v2 68/94] builtin/apply: make build_fake_ancestor() return -1 on error Christian Couder
2016-05-11 13:17 ` [PATCH v2 69/94] builtin/apply: make remove_file() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 70/94] builtin/apply: make add_conflicted_stages_file() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 71/94] builtin/apply: make add_index_file() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 72/94] builtin/apply: make create_file() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 73/94] builtin/apply: make write_out_one_result() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 74/94] builtin/apply: make write_out_results() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 75/94] builtin/apply: make try_create_file() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 76/94] builtin/apply: make create_one_file() " Christian Couder
2016-05-11 13:17 ` [PATCH v2 77/94] builtin/apply: rename option parsing functions Christian Couder
2016-05-11 13:17 ` [PATCH v2 78/94] apply: rename and move opt constants to apply.h Christian Couder
2016-05-11 13:17 ` [PATCH v2 80/94] apply: make some parsing functions static again Christian Couder
2016-05-11 13:17 ` [PATCH v2 81/94] run-command: make dup_devnull() non static Christian Couder
2016-05-11 13:17 ` [PATCH v2 82/94] apply: roll back index lock file in case of error Christian Couder
2016-05-11 13:17 ` [PATCH v2 83/94] environment: add set_index_file() Christian Couder
2016-05-11 13:17 ` Christian Couder [this message]
2016-05-11 13:17 ` [PATCH v2 85/94] write_or_die: use warning() instead of fprintf(stderr, ...) Christian Couder
2016-05-11 13:17 ` [PATCH v2 86/94] apply: add 'be_silent' variable to 'struct apply_state' Christian Couder
2016-05-11 13:17 ` [PATCH v2 87/94] apply: make 'be_silent' incomatible with 'apply_verbosely' Christian Couder
2016-05-11 13:17 ` [PATCH v2 88/94] apply: don't print on stdout when be_silent is set Christian Couder
2016-05-11 13:17 ` [PATCH v2 89/94] usage: add set_warn_routine() Christian Couder
2016-05-11 13:17 ` [PATCH v2 90/94] usage: add get_error_routine() and get_warn_routine() Christian Couder
2016-05-11 13:17 ` [PATCH v2 91/94] apply: change error_routine when be_silent is set Christian Couder
2016-05-11 13:17 ` [PATCH v2 92/94] am: use be_silent in 'struct apply_state' to shut up applying patches Christian Couder
2016-05-11 13:17 ` [PATCH v2 93/94] run-command: make dup_devnull() static again Christian Couder
2016-05-11 13:17 ` [PATCH v2 94/94] builtin/apply: add a cli option for be_silent Christian Couder
2016-05-12 17:06 ` [PATCH v2 00/94] libify apply and use lib in am Johannes Sixt
2016-05-12 18:02   ` Christian Couder
2016-06-09 21:10     ` Johannes Sixt
2016-06-10  6:40       ` Christian Couder
2016-06-10  7:01       ` Johannes Schindelin
2016-06-10  8:59         ` Christian Couder
2016-06-10 11:11           ` Johannes Schindelin
2016-06-10 17:04             ` Johannes Sixt
2016-06-10 20:31               ` Christian Couder
2016-06-11  7:02               ` Johannes Schindelin
2016-05-12 19:04   ` Junio C Hamano
2016-05-12 20:05     ` Christian Couder
2016-05-13  6:32 ` Johannes Schindelin
2016-05-13 18:49   ` Christian Couder
2016-05-14  6:26     ` Johannes Schindelin
2016-05-14  9:19       ` Christian Couder
2016-05-14 18:31         ` Junio C Hamano
2016-05-14 19:37           ` Christian Couder
2016-05-15 18:30             ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160511131745.2914-85-chriscool@tuxfamily.org \
    --to=christian.couder@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karsten.blees@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sbeller@google.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.