Skip to content

Commit

Permalink
child_process: fix deoptimizing use of arguments
Browse files Browse the repository at this point in the history
Removed or fixed use of arguments in execFile(),
normalizeExecArgs(), and normalizeSpawnArguments().

Refs: #10323
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=6010

Backport-Of: #11535
PR-URL: #11748
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
vsemozhetbyt authored and jasnell committed Mar 16, 2017
1 parent 14e3ad0 commit 5b1d61c
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 20 deletions.
146 changes: 146 additions & 0 deletions benchmark/child_process/child-process-params.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
'use strict';

const common = require('../common.js');
const cp = require('child_process');

const command = 'echo';
const args = ['hello'];
const options = {};
const cb = () => {};

const configs = {
n: [1e3],
methodName: [
'exec', 'execSync',
'execFile', 'execFileSync',
'spawn', 'spawnSync',
],
params: [1, 2, 3, 4],
};

const bench = common.createBenchmark(main, configs);

function main(conf) {
const n = +conf.n;
const methodName = conf.methodName;
const params = +conf.params;

const method = cp[methodName];

switch (methodName) {
case 'exec':
switch (params) {
case 1:
bench.start();
for (let i = 0; i < n; i++) method(command).kill();
bench.end(n);
break;
case 2:
bench.start();
for (let i = 0; i < n; i++) method(command, options).kill();
bench.end(n);
break;
case 3:
bench.start();
for (let i = 0; i < n; i++) method(command, options, cb).kill();
bench.end(n);
break;
}
break;
case 'execSync':
switch (params) {
case 1:
bench.start();
for (let i = 0; i < n; i++) method(command);
bench.end(n);
break;
case 2:
bench.start();
for (let i = 0; i < n; i++) method(command, options);
bench.end(n);
break;
}
break;
case 'execFile':
switch (params) {
case 1:
bench.start();
for (let i = 0; i < n; i++) method(command).kill();
bench.end(n);
break;
case 2:
bench.start();
for (let i = 0; i < n; i++) method(command, args).kill();
bench.end(n);
break;
case 3:
bench.start();
for (let i = 0; i < n; i++) method(command, args, options).kill();
bench.end(n);
break;
case 4:
bench.start();
for (let i = 0; i < n; i++) method(command, args, options, cb).kill();
bench.end(n);
break;
}
break;
case 'execFileSync':
switch (params) {
case 1:
bench.start();
for (let i = 0; i < n; i++) method(command);
bench.end(n);
break;
case 2:
bench.start();
for (let i = 0; i < n; i++) method(command, args);
bench.end(n);
break;
case 3:
bench.start();
for (let i = 0; i < n; i++) method(command, args, options);
bench.end(n);
break;
}
break;
case 'spawn':
switch (params) {
case 1:
bench.start();
for (let i = 0; i < n; i++) method(command).kill();
bench.end(n);
break;
case 2:
bench.start();
for (let i = 0; i < n; i++) method(command, args).kill();
bench.end(n);
break;
case 3:
bench.start();
for (let i = 0; i < n; i++) method(command, args, options).kill();
bench.end(n);
break;
}
break;
case 'spawnSync':
switch (params) {
case 1:
bench.start();
for (let i = 0; i < n; i++) method(command);
bench.end(n);
break;
case 2:
bench.start();
for (let i = 0; i < n; i++) method(command, args);
bench.end(n);
break;
case 3:
bench.start();
for (let i = 0; i < n; i++) method(command, args, options);
bench.end(n);
break;
}
break;
}
}
32 changes: 12 additions & 20 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,10 @@ exports._forkChild = function(fd) {
};


function normalizeExecArgs(command /*, options, callback*/) {
let options;
let callback;

if (typeof arguments[1] === 'function') {
function normalizeExecArgs(command, options, callback) {
if (typeof options === 'function') {
callback = options;
options = undefined;
callback = arguments[1];
} else {
options = arguments[1];
callback = arguments[2];
}

// Make a shallow copy so we don't clobber the user's options object.
Expand Down Expand Up @@ -142,7 +136,7 @@ exports.execFile = function(file /*, args, options, callback*/) {
callback = arguments[pos++];
}

if (!callback && arguments[pos] != null) {
if (!callback && pos < arguments.length && arguments[pos] != null) {
throw new TypeError('Incorrect value of args option');
}

Expand Down Expand Up @@ -175,6 +169,8 @@ exports.execFile = function(file /*, args, options, callback*/) {

var ex = null;

var cmd = file;

function exithandler(code, signal) {
if (exited) return;
exited = true;
Expand Down Expand Up @@ -202,7 +198,6 @@ exports.execFile = function(file /*, args, options, callback*/) {
return;
}

var cmd = file;
if (args.length !== 0)
cmd += ' ' + args.join(' ');

Expand Down Expand Up @@ -311,18 +306,15 @@ function _convertCustomFds(options) {
}
}

function normalizeSpawnArguments(file /*, args, options*/) {
var args, options;

if (Array.isArray(arguments[1])) {
args = arguments[1].slice(0);
options = arguments[2];
} else if (arguments[1] !== undefined &&
(arguments[1] === null || typeof arguments[1] !== 'object')) {
function normalizeSpawnArguments(file, args, options) {
if (Array.isArray(args)) {
args = args.slice(0);
} else if (args !== undefined &&
(args === null || typeof args !== 'object')) {
throw new TypeError('Incorrect value of args option');
} else {
options = args;
args = [];
options = arguments[1];
}

if (options === undefined)
Expand Down

0 comments on commit 5b1d61c

Please sign in to comment.