Skip the shell for simple scripts (~2x faster), fix Windows PATH joining - #3
Merged
Conversation
Scripts without shell metacharacters (the common case: vitest, tsc -p ., eslint src) are now exec'd directly instead of through sh -c, cutting ~4ms of shell startup. Extra args are passed as real argv entries on this path, so quoted arguments with spaces are no longer re-split. Scripts with metacharacters or leading VAR=value assignments still go through the shell, with exec-failure fallback to sh for its diagnostics. Also: - Join PATH with std::env::join_paths so the Windows branch gets ";" separators instead of ":" - Read package.json directly in the ancestor walk (one syscall instead of stat+open, removes the TOCTOU window) - Parse with from_slice into borrowed Cow strings (no per-script allocations or separate UTF-8 pass) - Use args_os so non-UTF-8 arguments no longer panic Benchmark on this machine: nr test drops from ~9.0ms to ~4.7ms median. README table regenerated (28x -> 31x headline). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The old harness wrapped every runner in Node's execSync, which spawns /bin/sh around each measured command. That added a flat ~7ms to every runner and drowned out the differences between the fast ones. hyperfine with --shell=none execs the runners directly, all in one invocation with warmup runs and statistical output. Headline on this machine: nr measures 3.7ms median (was reported as 8ms), 2x faster than bun and 66x faster than pnpm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
What
Profiling showed that
nr's own work is only ~2ms. About 70% of the runtime ofnr testwas spawning/bin/sh(which is bash on macOS, ~4ms of startup).Direct-exec fast path. Scripts with no shell metacharacters and no leading
VAR=valueassignment are now exec'd directly viaexecvpsemantics instead of throughsh -c. This covers the typical case (vitest,tsc -p .,eslint src). Anything with operators, quotes, globs, or env prefixes falls back to the shell exactly as before, and a failed direct exec also falls through toshso users keep its familiar "command not found" diagnostics.A free correctness win on this path: extra arguments are passed as real argv entries, so
nr argcount "two words"arrives as one argument instead of being re-split by the shell.Windows PATH bug fix.
node_modules/.binpaths were joined with:on all platforms, so thecmd /Cbranch received a broken PATH. Now usesstd::env::join_paths, which picks the right separator per platform.Micro-optimizations.
package.jsonis read directly inside the ancestor walk (one syscall instead of stat then open, and removes the TOCTOU window)serde_json::from_sliceinto borrowedCow<str>scripts, eliminating the separate UTF-8 validation pass and per-script allocationsenv::args_osreplacesenv::args, so non-UTF-8 arguments no longer panicNumbers
Median of 30 runs on this machine (Apple Silicon, subprocess harness with ~3ms constant spawn overhead):
nr test(shell-free script)README benchmark table regenerated: headline speedup vs pnpm went from 28x to 31x, and
nrwidened its lead over bun (8ms vs 10ms in that harness). Note the benchmark'sexecSyncwrapper adds a ~7ms constant to every runner, so the real-world improvement is larger than the table suggests.Tests
Four new cases in
test.sh:argc=2)&&still run through the shellFOO=barenv assignments still run through the shellnode_modules/.binshims resolve on the direct-exec pathAll 11 tests pass. The
testfixture script was changed fromecho 'test passed'toecho test passed(identical output) so the benchmark exercises the fast path.🤖 Generated with Claude Code