Re: Should we discuss Windows-related changes on git@vger.kernel.org?

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Jul 12, 2008, 01:07 UTC
Message-ID: <alpine.LFD.1.10.0807111750200.2875@woody.linux-foundation.org>
In-Reply-To: <alpine.LFD.1.10.0807111653500.2875@woody.linux-foundation.org>

On Fri, 11 Jul 2008, Linus Torvalds wrote:

> 
> If it's something that should be merged, and if it concerns code that I'm 
> interested in, I want to know about it. It's that simple.

Btw, an example of where I think we need to look at both windows and unix behavior and not try to make them two different camps is in that "start_command()" thing.

It was changed to have a totally separate __MINGW32__ part, but the thing is, the unix side could really be improved - and actually made more like the MINGW32 code at the same time!

For example, on many systems it is rather noticeably faster to use "vfork+execve" than it is to do "fork+execve", because you avoid a whole "duplicate and tear down page tables" sequence. So the UNIX code would actually be better off using "vfork()" instead of "fork()" there.

But it can't right now - because if "cmd->env" changes the environment, it would change it both in the caller and in the result.

It turns out that Windows has the exact same issue (because it uses a spawn thing), and already does a "copy_env() + change-in-copy + free" model for that reason.

If that was shared, the UNIX side could just use vfork, I believe. In fact, the following trivial - but horribly ugly - patch passes all the tests, by doing the vfork() in all cases except when the environment changes. But I don't know what coverage that has, though (maybe env is effectively always set?).

And I suspect there are other cases where we'd actually be better off trying to share things than having all the differences hidden away in compat layers.

			Linus

---
 builtin-grep.c |    2 +-
 run-command.c  |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index ef29910..5d3053a 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -168,7 +168,7 @@ static int exec_grep(int argc, const char **argv)
 	int status;
 
 	argv[argc] = NULL;
-	pid = fork();
+	pid = vfork();
 	if (pid < 0)
 		return pid;
 	if (!pid) {
diff --git a/run-command.c b/run-command.c
index 6e29fdf..200ba7b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -68,7 +68,7 @@ int start_command(struct child_process *cmd)
 	trace_argv_printf(cmd->argv, "trace: run_command:");
 
 #ifndef __MINGW32__
-	cmd->pid = fork();
+	cmd->pid = cmd->env ? fork() : vfork();
 	if (!cmd->pid) {
 		if (cmd->no_stdin)
 			dup_devnull(0);