Revert "benchmark: add test and all options and improve errors"

This reverts commit dac5795. Refs: #31396 PR-URL: #31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
nodejs · Feb 17, 2020 · 8974fa7 · 8974fa7
1 parent 5b3c4b3
commit 8974fa7
Show file tree

Hide file tree

Showing 37 changed files with 189 additions and 292 deletions.
diff --git a/benchmark/_cli.js b/benchmark/_cli.js
@@ -6,24 +6,22 @@ const path = require('path');
 // Create an object of all benchmark scripts
 const benchmarks = {};
 fs.readdirSync(__dirname)
-  .filter((name) => {
-    return name !== 'fixtures' &&
-           fs.statSync(path.resolve(__dirname, name)).isDirectory();
-  })
+  .filter((name) => fs.statSync(path.resolve(__dirname, name)).isDirectory())
   .forEach((category) => {
     benchmarks[category] = fs.readdirSync(path.resolve(__dirname, category))
       .filter((filename) => filename[0] !== '.' && filename[0] !== '_');
   });
 
 function CLI(usage, settings) {
+  if (!(this instanceof CLI)) return new CLI(usage, settings);
+
   if (process.argv.length < 3) {
     this.abort(usage); // Abort will exit the process
   }
 
   this.usage = usage;
   this.optional = {};
   this.items = [];
-  this.test = false;
 
   for (const argName of settings.arrayArgs) {
     this.optional[argName] = [];
@@ -36,7 +34,7 @@ function CLI(usage, settings) {
     if (arg === '--') {
       // Only items can follow --
       mode = 'item';
-    } else if (mode === 'both' && arg[0] === '-') {
+    } else if ('both' === mode && arg[0] === '-') {
       // Optional arguments declaration
 
       if (arg[1] === '-') {
@@ -63,8 +61,6 @@ function CLI(usage, settings) {
 
       // The next value can be either an option or an item
       mode = 'both';
-    } else if (arg === 'test') {
-      this.test = true;
     } else if (['both', 'item'].includes(mode)) {
       // item arguments
       this.items.push(arg);
@@ -87,15 +83,9 @@ CLI.prototype.abort = function(msg) {
 CLI.prototype.benchmarks = function() {
   const paths = [];
 
-  if (this.items.includes('all')) {
-    this.items = Object.keys(benchmarks);
-  }
-
   for (const category of this.items) {
-    if (benchmarks[category] === undefined) {
-      console.error(`The "${category}" category does not exist.`);
-      process.exit(1);
-    }
+    if (benchmarks[category] === undefined)
+      continue;
     for (const scripts of benchmarks[category]) {
       if (this.shouldSkip(scripts)) continue;
 

diff --git a/benchmark/_http-benchmarkers.js b/benchmark/_http-benchmarkers.js
@@ -43,8 +43,9 @@ class AutocannonBenchmarker {
     }
     if (!result || !result.requests || !result.requests.average) {
       return undefined;
+    } else {
+      return result.requests.average;
     }
-    return result.requests.average;
   }
 }
 
@@ -76,8 +77,9 @@ class WrkBenchmarker {
     const throughput = match && +match[1];
     if (!isFinite(throughput)) {
       return undefined;
+    } else {
+      return throughput;
     }
-    return throughput;
   }
 }
 
@@ -87,20 +89,18 @@ class WrkBenchmarker {
  */
 class TestDoubleBenchmarker {
   constructor(type) {
-    // `type` is the type of benchmarker. Possible values are 'http' and
-    // 'http2'.
+    // `type` is the type ofbenchmarker. Possible values are 'http' and 'http2'.
     this.name = `test-double-${type}`;
     this.executable = path.resolve(__dirname, '_test-double-benchmarker.js');
     this.present = fs.existsSync(this.executable);
     this.type = type;
   }
 
   create(options) {
-    const env = {
+    const env = Object.assign({
       duration: options.duration,
       test_url: `http://127.0.0.1:${options.port}${options.path}`,
-      ...process.env
-    };
+    }, process.env);
 
     const child = child_process.fork(this.executable,
                                      [this.type],
@@ -189,14 +189,13 @@ http_benchmarkers.forEach((benchmarker) => {
 });
 
 exports.run = function(options, callback) {
-  options = {
+  options = Object.assign({
     port: exports.PORT,
     path: '/',
     connections: 100,
     duration: 5,
     benchmarker: exports.default_http_benchmarker,
-    ...options
-  };
+  }, options);
   if (!options.benchmarker) {
     callback(new Error('Could not locate required http benchmarker. See ' +
                        `${requirementsURL} for further instructions.`));
@@ -213,7 +212,6 @@ exports.run = function(options, callback) {
                        'is  not installed'));
     return;
   }
-  process.env.duration = process.env.duration || options.duration || 5;
 
   const benchmarker_start = process.hrtime();
 
@@ -222,8 +220,7 @@ exports.run = function(options, callback) {
   child.stderr.pipe(process.stderr);
 
   let stdout = '';
-  child.stdout.setEncoding('utf8');
-  child.stdout.on('data', (chunk) => stdout += chunk);
+  child.stdout.on('data', (chunk) => stdout += chunk.toString());
 
   child.once('close', (code) => {
     const elapsed = process.hrtime(benchmarker_start);

diff --git a/benchmark/_test-double-benchmarker.js b/benchmark/_test-double-benchmarker.js
@@ -7,7 +7,7 @@ if (!['http', 'http2'].includes(myModule)) {
 
 const http = require(myModule);
 
-const duration = +process.env.duration;
+const duration = process.env.duration || 0;
 const url = process.env.test_url;
 
 const start = process.hrtime();
@@ -18,15 +18,13 @@ function request(res, client) {
   res.on('error', () => {});
   res.on('end', () => {
     throughput++;
-    const [sec, nanosec] = process.hrtime(start);
-    const ms = sec * 1000 + nanosec / 1e6;
-    if (ms < duration * 1000) {
+    const diff = process.hrtime(start);
+    if (duration > 0 && diff[0] < duration) {
       run();
     } else {
       console.log(JSON.stringify({ throughput }));
       if (client) {
         client.destroy();
-        process.exit(0);
       }
     }
   });
@@ -35,7 +33,7 @@ function request(res, client) {
 function run() {
   if (http.get) { // HTTP
     http.get(url, request);
-  } else { // HTTP/2
+  } else {        // HTTP/2
     const client = http.connect(url);
     client.on('error', (e) => { throw e; });
     request(client.request(), client);

diff --git a/benchmark/async_hooks/http-server.js b/benchmark/async_hooks/http-server.js
@@ -3,11 +3,10 @@ const common = require('../common.js');
 
 const bench = common.createBenchmark(main, {
   asyncHooks: ['init', 'before', 'after', 'all', 'disabled', 'none'],
-  connections: [50, 500],
-  duration: 5
+  connections: [50, 500]
 });
 
-function main({ asyncHooks, connections, duration }) {
+function main({ asyncHooks, connections }) {
   if (asyncHooks !== 'none') {
     let hooks = {
       init() {},
@@ -34,7 +33,6 @@ function main({ asyncHooks, connections, duration }) {
       bench.http({
         connections,
         path,
-        duration
       }, () => {
         server.close();
       });

diff --git a/benchmark/buffers/buffer-base64-encode.js b/benchmark/buffers/buffer-base64-encode.js
@@ -25,8 +25,6 @@ const common = require('../common.js');
 const bench = common.createBenchmark(main, {
   len: [64 * 1024 * 1024],
   n: [32]
-}, {
-  test: { len: 256 }
 });
 
 function main({ n, len }) {

diff --git a/benchmark/buffers/buffer-swap.js b/benchmark/buffers/buffer-swap.js
@@ -7,8 +7,6 @@ const bench = common.createBenchmark(main, {
   method: ['swap16', 'swap32', 'swap64'/* , 'htons', 'htonl', 'htonll' */],
   len: [64, 256, 768, 1024, 2056, 8192],
   n: [1e6]
-}, {
-  test: { len: 16 }
 });
 
 // The htons and htonl methods below are used to benchmark the