timers: clean up for readability

Remove micro-optimizations that no longer yield any benefits,
restructure timers & immediates to be a bit more straightforward.

Adjust timers benchmarks to run long enough to offer meaningful data.

PR-URL: https://github.com/nodejs/node/pull/17279
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

benchmark/timers/immediate.js

@@ -2,7 +2,7 @@
 const common = require('../common.js');
 
 const bench = common.createBenchmark(main, {
-  thousands: [2000],
+  thousands: [5000],
   type: ['depth', 'depth1', 'breadth', 'breadth1', 'breadth4', 'clear']
 });
 
@@ -88,6 +88,7 @@ function breadth1(N) {
 
 // concurrent setImmediate, 4 arguments
 function breadth4(N) {
+  N /= 2;
   var n = 0;
   bench.start();
   function cb(a1, a2, a3, a4) {
@@ -101,6 +102,7 @@ function breadth4(N) {
 }
 
 function clear(N) {
+  N *= 4;
   bench.start();
   function cb(a1) {
     if (a1 === 2)

benchmark/timers/set-immediate-breadth-args.js

@@ -19,9 +19,9 @@ function main(conf) {
   bench.start();
   for (let i = 0; i < N; i++) {
     if (i % 3 === 0)
-      setImmediate(cb3, 512, true, null);
+      setImmediate(cb3, 512, true, null, 512, true, null);
     else if (i % 2 === 0)
-      setImmediate(cb2, false, 5.1);
+      setImmediate(cb2, false, 5.1, 512);
     else
       setImmediate(cb1, 0);
   }

benchmark/timers/set-immediate-depth-args.js

@@ -2,7 +2,7 @@
 
 const common = require('../common.js');
 const bench = common.createBenchmark(main, {
-  millions: [10]
+  millions: [5]
 });
 
 function main(conf) {
@@ -15,9 +15,9 @@ function main(conf) {
   function cb3(n, arg2, arg3) {
     if (--n) {
       if (n % 3 === 0)
-        setImmediate(cb3, n, true, null);
+        setImmediate(cb3, n, true, null, 5.1, null, true);
       else if (n % 2 === 0)
-        setImmediate(cb2, n, 5.1);
+        setImmediate(cb2, n, 5.1, true);
       else
         setImmediate(cb1, n);
     }
@@ -25,9 +25,9 @@ function main(conf) {
   function cb2(n, arg2) {
     if (--n) {
       if (n % 3 === 0)
-        setImmediate(cb3, n, true, null);
+        setImmediate(cb3, n, true, null, 5.1, null, true);
       else if (n % 2 === 0)
-        setImmediate(cb2, n, 5.1);
+        setImmediate(cb2, n, 5.1, true);
       else
         setImmediate(cb1, n);
     }
@@ -35,9 +35,9 @@ function main(conf) {
   function cb1(n) {
     if (--n) {
       if (n % 3 === 0)
-        setImmediate(cb3, n, true, null);
+        setImmediate(cb3, n, true, null, 5.1, null, true);
       else if (n % 2 === 0)
-        setImmediate(cb2, n, 5.1);
+        setImmediate(cb2, n, 5.1, true);
       else
         setImmediate(cb1, n);
     }

benchmark/timers/timers-breadth.js

@@ -2,7 +2,7 @@
 const common = require('../common.js');
 
 const bench = common.createBenchmark(main, {
-  thousands: [500],
+  thousands: [5000],
 });
 
 function main(conf) {

benchmark/timers/timers-cancel-pooled.js

@@ -3,11 +3,11 @@ const common = require('../common.js');
 const assert = require('assert');
 
 const bench = common.createBenchmark(main, {
-  thousands: [500],
+  millions: [5],
 });
 
 function main(conf) {
-  const iterations = +conf.thousands * 1e3;
+  const iterations = +conf.millions * 1e6;
 
   var timer = setTimeout(() => {}, 1);
   for (var i = 0; i < iterations; i++) {
@@ -24,7 +24,7 @@ function main(conf) {
     clearTimeout(timer);
   }
 
-  bench.end(iterations / 1e3);
+  bench.end(iterations / 1e6);
 }
 
 function cb() {

benchmark/timers/timers-cancel-unpooled.js

@@ -3,11 +3,11 @@ const common = require('../common.js');
 const assert = require('assert');
 
 const bench = common.createBenchmark(main, {
-  thousands: [100],
+  millions: [1],
 });
 
 function main(conf) {
-  const iterations = +conf.thousands * 1e3;
+  const iterations = +conf.millions * 1e6;
 
   const timersList = [];
   for (var i = 0; i < iterations; i++) {
@@ -18,7 +18,7 @@ function main(conf) {
   for (var j = 0; j < iterations + 1; j++) {
     clearTimeout(timersList[j]);
   }
-  bench.end(iterations / 1e3);
+  bench.end(iterations / 1e6);
 }
 
 function cb() {

benchmark/timers/timers-insert-pooled.js

@@ -2,11 +2,11 @@
 const common = require('../common.js');
 
 const bench = common.createBenchmark(main, {
-  thousands: [500],
+  millions: [5],
 });
 
 function main(conf) {
-  const iterations = +conf.thousands * 1e3;
+  const iterations = +conf.millions * 1e6;
 
   bench.start();
 
@@ -14,5 +14,5 @@ function main(conf) {
     setTimeout(() => {}, 1);
   }
 
-  bench.end(iterations / 1e3);
+  bench.end(iterations / 1e6);
 }

benchmark/timers/timers-insert-unpooled.js

@@ -3,11 +3,11 @@ const common = require('../common.js');
 const assert = require('assert');
 
 const bench = common.createBenchmark(main, {
-  thousands: [100],
+  millions: [1],
 });
 
 function main(conf) {
-  const iterations = +conf.thousands * 1e3;
+  const iterations = +conf.millions * 1e6;
 
   const timersList = [];
 
@@ -15,7 +15,7 @@ function main(conf) {
   for (var i = 0; i < iterations; i++) {
     timersList.push(setTimeout(cb, i + 1));
   }
-  bench.end(iterations / 1e3);
+  bench.end(iterations / 1e6);
 
   for (var j = 0; j < iterations + 1; j++) {
     clearTimeout(timersList[j]);

benchmark/timers/timers-timeout-pooled.js

@@ -1,23 +1,35 @@
 'use strict';
 const common = require('../common.js');
 
+// The following benchmark measures setting up n * 1e6 timeouts,
+// which then get executed on the next uv tick
+
 const bench = common.createBenchmark(main, {
-  thousands: [500],
+  millions: [10],
 });
 
 function main(conf) {
-  const iterations = +conf.thousands * 1e3;
-  var count = 0;
+  const iterations = +conf.millions * 1e6;
+  let count = 0;
 
-  for (var i = 0; i < iterations; i++) {
-    setTimeout(cb, 1);
-  }
-
-  bench.start();
+  // Function tracking on the hidden class in V8 can cause misleading
+  // results in this benchmark if only a single function is used —
+  // alternate between two functions for a fairer benchmark
 
   function cb() {
     count++;
     if (count === iterations)
-      bench.end(iterations / 1e3);
+      bench.end(iterations / 1e6);
   }
+  function cb2() {
+    count++;
+    if (count === iterations)
+      bench.end(iterations / 1e6);
+  }
+
+  for (var i = 0; i < iterations; i++) {
+    setTimeout(i % 2 ? cb : cb2, 1);
+  }
+
+  bench.start();
 }

lib/timers.js

@@ -182,10 +182,12 @@ function insert(item, unrefed) {
     item._destroyed = false;
     item[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
     item[trigger_async_id_symbol] = initTriggerId();
-    if (async_hook_fields[kInit] > 0)
-      emitInit(
-        item[async_id_symbol], 'Timeout', item[trigger_async_id_symbol], item
-      );
+    if (async_hook_fields[kInit] > 0) {
+      emitInit(item[async_id_symbol],
+               'Timeout',
+               item[trigger_async_id_symbol],
+               item);
+    }
   }
 
   L.append(list, item);
@@ -430,74 +432,47 @@ function setTimeout(callback, after, arg1, arg2, arg3) {
     throw new errors.TypeError('ERR_INVALID_CALLBACK');
   }
 
-  var len = arguments.length;
-  var args;
-  if (len === 3) {
+  var i, args;
+  switch (arguments.length) {
+    // fast cases
+    case 1:
+    case 2:
+      break;
+    case 3:
       args = [arg1];
-  } else if (len === 4) {
+      break;
+    case 4:
       args = [arg1, arg2];
-  } else if (len > 4) {
+      break;
+    default:
       args = [arg1, arg2, arg3];
-    for (var i = 5; i < len; i++)
+      for (i = 5; i < arguments.length; i++) {
         // extend array dynamically, makes .apply run much faster in v6.0.0
         args[i - 2] = arguments[i];
       }
+      break;
+  }
 
-  return createSingleTimeout(callback, after, args);
+  return new Timeout(callback, after, args, false);
 }
 
 setTimeout[internalUtil.promisify.custom] = function(after, value) {
   const promise = createPromise();
-  createSingleTimeout(promise, after, [value]);
+  new Timeout(promise, after, [value], false);
   return promise;
 };
 
 exports.setTimeout = setTimeout;
 
-function createSingleTimeout(callback, after, args) {
-  after *= 1; // coalesce to number or NaN
-  if (!(after >= 1 && after <= TIMEOUT_MAX)) {
-    if (after > TIMEOUT_MAX) {
-      process.emitWarning(`${after} does not fit into` +
-                          ' a 32-bit signed integer.' +
-                          '\nTimeout duration was set to 1.',
-                          'TimeoutOverflowWarning');
-    }
-    after = 1; // schedule on next tick, follows browser behavior
-  }
-
-  var timer = new Timeout(after, callback, args);
-  if (process.domain)
-    timer.domain = process.domain;
-
-  active(timer);
-
-  return timer;
-}
-
 
 function ontimeout(timer) {
   var args = timer._timerArgs;
-  var callback = timer._onTimeout;
-  if (typeof callback !== 'function')
-    return promiseResolve(callback, args[0]);
+  if (typeof timer._onTimeout !== 'function')
+    return promiseResolve(timer._onTimeout, args[0]);
   if (!args)
     timer._onTimeout();
-  else {
-    switch (args.length) {
-      case 1:
-        timer._onTimeout(args[0]);
-        break;
-      case 2:
-        timer._onTimeout(args[0], args[1]);
-        break;
-      case 3:
-        timer._onTimeout(args[0], args[1], args[2]);
-        break;
-      default:
-        Function.prototype.apply.call(callback, timer, args);
-    }
-  }
+  else
+    Reflect.apply(timer._onTimeout, timer, args);
   if (timer._repeat)
     rearm(timer);
 }
@@ -534,44 +509,30 @@ exports.setInterval = function(callback, repeat, arg1, arg2, arg3) {
     throw new errors.TypeError('ERR_INVALID_CALLBACK');
   }
 
-  var len = arguments.length;
-  var args;
-  if (len === 3) {
+  var i, args;
+  switch (arguments.length) {
+    // fast cases
+    case 1:
+    case 2:
+      break;
+    case 3:
       args = [arg1];
-  } else if (len === 4) {
+      break;
+    case 4:
       args = [arg1, arg2];
-  } else if (len > 4) {
+      break;
+    default:
       args = [arg1, arg2, arg3];
-    for (var i = 5; i < len; i++)
+      for (i = 5; i < arguments.length; i++) {
         // extend array dynamically, makes .apply run much faster in v6.0.0
         args[i - 2] = arguments[i];
       }
+      break;
+  }
 
-  return createRepeatTimeout(callback, repeat, args);
+  return new Timeout(callback, repeat, args, true);
 };
 
-function createRepeatTimeout(callback, repeat, args) {
-  repeat *= 1; // coalesce to number or NaN
-  if (!(repeat >= 1 && repeat <= TIMEOUT_MAX)) {
-    if (repeat > TIMEOUT_MAX) {
-      process.emitWarning(`${repeat} does not fit into` +
-                          ' a 32-bit signed integer.' +
-                          '\nInterval duration was set to 1.',
-                          'TimeoutOverflowWarning');
-    }
-    repeat = 1; // schedule on next tick, follows browser behavior
-  }
-
-  var timer = new Timeout(repeat, callback, args);
-  timer._repeat = repeat;
-  if (process.domain)
-    timer.domain = process.domain;
-
-  active(timer);
-
-  return timer;
-}
-
 exports.clearInterval = function(timer) {
   if (timer && timer._repeat) {
     timer._repeat = null;
@@ -580,22 +541,41 @@ exports.clearInterval = function(timer) {
 };
 
 
-function Timeout(after, callback, args) {
+function Timeout(callback, after, args, isRepeat) {
+  after *= 1; // coalesce to number or NaN
+  if (!(after >= 1 && after <= TIMEOUT_MAX)) {
+    if (after > TIMEOUT_MAX) {
+      process.emitWarning(`${after} does not fit into` +
+                          ' a 32-bit signed integer.' +
+                          '\nTimeout duration was set to 1.',
+                          'TimeoutOverflowWarning');
+    }
+    after = 1; // schedule on next tick, follows browser behavior
+  }
+
   this._called = false;
   this._idleTimeout = after;
   this._idlePrev = this;
   this._idleNext = this;
   this._idleStart = null;
+  // this must be set to null first to avoid function tracking
+  // on the hidden class, revisit in V8 versions after 6.2
+  this._onTimeout = null;
   this._onTimeout = callback;
   this._timerArgs = args;
-  this._repeat = null;
+  this._repeat = isRepeat ? after : null;
   this._destroyed = false;
+
   this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
   this[trigger_async_id_symbol] = initTriggerId();
-  if (async_hook_fields[kInit] > 0)
-    emitInit(
-      this[async_id_symbol], 'Timeout', this[trigger_async_id_symbol], this
-    );
+  if (async_hook_fields[kInit] > 0) {
+    emitInit(this[async_id_symbol],
+             'Timeout',
+             this[trigger_async_id_symbol],
+             this);
+  }
+
+  active(this);
 }
 
 
@@ -653,9 +633,7 @@ Timeout.prototype.ref = function() {
 Timeout.prototype.close = function() {
   this._onTimeout = null;
   if (this._handle) {
-    // Fewer checks may be possible, but these cover everything.
     if (async_hook_fields[kDestroy] > 0 &&
-        this &&
         typeof this[async_id_symbol] === 'number' &&
         !this._destroyed) {
       emitDestroy(this[async_id_symbol]);
@@ -796,42 +774,38 @@ function tryOnImmediate(immediate, oldTail) {
 
 function runCallback(timer) {
   const argv = timer._argv;
-  const argc = argv ? argv.length : 0;
   if (typeof timer._onImmediate !== 'function')
     return promiseResolve(timer._onImmediate, argv[0]);
-  switch (argc) {
-    // fast-path callbacks with 0-3 arguments
-    case 0:
+  if (!argv)
     return timer._onImmediate();
-    case 1:
-      return timer._onImmediate(argv[0]);
-    case 2:
-      return timer._onImmediate(argv[0], argv[1]);
-    case 3:
-      return timer._onImmediate(argv[0], argv[1], argv[2]);
-    // more than 3 arguments run slower with .apply
-    default:
-      return Function.prototype.apply.call(timer._onImmediate, timer, argv);
-  }
+  Reflect.apply(timer._onImmediate, timer, argv);
 }
 
 
-function Immediate() {
-  // assigning the callback here can cause optimize/deoptimize thrashing
-  // so have caller annotate the object (node v6.0.0, v8 5.0.71.35)
+function Immediate(callback, args) {
   this._idleNext = null;
   this._idlePrev = null;
+  // this must be set to null first to avoid function tracking
+  // on the hidden class, revisit in V8 versions after 6.2
   this._onImmediate = null;
-  this._argv = null;
-  this._onImmediate = null;
+  this._onImmediate = callback;
+  this._argv = args;
   this._destroyed = false;
-  this.domain = process.domain;
+
   this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
   this[trigger_async_id_symbol] = initTriggerId();
-  if (async_hook_fields[kInit] > 0)
-    emitInit(
-      this[async_id_symbol], 'Immediate', this[trigger_async_id_symbol], this
-    );
+  if (async_hook_fields[kInit] > 0) {
+    emitInit(this[async_id_symbol],
+             'Immediate',
+             this[trigger_async_id_symbol],
+             this);
+  }
+
+  if (scheduledImmediateCount[0] === 0)
+    activateImmediateCheck();
+  scheduledImmediateCount[0]++;
+
+  immediateQueue.append(this);
 }
 
 function setImmediate(callback, arg1, arg2, arg3) {
@@ -840,7 +814,6 @@ function setImmediate(callback, arg1, arg2, arg3) {
   }
 
   var i, args;
-
   switch (arguments.length) {
     // fast cases
     case 1:
@@ -853,37 +826,24 @@ function setImmediate(callback, arg1, arg2, arg3) {
       break;
     default:
       args = [arg1, arg2, arg3];
-      for (i = 4; i < arguments.length; i++)
+      for (i = 4; i < arguments.length; i++) {
         // extend array dynamically, makes .apply run much faster in v6.0.0
         args[i - 1] = arguments[i];
+      }
       break;
   }
-  return createImmediate(args, callback);
+
+  return new Immediate(callback, args);
 }
 
 setImmediate[internalUtil.promisify.custom] = function(value) {
   const promise = createPromise();
-  createImmediate([value], promise);
+  new Immediate(promise, [value]);
   return promise;
 };
 
 exports.setImmediate = setImmediate;
 
-function createImmediate(args, callback) {
-  // declaring it `const immediate` causes v6.0.0 to deoptimize this function
-  var immediate = new Immediate();
-  immediate._argv = args;
-  immediate._onImmediate = callback;
-
-  if (scheduledImmediateCount[0] === 0)
-    activateImmediateCheck();
-  scheduledImmediateCount[0]++;
-
-  immediateQueue.append(immediate);
-
-  return immediate;
-}
-
 
 exports.clearImmediate = function(immediate) {
   if (!immediate) return;