timers: refactor timers

Consolidates the implementation of regular and internal (_unrefActive)
timers.

Also includes a couple optimizations:
- Isolates the try/catch from listOnTimeout() in a new tryOnTimeout().
- Uses a TimersList constructor as the base for linkedlists.

Additionally includes other cleanup and clarification, such as a rename
of "Timer" to "TimerWrap".

PR-URL: https://github.com/nodejs/node/pull/4007
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>

lib/timers.js

@@ -1,16 +1,18 @@
 'use strict';
 
-const Timer = process.binding('timer_wrap').Timer;
+const TimerWrap = process.binding('timer_wrap').Timer;
 const L = require('internal/linkedlist');
-const assert = require('assert').ok;
+const assert = require('assert');
 const util = require('util');
 const debug = util.debuglog('timer');
-const kOnTimeout = Timer.kOnTimeout | 0;
+const kOnTimeout = TimerWrap.kOnTimeout | 0;
 
 // Timeout values > TIMEOUT_MAX are set to 1.
 const TIMEOUT_MAX = 2147483647; // 2^31-1
 
 // IDLE TIMEOUTS
+// Object maps containing linked lists of timers, keyed and sorted by their
+// duration in milliseconds.
 //
 // Because often many sockets will have the same idle timeout we will not
 // use one timeout watcher per item. It is too much overhead.  Instead
@@ -18,117 +20,151 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1
 // and a linked list. This technique is described in the libev manual:
 // http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Be_smart_about_timeouts
 
-// Object containing all lists, timers
+const refedLists = {};
-// key = time in milliseconds
+const unrefedLists = {};
-// value = list
-var lists = {};
 
 
-// call this whenever the item is active (not idle)
+// Schedule or re-schedule a timer.
-// it will reset its timeout.
+// The item must have been enroll()'d first.
-// the main function - creates lists on demand and the watchers associated
-// with them.
 exports.active = function(item) {
+  insert(item, false);
+};
+
+// Internal APIs that need timeouts should use `_unrefActive()` instead of
+// `active()` so that they do not unnecessarily keep the process open.
+exports._unrefActive = function(item) {
+  insert(item, true);
+};
+
+
+// The underlying logic for scheduling or re-scheduling a timer.
+function insert(item, unrefed) {
   const msecs = item._idleTimeout;
   if (msecs < 0 || msecs === undefined) return;
 
-  item._idleStart = Timer.now();
+  item._idleStart = TimerWrap.now();
 
-  var list;
+  const lists = unrefed === true ? unrefedLists : refedLists;
-
-  if (lists[msecs]) {
-    list = lists[msecs];
-  } else {
-    list = new Timer();
-    list.start(msecs, 0);
 
+  var list = lists[msecs];
+  if (!list) {
+    debug('no %d list was found in insert, creating a new one', msecs);
+    list = new TimersList(msecs, unrefed);
     L.init(list);
+    list._timer._list = list;
+
+    if (unrefed === true) list._timer.unref();
+    list._timer.start(msecs, 0);
 
     lists[msecs] = list;
-    list.msecs = msecs;
+    list._timer[kOnTimeout] = listOnTimeout;
-    list[kOnTimeout] = listOnTimeout;
   }
 
   L.append(list, item);
   assert(!L.isEmpty(list)); // list is not empty
-};
+}
+
+function TimersList(msecs, unrefed) {
+  this._idleNext = null; // Create the list with the linkedlist properties to
+  this._idlePrev = null; // prevent any unnecessary hidden class changes.
+  this._timer = new TimerWrap();
+  this._unrefed = unrefed;
+  this.msecs = msecs;
+}
 
 function listOnTimeout() {
-  var msecs = this.msecs;
+  var list = this._list;
-  var list = this;
+  var msecs = list.msecs;
 
   debug('timeout callback %d', msecs);
 
-  var now = Timer.now();
+  var now = TimerWrap.now();
   debug('now: %s', now);
 
-  var diff, first, threw;
+  var diff, timer;
-  while (first = L.peek(list)) {
+  while (timer = L.peek(list)) {
-    diff = now - first._idleStart;
+    diff = now - timer._idleStart;
+
     if (diff < msecs) {
-      list.start(msecs - diff, 0);
+      this.start(msecs - diff, 0);
       debug('%d list wait because diff is %d', msecs, diff);
       return;
-    } else {
+    }
-      L.remove(first);
-      assert(first !== L.peek(list));
 
-      if (!first._onTimeout) continue;
 
-      // v0.4 compatibility: if the timer callback throws and the
+    L.remove(timer);
+    assert(timer !== L.peek(list));
+
+    if (!timer._onTimeout) continue;
+
+    var domain = timer.domain;
+    if (domain) {
+
+      // If the timer callback throws and the
       // domain or uncaughtException handler ignore the exception,
       // other timers that expire on this tick should still run.
       //
-      // https://github.com/joyent/node/issues/2631
+      // https://github.com/nodejs/node-v0.x-archive/issues/2631
-      var domain = first.domain;
+      if (domain._disposed)
-      if (domain && domain._disposed)
         continue;
 
-      try {
+      domain.enter();
-        if (domain)
-          domain.enter();
-        threw = true;
-        first._called = true;
-        first._onTimeout();
-        if (domain)
-          domain.exit();
-        threw = false;
-      } finally {
-        if (threw) {
-          // We need to continue processing after domain error handling
-          // is complete, but not by using whatever domain was left over
-          // when the timeout threw its exception.
-          var oldDomain = process.domain;
-          process.domain = null;
-          process.nextTick(listOnTimeoutNT, list);
-          process.domain = oldDomain;
-        }
-      }
     }
+
+    tryOnTimeout(timer, list);
+
+    if (domain)
+      domain.exit();
   }
 
   debug('%d list empty', msecs);
   assert(L.isEmpty(list));
-  list.close();
+  this.close();
-  delete lists[msecs];
+  if (list._unrefed === true) {
+    delete unrefedLists[msecs];
+  } else {
+    delete refedLists[msecs];
+  }
+}
+
+
+// An optimization so that the try/finally only de-optimizes what is in this
+// smaller function.
+function tryOnTimeout(timer, list) {
+  timer._called = true;
+  var threw = true;
+  try {
+    timer._onTimeout();
+    threw = false;
+  } finally {
+    if (!threw) return;
+
+    // We need to continue processing after domain error handling
+    // is complete, but not by using whatever domain was left over
+    // when the timeout threw its exception.
+    const domain = process.domain;
+    process.domain = null;
+    process.nextTick(listOnTimeoutNT, list);
+    process.domain = domain;
+  }
 }
 
 
 function listOnTimeoutNT(list) {
-  list[kOnTimeout]();
+  list._timer[kOnTimeout]();
 }
 
 
 function reuse(item) {
   L.remove(item);
 
-  var list = lists[item._idleTimeout];
+  var list = refedLists[item._idleTimeout];
   // if empty - reuse the watcher
   if (list && L.isEmpty(list)) {
     debug('reuse hit');
-    list.stop();
+    list._timer.stop();
-    delete lists[item._idleTimeout];
+    delete refedLists[item._idleTimeout];
-    return list;
+    return list._timer;
   }
 
   return null;
@@ -136,10 +172,10 @@ function reuse(item) {
 
 
 const unenroll = exports.unenroll = function(item) {
-  var list = reuse(item);
+  var handle = reuse(item);
-  if (list) {
+  if (handle) {
     debug('unenroll: list empty');
-    list.close();
+    handle.close();
   }
   // if active is called later, then we want to make sure not to insert again
   item._idleTimeout = -1;
@@ -328,7 +364,7 @@ Timeout.prototype.unref = function() {
   if (this._handle) {
     this._handle.unref();
   } else if (typeof this._onTimeout === 'function') {
-    var now = Timer.now();
+    var now = TimerWrap.now();
     if (!this._idleStart) this._idleStart = now;
     var delay = this._idleStart + this._idleTimeout - now;
     if (delay < 0) delay = 0;
@@ -341,7 +377,7 @@ Timeout.prototype.unref = function() {
 
     var handle = reuse(this);
 
-    this._handle = handle || new Timer();
+    this._handle = handle || new TimerWrap();
     this._handle.owner = this;
     this._handle[kOnTimeout] = unrefdHandle;
     this._handle.start(delay, 0);
@@ -495,152 +531,3 @@ exports.clearImmediate = function(immediate) {
     process._needImmediateCallback = false;
   }
 };
-
-
-// Internal APIs that need timeouts should use timers._unrefActive instead of
-// timers.active as internal timeouts shouldn't hold the loop open
-
-var unrefList, unrefTimer;
-
-function _makeTimerTimeout(timer) {
-  var domain = timer.domain;
-  var msecs = timer._idleTimeout;
-
-  L.remove(timer);
-
-  // Timer has been unenrolled by another timer that fired at the same time,
-  // so don't make it timeout.
-  if (msecs <= 0)
-    return;
-
-  if (!timer._onTimeout)
-    return;
-
-  if (domain) {
-    if (domain._disposed)
-      return;
-
-    domain.enter();
-  }
-
-  debug('unreftimer firing timeout');
-  timer._called = true;
-  _runOnTimeout(timer);
-
-  if (domain)
-    domain.exit();
-}
-
-function _runOnTimeout(timer) {
-  var threw = true;
-  try {
-    timer._onTimeout();
-    threw = false;
-  } finally {
-    if (threw) process.nextTick(unrefTimeout);
-  }
-}
-
-function unrefTimeout() {
-  var now = Timer.now();
-
-  debug('unrefTimer fired');
-
-  var timeSinceLastActive;
-  var nextTimeoutTime;
-  var nextTimeoutDuration;
-  var minNextTimeoutTime = TIMEOUT_MAX;
-  var timersToTimeout = [];
-
-  // The actual timer fired and has not yet been rearmed,
-  // let's consider its next firing time is invalid for now.
-  // It may be set to a relevant time in the future once
-  // we scanned through the whole list of timeouts and if
-  // we find a timeout that needs to expire.
-  unrefTimer.when = -1;
-
-  // Iterate over the list of timeouts,
-  // call the onTimeout callback for those expired,
-  // and rearm the actual timer if the next timeout to expire
-  // will expire before the current actual timer.
-  var cur = unrefList._idlePrev;
-  while (cur !== unrefList) {
-    timeSinceLastActive = now - cur._idleStart;
-
-    if (timeSinceLastActive < cur._idleTimeout) {
-      // This timer hasn't expired yet, but check if its expiring time is
-      // earlier than the actual timer's expiring time
-
-      nextTimeoutDuration = cur._idleTimeout - timeSinceLastActive;
-      nextTimeoutTime = now + nextTimeoutDuration;
-      if (minNextTimeoutTime === TIMEOUT_MAX ||
-          (nextTimeoutTime < minNextTimeoutTime)) {
-        // We found a timeout that will expire earlier,
-        // store its next timeout time now so that we
-        // can rearm the actual timer accordingly when
-        // we scanned through the whole list.
-        minNextTimeoutTime = nextTimeoutTime;
-      }
-    } else {
-      // We found a timer that expired. Do not call its _onTimeout callback
-      // right now, as it could mutate any item of the list (including itself).
-      // Instead, add it to another list that will be processed once the list
-      // of current timers has been fully traversed.
-      timersToTimeout.push(cur);
-    }
-
-    cur = cur._idlePrev;
-  }
-
-  var nbTimersToTimeout = timersToTimeout.length;
-  for (var timerIdx = 0; timerIdx < nbTimersToTimeout; ++timerIdx)
-    _makeTimerTimeout(timersToTimeout[timerIdx]);
-
-
-  // Rearm the actual timer with the timeout delay
-  // of the earliest timeout found.
-  if (minNextTimeoutTime !== TIMEOUT_MAX) {
-    unrefTimer.start(minNextTimeoutTime - now, 0);
-    unrefTimer.when = minNextTimeoutTime;
-    debug('unrefTimer rescheduled');
-  } else if (L.isEmpty(unrefList)) {
-    debug('unrefList is empty');
-  }
-}
-
-
-exports._unrefActive = function(item) {
-  var msecs = item._idleTimeout;
-  if (!msecs || msecs < 0) return;
-  assert(msecs >= 0);
-
-  L.remove(item);
-
-  if (!unrefList) {
-    debug('unrefList initialized');
-    unrefList = {};
-    L.init(unrefList);
-
-    debug('unrefTimer initialized');
-    unrefTimer = new Timer();
-    unrefTimer.unref();
-    unrefTimer.when = -1;
-    unrefTimer[kOnTimeout] = unrefTimeout;
-  }
-
-  var now = Timer.now();
-  item._idleStart = now;
-
-  var when = now + msecs;
-
-  // If the actual timer is set to fire too late, or not set to fire at all,
-  // we need to make it fire earlier
-  if (unrefTimer.when === -1 || unrefTimer.when > when) {
-    unrefTimer.start(msecs, 0);
-    unrefTimer.when = when;
-    debug('unrefTimer scheduled');
-  }
-
-  debug('unrefList append to end');
-  L.append(unrefList, item);
-};

test/message/timeout_throw.out

@@ -3,4 +3,5 @@
   ^
 ReferenceError: undefined_reference_error_maker is not defined
     at null._onTimeout (*test*message*timeout_throw.js:*:*)
+    at tryOnTimeout (timers.js:*:*)
     at Timer.listOnTimeout (timers.js:*:*)