Avoid linear scan of timers when calling _unrefActive.

lib/timers.js

@@ -470,47 +470,85 @@ var unrefList, unrefTimer;
 function unrefTimeout() {
   var now = Timer.now();
 
-  debug('unrefTimer fired');
+  var timeSinceLastActive,
+      nextTimeoutTime,
+      nextTimeoutDuration,
+      minNextTimeoutTime;
+
+  var domain, hasQueue, threw;
+  var itemToDelete;
+
+  // 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;
 
-  var diff, domain, first, hasQueue, threw;
-  while (first = L.peek(unrefList)) {
-    diff = now - first._idleStart;
+  // 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) {
+      nextTimeoutDuration = cur._idleTimeout - timeSinceLastActive;
+      nextTimeoutTime = now + nextTimeoutDuration;
+      if (minNextTimeoutTime == null ||
+          (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;
+      }
 
-    if (diff < first._idleTimeout) {
-      diff = first._idleTimeout - diff;
-      unrefTimer.start(diff, 0);
-      unrefTimer.when = now + diff;
-      debug('unrefTimer rescheudling for later');
-      return;
-    }
+      cur = cur._idlePrev;
+    } else {
+      try {
+        domain = cur.domain;
+        if (!cur._onTimeout) continue;
+        if (domain && domain._disposed) continue;
+        hasQueue = !!cur._asyncQueue;
 
-    L.remove(first);
+        if (hasQueue)
+          loadAsyncQueue(cur);
+        if (domain) domain.enter();
 
-    domain = first.domain;
+        threw = true;
 
-    if (!first._onTimeout) continue;
-    if (domain && domain._disposed) continue;
-    hasQueue = !!first._asyncQueue;
+        itemToDelete = cur;
+        // Move to the previous item before calling the _onTimeout callback,
+        // as it can mutate the list.
+        cur = cur._idlePrev;
 
-    try {
-      if (hasQueue)
-        loadAsyncQueue(first);
-      if (domain) domain.enter();
-      threw = true;
-      debug('unreftimer firing timeout');
-      first._onTimeout();
-      threw = false;
-      if (domain)
-        domain.exit();
-      if (hasQueue)
-        unloadAsyncQueue(first);
-    } finally {
-      if (threw) process.nextTick(unrefTimeout);
+        // Remove the timeout from the list because it expired.
+        L.remove(itemToDelete);
+
+        debug('unreftimer firing timeout');
+        itemToDelete._onTimeout();
+
+        threw = false;
+
+        if (domain)
+          domain.exit();
+        if (hasQueue)
+          unloadAsyncQueue(cur);
+      } finally {
+        if (threw) process.nextTick(unrefTimeout);
+      }
     }
   }
 
-  debug('unrefList is empty');
-  unrefTimer.when = -1;
+  // Rearm the actual timer with the timeout delay
+  // of the earliest timeout found.
+  if (minNextTimeoutTime != null) {
+    unrefTimer.start(minNextTimeoutTime - now, 0);
+    unrefTimer.when = minNextTimeoutTime;
+    debug('unrefTimer rescheduled');
+  }
 }
 
 
@@ -536,38 +574,14 @@ exports._unrefActive = function(item) {
   var now = Timer.now();
   item._idleStart = now;
 
-  if (L.isEmpty(unrefList)) {
-    debug('unrefList empty');
-    L.append(unrefList, item);
+  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 = now + msecs;
+    unrefTimer.when = when;
     debug('unrefTimer scheduled');
-    return;
-  }
-
-  var when = now + msecs;
-
-  debug('unrefList find where we can insert');
-
-  var cur, them;
-
-  for (cur = unrefList._idlePrev; cur != unrefList; cur = cur._idlePrev) {
-    them = cur._idleStart + cur._idleTimeout;
-
-    if (when < them) {
-      debug('unrefList inserting into middle of list');
-
-      L.append(cur, item);
-
-      if (unrefTimer.when > when) {
-        debug('unrefTimer is scheduled to fire too late, reschedule');
-        unrefTimer.start(msecs, 0);
-        unrefTimer.when = when;
-      }
-
-      return;
-    }
   }
 
   debug('unrefList append to end');

test/simple/test-timers-unref-active.js

@@ -0,0 +1,42 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var timers = require('timers');
+var assert = require('assert');
+
+var someObject = {};
+var nbTimeouts = 0;
+
+timers.unenroll(someObject);
+timers.enroll(someObject, 100);
+
+someObject._onTimeout = function _onTimeout() {
+  ++nbTimeouts;
+  timers._unrefActive(someObject);
+}
+
+function startTimer() { timers._unrefActive(someObject); }
+
+startTimer();
+
+setTimeout(function () {
+    assert(nbTimeouts === 10);
+}, 1050);