child_process: fix deoptimizing use of arguments
Checklist
-
make -j4 test
(UNIX), orvcbuild test
(Windows) passes -
tests and/or benchmarks are included -
commit message follows commit guidelines
Affected core subsystem(s)
child_process, benchmarks
This is the last unaddressed case from mentioned in the https://github.com/nodejs/node/issues/10323. PR fixes three functions.
1. Function normalizeExecArgs()
is called by exports.exec()
and exports.execSync()
with a variable number of parameters.
To check deoptimizations, run this code with --trace_opt --trace_deopt
flags:
const execSync = require('child_process').execSync;
for (let i = 0; i < 1e3; i++) execSync('echo');
You will see messages about normalizeExecArgs()
deoptimization.
None of the current benchmarks checks this case: the only benchmark calling exports.exec()
does it once and always with 2 parameters.
2. Function exports.execFile()
is called by users with a variable number of parameters. See also https://github.com/nodejs/node/issues/10323#issuecomment-268038165
To check deoptimizations, run this code with --trace_opt --trace_deopt
flags:
const execFile = require('child_process').execFile;
for (let i = 0; i < 1e3; i++) execFile('echo').kill();
You will see messages about exports.execFile()
deoptimization.
None of the current benchmarks checks exports.execFile()
.
3. Function normalizeSpawnArguments()
is called by exports.spawn()
, exports.spawnSync()
, and exports.execFileSync()
with a variable number of parameters.
To check deoptimizations, run this code with --trace_opt --trace_deopt
flags:
const spawnSync = require('child_process').spawnSync;
for (let i = 0; i < 1e3; i++) spawnSync('echo');
You will see messages about normalizeSpawnArguments()
deoptimization.
None of the current benchmarks checks exports.spawnSync()
or exports.execFileSync()
. Two of them call exports.spawn()
once with 3 params (not appropriate). One of them calls exports.spawn()
1000 times with 2 params, but it does not cover all sub-cases.
4. While fixing these cases, I've found an absolutely new deopt case in the exports.execFile()
function introduced by v8 5.6. See the fourth commit and explanation in the upstream issue.
5. As changes affect almost all main child_process
methods (except .fork()
), I've added a benchmark calling these 6 methods with 1, 2, 3, and 4 params (when it is appropriate). Benchmarking is a bit murky in this case because of many external sync/async processes involved, but it seems no significant degradation is caused, moreover some tiny improvement is evident.
improvement confidence p.value
child_process\\child-process-params.js params=1 methodName="exec" n=1000 0.59 % 6.187539e-02
child_process\\child-process-params.js params=1 methodName="execFile" n=1000 0.62 % * 2.916666e-02
child_process\\child-process-params.js params=1 methodName="execFileSync" n=1000 0.32 % 5.213650e-01
child_process\\child-process-params.js params=1 methodName="execSync" n=1000 0.26 % * 4.716863e-02
child_process\\child-process-params.js params=1 methodName="spawn" n=1000 0.69 % *** 2.204024e-04
child_process\\child-process-params.js params=1 methodName="spawnSync" n=1000 0.19 % 7.165778e-01
child_process\\child-process-params.js params=2 methodName="exec" n=1000 0.72 % * 4.544538e-02
child_process\\child-process-params.js params=2 methodName="execFile" n=1000 0.68 % * 2.451089e-02
child_process\\child-process-params.js params=2 methodName="execFileSync" n=1000 -0.30 % 5.470930e-01
child_process\\child-process-params.js params=2 methodName="execSync" n=1000 0.30 % 1.907476e-01
child_process\\child-process-params.js params=2 methodName="spawn" n=1000 0.40 % ** 6.548929e-03
child_process\\child-process-params.js params=2 methodName="spawnSync" n=1000 0.51 % 2.825198e-01
child_process\\child-process-params.js params=3 methodName="exec" n=1000 0.37 % 2.182035e-01
child_process\\child-process-params.js params=3 methodName="execFile" n=1000 0.86 % *** 1.298733e-06
child_process\\child-process-params.js params=3 methodName="execFileSync" n=1000 0.22 % 7.333930e-01
child_process\\child-process-params.js params=3 methodName="spawn" n=1000 0.95 % ** 7.015692e-03
child_process\\child-process-params.js params=3 methodName="spawnSync" n=1000 -0.43 % 4.574352e-01
child_process\\child-process-params.js params=4 methodName="execFile" n=1000 1.15 % *** 6.094645e-07