child_process: fix deoptimizing use of arguments

vsemozhetbyt · MylesBorins · commit b46cf355261d · 2017-07-11T17:43:47.000+01:00
Remove use of arguments in normalizeExecArgs() and normalizeSpawnArguments(). Refs: #10323 PR-URL: #11535 Backport-PR-URL: #13752 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
diff --git a/benchmark/child_process/child-process-params.js 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
@@ -74,16 +74,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.
@@ -303,18 +297,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)

-Original file line number
+Diff line change
@@ @@ -0,0 +1,146 @@ @@
 +'use strict';
++
 +constcommon=require('../common.js');
 +constcp=require('child_process');
++
 +constcommand='echo';
 +constargs=['hello'];
 +constoptions={};
 +constcb=()=>{};
++
 +constconfigs={
 +n: [1e3],
 +methodName: [
 +'exec','execSync',
 +'execFile','execFileSync',
 +'spawn','spawnSync',
 +],
 +params: [1,2,3,4],
 +};
++
 +constbench=common.createBenchmark(main,configs);
++
 +functionmain(conf){
 +constn=+conf.n;
 +constmethodName=conf.methodName;
 +constparams=+conf.params;
++
 +constmethod=cp[methodName];
++
 +switch(methodName){
 +case'exec':
 +switch(params){
 +case1:
 +bench.start();
 +for(leti=0;i<n;i++)method(command).kill();
 +bench.end(n);
 +break;
 +case2:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,options).kill();
 +bench.end(n);
 +break;
 +case3:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,options,cb).kill();
 +bench.end(n);
 +break;
 +}
 +break;
 +case'execSync':
 +switch(params){
 +case1:
 +bench.start();
 +for(leti=0;i<n;i++)method(command);
 +bench.end(n);
 +break;
 +case2:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,options);
 +bench.end(n);
 +break;
 +}
 +break;
 +case'execFile':
 +switch(params){
 +case1:
 +bench.start();
 +for(leti=0;i<n;i++)method(command).kill();
 +bench.end(n);
 +break;
 +case2:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args).kill();
 +bench.end(n);
 +break;
 +case3:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args,options).kill();
 +bench.end(n);
 +break;
 +case4:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args,options,cb).kill();
 +bench.end(n);
 +break;
 +}
 +break;
 +case'execFileSync':
 +switch(params){
 +case1:
 +bench.start();
 +for(leti=0;i<n;i++)method(command);
 +bench.end(n);
 +break;
 +case2:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args);
 +bench.end(n);
 +break;
 +case3:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args,options);
 +bench.end(n);
 +break;
 +}
 +break;
 +case'spawn':
 +switch(params){
 +case1:
 +bench.start();
 +for(leti=0;i<n;i++)method(command).kill();
 +bench.end(n);
 +break;
 +case2:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args).kill();
 +bench.end(n);
 +break;
 +case3:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args,options).kill();
 +bench.end(n);
 +break;
 +}
 +break;
 +case'spawnSync':
 +switch(params){
 +case1:
 +bench.start();
 +for(leti=0;i<n;i++)method(command);
 +bench.end(n);
 +break;
 +case2:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args);
 +bench.end(n);
 +break;
 +case3:
 +bench.start();
 +for(leti=0;i<n;i++)method(command,args,options);
 +bench.end(n);
 +break;
 +}
 +break;
 +}
 +}
-Original file line number
+Diff line change
 };
 -functionnormalizeExecArgs(command/*, options, callback*/){
 -letoptions;
 -letcallback;
+-
 -if(typeofarguments[1]==='function'){
 +functionnormalizeExecArgs(command,options,callback){
 +if(typeofoptions==='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.
+}
+}
 -functionnormalizeSpawnArguments(file/*, args, options*/){
 -varargs,options;
+-
 -if(Array.isArray(arguments[1])){
 -args=arguments[1].slice(0);
 -options=arguments[2];
 -}elseif(arguments[1]!==undefined&&
 -(arguments[1]===null||typeofarguments[1]!=='object')){
 +functionnormalizeSpawnArguments(file,args,options){
 +if(Array.isArray(args)){
 +args=args.slice(0);
 +}elseif(args!==undefined&&
 +(args===null||typeofargs!=='object')){
 thrownewTypeError('Incorrect value of args option');
 }else{
 +options=args;
 args=[];
 -options=arguments[1];
+}
 if(options===undefined)