From: Eric Wong <e@80x24.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jeff King <peff@peff.net>, Brandon Williams <bmwill@google.com>,
Jonathan Nieder <jrnieder@gmail.com>,
Eric Rannaud <eric.rannaud@gmail.com>,
Git Mailing List <git@vger.kernel.org>,
Johannes Schindelin <johannes.schindelin@gmx.de>,
Jeremy Serror <jeremy.serror@gmail.com>
Subject: [TANGENT] run-command: use vfork instead of fork
Date: Tue, 16 May 2017 19:35:57 +0000 [thread overview]
Message-ID: <20170516193557.GA14257@dcvr> (raw)
In-Reply-To: <CA+55aFwB-MWASj7dZWkXWhgd4gvEfoOhL6Fo7kXeJSm9dht4Jg@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> Also, if people really want to optimize the code that executes an
> external program (whether in shell or directly), I think it might be
> worth it to look at replacing the "fork()" with a "vfork()".
>
> Something like this
>
> - cmd->pid = fork();
> + cmd->pid = (cmd->git_cmd || cmd->env) ? fork() : vfork();
>
> might work (the native git_cmd case needs a real fork, and if we
> change the environment variables we need it too, but the other cases
> look like they might work with vfork()).
>
> Using vfork() can be hugely more efficient, because you don't have the
> extra page table copies and teardown, but also avoid a lot of possible
> copy-on-write faults.
Fwiw, most of the vfork preparation was already done by Brandon
and myself a few weeks ago, and cooking in pu.
I think only the patch below would be needed to enable vfork
(along with any build-time detection)
However, I haven't noticed enough forking in git to make a
difference (but maybe others do). I think it would make a
bigger difference if such changes were made to bash, dash,
make, and perl5.
--------8<------------
Subject: [PATCH] run-command: use vfork instead of fork
To enable vfork, we merely have to avoid modifying memory we
share with the parent, so the guard functions
`child_(error|warn|die)_fn` can now be disabled.
FIXME: still missing autoconf + Makefile portability tweaks.
Signed-off-by: Eric Wong <e@80x24.org>
---
run-command.c | 28 +---------------------------
1 file changed, 1 insertion(+), 27 deletions(-)
diff --git a/run-command.c b/run-command.c
index 9e36151bf9..0292dd94b6 100644
--- a/run-command.c
+++ b/run-command.c
@@ -324,25 +324,6 @@ static void fake_fatal(const char *err, va_list params)
vreportf("fatal: ", err, params);
}
-static void child_error_fn(const char *err, va_list params)
-{
- const char msg[] = "error() should not be called in child\n";
- xwrite(2, msg, sizeof(msg) - 1);
-}
-
-static void child_warn_fn(const char *err, va_list params)
-{
- const char msg[] = "warn() should not be called in child\n";
- xwrite(2, msg, sizeof(msg) - 1);
-}
-
-static void NORETURN child_die_fn(const char *err, va_list params)
-{
- const char msg[] = "die() should not be called in child\n";
- xwrite(2, msg, sizeof(msg) - 1);
- _exit(2);
-}
-
/* this runs in the parent process */
static void child_err_spew(struct child_process *cmd, struct child_err *cerr)
{
@@ -658,17 +639,10 @@ int start_command(struct child_process *cmd)
* never be released in the child process. This means only
* Async-Signal-Safe functions are permitted in the child.
*/
- cmd->pid = fork();
+ cmd->pid = vfork();
failed_errno = errno;
if (!cmd->pid) {
int sig;
- /*
- * Ensure the default die/error/warn routines do not get
- * called, they can take stdio locks and malloc.
- */
- set_die_routine(child_die_fn);
- set_error_routine(child_error_fn);
- set_warn_routine(child_warn_fn);
close(notify_pipe[0]);
set_cloexec(notify_pipe[1]);
--
Also fetchable: git://bogomips.org/git-svn vfork-test
commit 5f88d79182aaabc5ea467d1d29e13e45bd2b99bf
next prev parent reply other threads:[~2017-05-16 19:36 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-15 18:08 git rebase regression: cannot pass a shell expression directly to --exec Eric Rannaud
2017-05-16 3:25 ` Jeff King
2017-05-16 3:37 ` Jeff King
2017-05-16 16:41 ` Jonathan Nieder
2017-05-16 16:47 ` Jeff King
2017-05-16 17:11 ` Eric Rannaud
2017-05-16 17:15 ` Brandon Williams
2017-05-16 17:23 ` Jeff King
2017-05-16 17:30 ` Brandon Williams
2017-05-16 19:14 ` Linus Torvalds
2017-05-16 19:35 ` Eric Wong [this message]
2017-05-16 20:47 ` [TANGENT] run-command: use vfork instead of fork Linus Torvalds
2017-05-16 21:11 ` Brandon Williams
2017-05-16 20:12 ` git rebase regression: cannot pass a shell expression directly to --exec Johannes Schindelin
2017-05-16 20:27 ` Linus Torvalds
2017-05-16 17:37 ` Jonathan Nieder
2017-05-16 3:40 ` Junio C Hamano
2017-05-16 3:53 ` Jeff King
2017-05-16 4:08 ` Jeff King
2017-05-16 16:45 ` Eric Rannaud
2017-05-16 10:46 ` Johannes Schindelin
2017-05-16 10:23 ` Johannes Schindelin
2017-05-16 16:18 ` Jeff King
2017-05-16 16:59 ` Eric Rannaud
2017-05-16 17:14 ` Kevin Daudt
2017-05-16 17:29 ` Eric Rannaud
2017-05-16 17:41 ` Jeff King
2017-05-16 17:21 ` Eric Rannaud
2017-05-16 17:37 ` Jeff King
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=20170516193557.GA14257@dcvr \
--to=e@80x24.org \
--cc=bmwill@google.com \
--cc=eric.rannaud@gmail.com \
--cc=git@vger.kernel.org \
--cc=jeremy.serror@gmail.com \
--cc=johannes.schindelin@gmx.de \
--cc=jrnieder@gmail.com \
--cc=peff@peff.net \
--cc=torvalds@linux-foundation.org \
/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.