src: run node --run scripts without a subshell#63978
Open
anonrig wants to merge 1 commit into
Open
Conversation
node --run previously spawned the script's command through /bin/sh and kept the (large) node process resident to wait for the child, then went through the normal process shutdown. For a launcher that never initializes V8/JS this is mostly wasted work: forking the node process, an event-loop round-trip and tearing down V8/ICU/OpenSSL static state. On POSIX, replace this with a direct hand-off: - Simple commands (a plain `program args` invocation with no shell metacharacters) are executed directly with execvp(), bypassing the shell entirely. Positional arguments are passed through verbatim. - Anything that needs shell syntax execve()s /bin/sh, still replacing the node process instead of forking a child and waiting for it. - If the exec fails (e.g. the command is a shell builtin or is not found) it falls back to the original uv_spawn() path. ICU initialization is skipped for --run as well, since the Intl APIs are never reached. Because node now execs the target directly, the script's exact exit status is propagated instead of being collapsed to 1. Benchmarks on macOS for `node --run` of a node_modules/.bin binary show roughly a 1.4x reduction in wall-clock time versus the previous shell spawn. Windows keeps the existing spawn-based behavior. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
|
Review requested:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
node --run <script>is a thin launcher: it never initializes V8/JS, itjust runs the package.json script command. Today it always spawns the
command through
/bin/sh, keeps the (large) node process resident towait for the child, and then goes through the normal shutdown. That means
paying for a fork of the node process, an event-loop round-trip, and the
teardown of V8/ICU/OpenSSL static state β none of which the task runner
needs.
This PR makes the POSIX path hand the process off directly:
program argsinvocation with no shellmetacharacters) are run directly via
execvp(), skipping/bin/shentirely. The augmented
node_modules/.binPATH and theNODE_RUN_*variables are applied first so resolution and the child environment are
unchanged. Positional arguments after
--are passed through verbatim.&&,|,$, globs, quotes, β¦)execve()/bin/sh -c <command>, still replacing the node processinstead of forking a child and waiting.
it falls back to the original
uv_spawn()path, so behavior ispreserved for every case.
ICU initialization is also skipped for
--run, since the Intl APIs arenever reached.
Windows keeps the existing spawn-based behavior.
Benchmark
macOS,
node --runof anode_modules/.binbinary, interleaved A/B,600 iters Γ 8 rounds:
node --run <bin>The remaining time is essentially node's own load floor (
node -vis~12β13 ms of pre-
main()dyld + V8/ICU static init), so this removesmost of the avoidable overhead between that floor and the script.
Behavior change worth a look
Because node now execs the target/shell directly, the script's exact
exit status is propagated (e.g.
exit 2β2). The previous codecollapsed any non-zero status to
1. This matchesnpm run/shellsemantics; the existing
test/parallel/test-node-run.jsassertions(codes 0 and 1) still pass, but flagging it explicitly.
Test plan
test/parallel/test-node-run.jsandtest/message/node_run_non_existent.jspass (covernode_modules/.binresolution,NODE_RUN_*env vars, parent-dirpackage.json search, and verbatim positional args β all now via the
fast path)
cpplintcleanMade with Cursor