Re: Should we discuss Windows-related changes on

[Linus Torvalds <torvalds@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);