From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Karsten Blees" <karsten.blees@gmail.com>,
"Nguyen Thai Ngoc Duy" <pclouds@gmail.com>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
"Stefan Beller" <sbeller@google.com>,
"Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>,
"Christian Couder" <chriscool@tuxfamily.org>
Subject: [PATCH 83/83] builtin/am: use apply api in run_apply()
Date: Sun, 24 Apr 2016 15:39:49 +0200 [thread overview]
Message-ID: <1461505189-16234-4-git-send-email-chriscool@tuxfamily.org> (raw)
In-Reply-To: <1461505189-16234-1-git-send-email-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`
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/am.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 85 insertions(+), 18 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index d003939..85a77d7 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,105 @@ 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;
+
+ 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))
+ 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.1.300.g5fed0c0
next prev parent reply other threads:[~2016-04-24 13:40 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-24 13:39 [PATCH 80/83] run-command: make dup_devnull() non static Christian Couder
2016-04-24 13:39 ` [PATCH 81/83] apply: roll back index in case of error Christian Couder
2016-04-25 16:06 ` Johannes Schindelin
2016-05-02 7:07 ` Johannes Schindelin
2016-05-02 7:18 ` Eric Sunshine
2016-05-03 12:57 ` Christian Couder
2016-04-24 13:39 ` [PATCH 82/83] environment: add set_index_file() Christian Couder
2016-05-03 15:36 ` Junio C Hamano
2016-05-04 11:50 ` Christian Couder
2016-04-24 13:39 ` Christian Couder [this message]
2016-04-25 15:03 ` [PATCH 83/83] builtin/am: use apply api in run_apply() Johannes Schindelin
2016-05-05 10:04 ` Christian Couder
2016-04-25 15:05 ` [PATCH 80/83] run-command: make dup_devnull() non static Johannes Schindelin
2016-05-05 9:50 ` Christian Couder
2016-05-05 20:07 ` Johannes Sixt
2016-05-06 13:56 ` Christian Couder
2016-05-06 15:34 ` Johannes Schindelin
2016-05-07 10:12 ` Christian Couder
2016-05-07 12:13 ` Johannes Schindelin
2016-05-07 13:46 ` Christian Couder
2016-05-08 6:33 ` Johannes Schindelin
2016-05-08 10:15 ` Duy Nguyen
2016-05-09 15:05 ` Johannes Schindelin
2016-05-09 17:40 ` 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=1461505189-16234-4-git-send-email-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=sbeller@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).