benchmark: refactor console benchmark

Fix and refactor the console benchmark. It did not test console
so it got renamed and mainly tests the different ways of passing
arguments through.

PR-URL: https://github.com/nodejs/node/pull/17707
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

benchmark/misc/arguments.js

@@ -0,0 +1,59 @@
+'use strict';
+
+const { createBenchmark } = require('../common.js');
+const { format } = require('util');
+
+const methods = [
+  'restAndSpread',
+  'argumentsAndApply',
+  'restAndApply',
+  'predefined'
+];
+
+const bench = createBenchmark(main, {
+  method: methods,
+  n: [1e6]
+});
+
+function usingRestAndSpread(...args) {
+  format(...args);
+}
+
+function usingRestAndApply(...args) {
+  format.apply(null, args);
+}
+
+function usingArgumentsAndApply() {
+  format.apply(null, arguments);
+}
+
+function usingPredefined() {
+  format('part 1', 'part', 2, 'part 3', 'part', 4);
+}
+
+function main({ n, method, args }) {
+  var fn;
+  switch (method) {
+    // '' is a default case for tests
+    case '':
+    case 'restAndSpread':
+      fn = usingRestAndSpread;
+      break;
+    case 'restAndApply':
+      fn = usingRestAndApply;
+      break;
+    case 'argumentsAndApply':
+      fn = usingArgumentsAndApply;
+      break;
+    case 'predefined':
+      fn = usingPredefined;
+      break;
+    default:
+      throw new Error(`Unexpected method "${method}"`);
+  }
+
+  bench.start();
+  for (var i = 0; i < n; i++)
+    fn('part 1', 'part', 2, 'part 3', 'part', 4);
+  bench.end(n);
+}

benchmark/misc/console.js

@@ -1,126 +0,0 @@
-'use strict';
-
-const common = require('../common.js');
-const assert = require('assert');
-const Writable = require('stream').Writable;
-const util = require('util');
-
-const methods = [
-  'restAndSpread',
-  'argumentsAndApply',
-  'restAndApply',
-  'restAndConcat'
-];
-
-const bench = common.createBenchmark(main, {
-  method: methods,
-  concat: [1, 0],
-  n: [1000000]
-});
-
-const nullStream = createNullStream();
-
-function usingRestAndConcat(...args) {
-  nullStream.write(`this is ${args[0]} of ${args[1]}\n`);
-}
-
-function usingRestAndSpreadTS(...args) {
-  nullStream.write(`${util.format(...args)}\n`);
-}
-
-function usingRestAndApplyTS(...args) {
-  nullStream.write(`${util.format.apply(null, args)}\n`);
-}
-
-function usingArgumentsAndApplyTS() {
-  nullStream.write(`${util.format.apply(null, arguments)}\n`);
-}
-
-function usingRestAndSpreadC(...args) {
-  nullStream.write(`${util.format(...args)}\n`);
-}
-
-function usingRestAndApplyC(...args) {
-  nullStream.write(`${util.format.apply(null, args)}\n`);
-}
-
-function usingArgumentsAndApplyC() {
-  nullStream.write(`${util.format.apply(null, arguments)}\n`);
-}
-
-function runUsingRestAndConcat(n) {
-
-  var i = 0;
-  bench.start();
-  for (; i < n; i++)
-    usingRestAndConcat('a', 1);
-  bench.end(n);
-}
-
-function runUsingRestAndSpread(n, concat) {
-
-  const method = concat ? usingRestAndSpreadC : usingRestAndSpreadTS;
-
-  var i = 0;
-  bench.start();
-  for (; i < n; i++)
-    method('this is %s of %d', 'a', 1);
-  bench.end(n);
-}
-
-function runUsingRestAndApply(n, concat) {
-
-  const method = concat ? usingRestAndApplyC : usingRestAndApplyTS;
-
-  var i = 0;
-  bench.start();
-  for (; i < n; i++)
-    method('this is %s of %d', 'a', 1);
-  bench.end(n);
-}
-
-function runUsingArgumentsAndApply(n, concat) {
-
-  const method = concat ? usingArgumentsAndApplyC : usingArgumentsAndApplyTS;
-
-  var i = 0;
-  bench.start();
-  for (; i < n; i++)
-    method('this is %s of %d', 'a', 1);
-  bench.end(n);
-}
-
-function main(conf) {
-  const n = +conf.n;
-  switch (conf.method) {
-    // '' is a default case for tests
-    case '':
-    case 'restAndSpread':
-      runUsingRestAndSpread(n, conf.concat);
-      break;
-    case 'restAndApply':
-      runUsingRestAndApply(n, conf.concat);
-      break;
-    case 'argumentsAndApply':
-      runUsingArgumentsAndApply(n, conf.concat);
-      break;
-    case 'restAndConcat':
-      if (conf.concat)
-        runUsingRestAndConcat(n);
-      break;
-    default:
-      throw new Error('Unexpected method');
-  }
-}
-
-function createNullStream() {
-  // Used to approximate /dev/null
-  function NullStream() {
-    Writable.call(this, {});
-  }
-  util.inherits(NullStream, Writable);
-  NullStream.prototype._write = function(cb) {
-    assert.strictEqual(cb.toString(), 'this is a of 1\n');
-  };
-  return new NullStream();
-}

lib/console.js

@@ -132,12 +132,12 @@ function write(ignoreErrors, stream, string, errorhandler, groupIndent) {
 }
 
 
-// As of v8 5.0.71.32, the combination of rest param, template string
-// and .apply(null, args) benchmarks consistently faster than using
-// the spread operator when calling util.format.
 Console.prototype.log = function log(...args) {
   write(this._ignoreErrors,
         this._stdout,
+        // The performance of .apply and the spread operator seems on par in V8
+        // 6.3 but the spread operator, unlike .apply(), pushes the elements
+        // onto the stack. That is, it makes stack overflows more likely.
         util.format.apply(null, args),
         this._stdoutErrorHandler,
         this[kGroupIndent]);