Skip to content

Commit 5b1d61c

Browse files
vsemozhetbytjasnell
vsemozhetbyt
authored and
jasnell
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 Backport-Of: #11535 PR-URL: #11748 Reviewed-By: James M Snell <[email protected]>
1 parent 14e3ad0 commit 5b1d61c

File tree

2 files changed

+158
-20
lines changed

2 files changed

+158
-20
lines changed

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

+146
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

+12-20
Original file line numberDiff line numberDiff line change
@@ -79,16 +79,10 @@ exports._forkChild = function(fd) {
7979
};
8080

8181

82-
function normalizeExecArgs(command /*, options, callback*/) {
83-
let options;
84-
let callback;
85-
86-
if (typeof arguments[1] === 'function') {
82+
function normalizeExecArgs(command, options, callback) {
83+
if (typeof options === 'function') {
84+
callback = options;
8785
options = undefined;
88-
callback = arguments[1];
89-
} else {
90-
options = arguments[1];
91-
callback = arguments[2];
9286
}
9387

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

145-
if (!callback && arguments[pos] != null) {
139+
if (!callback && pos < arguments.length && arguments[pos] != null) {
146140
throw new TypeError('Incorrect value of args option');
147141
}
148142

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

176170
var ex = null;
177171

172+
var cmd = file;
173+
178174
function exithandler(code, signal) {
179175
if (exited) return;
180176
exited = true;
@@ -202,7 +198,6 @@ exports.execFile = function(file /*, args, options, callback*/) {
202198
return;
203199
}
204200

205-
var cmd = file;
206201
if (args.length !== 0)
207202
cmd += ' ' + args.join(' ');
208203

@@ -311,18 +306,15 @@ function _convertCustomFds(options) {
311306
}
312307
}
313308

314-
function normalizeSpawnArguments(file /*, args, options*/) {
315-
var args, options;
316-
317-
if (Array.isArray(arguments[1])) {
318-
args = arguments[1].slice(0);
319-
options = arguments[2];
320-
} else if (arguments[1] !== undefined &&
321-
(arguments[1] === null || typeof arguments[1] !== 'object')) {
309+
function normalizeSpawnArguments(file, args, options) {
310+
if (Array.isArray(args)) {
311+
args = args.slice(0);
312+
} else if (args !== undefined &&
313+
(args === null || typeof args !== 'object')) {
322314
throw new TypeError('Incorrect value of args option');
323315
} else {
316+
options = args;
324317
args = [];
325-
options = arguments[1];
326318
}
327319

328320
if (options === undefined)

0 commit comments

Comments
 (0)