From 1f8b85786eed623319e5c71a5341b15e3006f870 Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Mon, 18 Mar 2024 12:38:49 +0100
Subject: [PATCH] libutil: remove vfork
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

vfork confers a large performance advantage over fork, measured locally
at 16µs per vfork agains 90µs per fork. however nix *almost always*
follows a vfork up with an execve-family call, melting the performance
advantage from 6x to only 15%. in most of those cases it's doing things
that are undefined behavior (like manipulating the heap, or even
throwing exceptions and trashing the parent process stack).

most notably the one place that could benefit from the vfork performance
improvement is linux derivation sandbox setup—which doesn't use vfork.

Change-Id: I2037b7384d5a4ca24da219a569e1b1f39531410e
---
 src/libutil/util.cc | 18 +++---------------
 src/libutil/util.hh |  1 -
 src/nix/daemon.cc   |  1 -
 3 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 9bb769fc2..328548103 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -1095,16 +1095,9 @@ void killUser(uid_t uid)
 //////////////////////////////////////////////////////////////////////
 
 
-/* Wrapper around vfork to prevent the child process from clobbering
-   the caller's stack frame in the parent. */
-static pid_t doFork(bool allowVfork, std::function<void()> fun) __attribute__((noinline));
-static pid_t doFork(bool allowVfork, std::function<void()> fun)
+static pid_t doFork(std::function<void()> fun)
 {
-#ifdef __linux__
-    pid_t pid = allowVfork ? vfork() : fork();
-#else
     pid_t pid = fork();
-#endif
     if (pid != 0) return pid;
     fun();
     abort();
@@ -1124,8 +1117,7 @@ static int childEntry(void * arg)
 pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
 {
     std::function<void()> wrapper = [&]() {
-        if (!options.allowVfork)
-            logger = makeSimpleLogger();
+        logger = makeSimpleLogger();
         try {
 #if __linux__
             if (options.dieWithParent && prctl(PR_SET_PDEATHSIG, SIGKILL) == -1)
@@ -1162,7 +1154,7 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
         throw Error("clone flags are only supported on Linux");
         #endif
     } else
-        pid = doFork(options.allowVfork, wrapper);
+        pid = doFork(wrapper);
 
     if (pid == -1) throw SysError("unable to fork");
 
@@ -1226,10 +1218,6 @@ void runProgram2(const RunOptions & options)
     if (source) in.create();
 
     ProcessOptions processOptions;
-    // vfork implies that the environment of the main process and the fork will
-    // be shared (technically this is undefined, but in practice that's the
-    // case), so we can't use it if we alter the environment
-    processOptions.allowVfork = !options.environment;
 
     std::optional<Finally<std::function<void()>>> resumeLoggerDefer;
     if (options.isInteractive) {
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index ed4c8705a..34ac33349 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -413,7 +413,6 @@ struct ProcessOptions
     std::string errorPrefix = "";
     bool dieWithParent = true;
     bool runExitHandlers = false;
-    bool allowVfork = false;
     /**
      * use clone() with the specified flags (Linux only)
      */
diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc
index 42ce3b996..4f9907ad7 100644
--- a/src/nix/daemon.cc
+++ b/src/nix/daemon.cc
@@ -347,7 +347,6 @@ static void daemonLoop(std::optional<TrustedFlag> forceTrustClientOpt)
             options.errorPrefix = "unexpected Nix daemon error: ";
             options.dieWithParent = false;
             options.runExitHandlers = true;
-            options.allowVfork = false;
             startProcess([&]() {
                 fdSocket = -1;