forked from lix-project/lix
libutil: remove vfork
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
This commit is contained in:
parent
2890840b96
commit
1f8b85786e
|
@ -1095,16 +1095,9 @@ void killUser(uid_t uid)
|
||||||
//////////////////////////////////////////////////////////////////////
|
//////////////////////////////////////////////////////////////////////
|
||||||
|
|
||||||
|
|
||||||
/* Wrapper around vfork to prevent the child process from clobbering
|
static pid_t doFork(std::function<void()> fun)
|
||||||
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)
|
|
||||||
{
|
{
|
||||||
#ifdef __linux__
|
|
||||||
pid_t pid = allowVfork ? vfork() : fork();
|
|
||||||
#else
|
|
||||||
pid_t pid = fork();
|
pid_t pid = fork();
|
||||||
#endif
|
|
||||||
if (pid != 0) return pid;
|
if (pid != 0) return pid;
|
||||||
fun();
|
fun();
|
||||||
abort();
|
abort();
|
||||||
|
@ -1124,7 +1117,6 @@ static int childEntry(void * arg)
|
||||||
pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
|
pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
|
||||||
{
|
{
|
||||||
std::function<void()> wrapper = [&]() {
|
std::function<void()> wrapper = [&]() {
|
||||||
if (!options.allowVfork)
|
|
||||||
logger = makeSimpleLogger();
|
logger = makeSimpleLogger();
|
||||||
try {
|
try {
|
||||||
#if __linux__
|
#if __linux__
|
||||||
|
@ -1162,7 +1154,7 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
|
||||||
throw Error("clone flags are only supported on Linux");
|
throw Error("clone flags are only supported on Linux");
|
||||||
#endif
|
#endif
|
||||||
} else
|
} else
|
||||||
pid = doFork(options.allowVfork, wrapper);
|
pid = doFork(wrapper);
|
||||||
|
|
||||||
if (pid == -1) throw SysError("unable to fork");
|
if (pid == -1) throw SysError("unable to fork");
|
||||||
|
|
||||||
|
@ -1226,10 +1218,6 @@ void runProgram2(const RunOptions & options)
|
||||||
if (source) in.create();
|
if (source) in.create();
|
||||||
|
|
||||||
ProcessOptions processOptions;
|
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;
|
std::optional<Finally<std::function<void()>>> resumeLoggerDefer;
|
||||||
if (options.isInteractive) {
|
if (options.isInteractive) {
|
||||||
|
|
|
@ -413,7 +413,6 @@ struct ProcessOptions
|
||||||
std::string errorPrefix = "";
|
std::string errorPrefix = "";
|
||||||
bool dieWithParent = true;
|
bool dieWithParent = true;
|
||||||
bool runExitHandlers = false;
|
bool runExitHandlers = false;
|
||||||
bool allowVfork = false;
|
|
||||||
/**
|
/**
|
||||||
* use clone() with the specified flags (Linux only)
|
* use clone() with the specified flags (Linux only)
|
||||||
*/
|
*/
|
||||||
|
|
|
@ -347,7 +347,6 @@ static void daemonLoop(std::optional<TrustedFlag> forceTrustClientOpt)
|
||||||
options.errorPrefix = "unexpected Nix daemon error: ";
|
options.errorPrefix = "unexpected Nix daemon error: ";
|
||||||
options.dieWithParent = false;
|
options.dieWithParent = false;
|
||||||
options.runExitHandlers = true;
|
options.runExitHandlers = true;
|
||||||
options.allowVfork = false;
|
|
||||||
startProcess([&]() {
|
startProcess([&]() {
|
||||||
fdSocket = -1;
|
fdSocket = -1;
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue