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>

1 files changed, 102 insertions, 142 deletions

diff --git a/lib/timers.js b/lib/timers.js
index 3c522e76f1..d0828669d7 100644
--- a/lib/timers.js
+++ b/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) {
-    args = [arg1];
-  } else if (len === 4) {
-    args = [arg1, arg2];
-  } else if (len > 4) {
-    args = [arg1, arg2, arg3];
-    for (var i = 5; i < len; i++)
-      // extend array dynamically, makes .apply run much faster in v6.0.0
-      args[i - 2] = arguments[i];
+  var i, args;
+  switch (arguments.length) {
+    // fast cases
+    case 1:
+    case 2:
+      break;
+    case 3:
+      args = [arg1];
+      break;
+    case 4:
+      args = [arg1, arg2];
+      break;
+    default:
+      args = [arg1, arg2, arg3];
+      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) {
-    args = [arg1];
-  } else if (len === 4) {
-    args = [arg1, arg2];
-  } else if (len > 4) {
-    args = [arg1, arg2, arg3];
-    for (var i = 5; i < len; i++)
-      // extend array dynamically, makes .apply run much faster in v6.0.0
-      args[i - 2] = arguments[i];
+  var i, args;
+  switch (arguments.length) {
+    // fast cases
+    case 1:
+    case 2:
+      break;
+    case 3:
+      args = [arg1];
+      break;
+    case 4:
+      args = [arg1, arg2];
+      break;
+    default:
+      args = [arg1, arg2, arg3];
+      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:
-      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);
-  }
+  if (!argv)
+    return timer._onImmediate();
+  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;