Skip to content

child_process: fix deoptimizing use of arguments

Checklist
  • make -j4 test (UNIX), or vcbuild 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

Merge request reports

Loading