Skip to content

Commit dd81d53

Browse files
vsemozhetbytcjihrig
vsemozhetbyt
authored and
cjihrig
committed
child_process: fix deoptimizing use of arguments
Removed or fixed use of arguments in execFile(), normalizeExecArgs(), and normalizeSpawnArguments(). Refs: #10323 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=6010 PR-URL: #11535 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
1 parent abc13e5 commit dd81d53

File tree

2 files changed

+158
-20
lines changed

2 files changed

+158
-20
lines changed

‎benchmark/child_process/child-process-params.js‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
'use strict';
2+
3+
const common = require('../common.js');
4+
const cp = require('child_process');
5+
6+
const command = 'echo';
7+
const args = ['hello'];
8+
const options = {};
9+
const cb = () => {};
10+
11+
const configs = {
12+
n: [1e3],
13+
methodName: [
14+
'exec', 'execSync',
15+
'execFile', 'execFileSync',
16+
'spawn', 'spawnSync',
17+
],
18+
params: [1, 2, 3, 4],
19+
};
20+
21+
const bench = common.createBenchmark(main, configs);
22+
23+
function main(conf) {
24+
const n = +conf.n;
25+
const methodName = conf.methodName;
26+
const params = +conf.params;
27+
28+
const method = cp[methodName];
29+
30+
switch (methodName) {
31+
case 'exec':
32+
switch (params) {
33+
case 1:
34+
bench.start();
35+
for (let i = 0; i < n; i++) method(command).kill();
36+
bench.end(n);
37+
break;
38+
case 2:
39+
bench.start();
40+
for (let i = 0; i < n; i++) method(command, options).kill();
41+
bench.end(n);
42+
break;
43+
case 3:
44+
bench.start();
45+
for (let i = 0; i < n; i++) method(command, options, cb).kill();
46+
bench.end(n);
47+
break;
48+
}
49+
break;
50+
case 'execSync':
51+
switch (params) {
52+
case 1:
53+
bench.start();
54+
for (let i = 0; i < n; i++) method(command);
55+
bench.end(n);
56+
break;
57+
case 2:
58+
bench.start();
59+
for (let i = 0; i < n; i++) method(command, options);
60+
bench.end(n);
61+
break;
62+
}
63+
break;
64+
case 'execFile':
65+
switch (params) {
66+
case 1:
67+
bench.start();
68+
for (let i = 0; i < n; i++) method(command).kill();
69+
bench.end(n);
70+
break;
71+
case 2:
72+
bench.start();
73+
for (let i = 0; i < n; i++) method(command, args).kill();
74+
bench.end(n);
75+
break;
76+
case 3:
77+
bench.start();
78+
for (let i = 0; i < n; i++) method(command, args, options).kill();
79+
bench.end(n);
80+
break;
81+
case 4:
82+
bench.start();
83+
for (let i = 0; i < n; i++) method(command, args, options, cb).kill();
84+
bench.end(n);
85+
break;
86+
}
87+
break;
88+
case 'execFileSync':
89+
switch (params) {
90+
case 1:
91+
bench.start();
92+
for (let i = 0; i < n; i++) method(command);
93+
bench.end(n);
94+
break;
95+
case 2:
96+
bench.start();
97+
for (let i = 0; i < n; i++) method(command, args);
98+
bench.end(n);
99+
break;
100+
case 3:
101+
bench.start();
102+
for (let i = 0; i < n; i++) method(command, args, options);
103+
bench.end(n);
104+
break;
105+
}
106+
break;
107+
case 'spawn':
108+
switch (params) {
109+
case 1:
110+
bench.start();
111+
for (let i = 0; i < n; i++) method(command).kill();
112+
bench.end(n);
113+
break;
114+
case 2:
115+
bench.start();
116+
for (let i = 0; i < n; i++) method(command, args).kill();
117+
bench.end(n);
118+
break;
119+
case 3:
120+
bench.start();
121+
for (let i = 0; i < n; i++) method(command, args, options).kill();
122+
bench.end(n);
123+
break;
124+
}
125+
break;
126+
case 'spawnSync':
127+
switch (params) {
128+
case 1:
129+
bench.start();
130+
for (let i = 0; i < n; i++) method(command);
131+
bench.end(n);
132+
break;
133+
case 2:
134+
bench.start();
135+
for (let i = 0; i < n; i++) method(command, args);
136+
bench.end(n);
137+
break;
138+
case 3:
139+
bench.start();
140+
for (let i = 0; i < n; i++) method(command, args, options);
141+
bench.end(n);
142+
break;
143+
}
144+
break;
145+
}
146+
}

‎lib/child_process.js‎

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -93,16 +93,10 @@ exports._forkChild = function(fd) {
9393
};
9494

9595

96-
function normalizeExecArgs(command /*, options, callback*/) {
97-
let options;
98-
let callback;
99-
100-
if (typeof arguments[1] === 'function') {
96+
function normalizeExecArgs(command, options, callback) {
97+
if (typeof options === 'function') {
98+
callback = options;
10199
options = undefined;
102-
callback = arguments[1];
103-
} else {
104-
options = arguments[1];
105-
callback = arguments[2];
106100
}
107101

108102
// 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*/) {
156150
callback = arguments[pos++];
157151
}
158152

159-
if (!callback && arguments[pos] != null) {
153+
if (!callback && pos < arguments.length && arguments[pos] != null) {
160154
throw new TypeError('Incorrect value of args option');
161155
}
162156

@@ -195,6 +189,8 @@ exports.execFile = function(file /*, args, options, callback*/) {
195189

196190
var ex = null;
197191

192+
var cmd = file;
193+
198194
function exithandler(code, signal) {
199195
if (exited) return;
200196
exited = true;
@@ -222,7 +218,6 @@ exports.execFile = function(file /*, args, options, callback*/) {
222218
return;
223219
}
224220

225-
var cmd = file;
226221
if (args.length !== 0)
227222
cmd += ' ' + args.join(' ');
228223

@@ -330,21 +325,18 @@ function _convertCustomFds(options) {
330325
}
331326
}
332327

333-
function normalizeSpawnArguments(file /*, args, options*/) {
334-
var args, options;
335-
328+
function normalizeSpawnArguments(file, args, options) {
336329
if (typeof file !== 'string' || file.length === 0)
337330
throw new TypeError('"file" argument must be a non-empty string');
338331

339-
if (Array.isArray(arguments[1])) {
340-
args = arguments[1].slice(0);
341-
options = arguments[2];
342-
} else if (arguments[1] !== undefined &&
343-
(arguments[1] === null || typeof arguments[1] !== 'object')) {
332+
if (Array.isArray(args)) {
333+
args = args.slice(0);
334+
} else if (args !== undefined &&
335+
(args === null || typeof args !== 'object')) {
344336
throw new TypeError('Incorrect value of args option');
345337
} else {
338+
options = args;
346339
args = [];
347-
options = arguments[1];
348340
}
349341

350342
if (options === undefined)

0 commit comments

Comments
 (0)