From 20480ff78899bf737526b47d453aed92a5c9a5c5 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sun, 19 Feb 2017 18:23:59 +0200 Subject: [PATCH] child_process: fix deoptimizing use of arguments Fixed functions: normalizeExecArgs(), exports.execFile(), normalizeSpawnArguments() Refs: https://github.com/nodejs/node/issues/10323 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=6010 --- .../child_process/child-process-params.js | 146 ++++++++++++++++++ lib/child_process.js | 32 ++-- 2 files changed, 158 insertions(+), 20 deletions(-) create mode 100644 benchmark/child_process/child-process-params.js diff --git a/benchmark/child_process/child-process-params.js b/benchmark/child_process/child-process-params.js new file mode 100644 index 00000000000000..644b2136a0f03f --- /dev/null +++ b/benchmark/child_process/child-process-params.js @@ -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; + } +} diff --git a/lib/child_process.js b/lib/child_process.js index 9ca5189a73a641..aff6025e332666 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -93,16 +93,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. @@ -156,7 +150,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'); } @@ -195,6 +189,8 @@ exports.execFile = function(file /*, args, options, callback*/) { var ex = null; + var cmd = file; + function exithandler(code, signal) { if (exited) return; exited = true; @@ -222,7 +218,6 @@ exports.execFile = function(file /*, args, options, callback*/) { return; } - var cmd = file; if (args.length !== 0) cmd += ' ' + args.join(' '); @@ -330,21 +325,18 @@ function _convertCustomFds(options) { } } -function normalizeSpawnArguments(file /*, args, options*/) { - var args, options; - +function normalizeSpawnArguments(file, args, options) { if (typeof file !== 'string' || file.length === 0) throw new TypeError('"file" argument must be a non-empty string'); - 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')) { + 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)