timers: optimize same-tick unref

lib/timers.js

@@ -97,30 +97,44 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1
 const refedLists = {};
 const unrefedLists = {};
 
+const queuedTimers = L.create();
+let insertNTScheduled = false;
 
 // Schedule or re-schedule a timer.
 // The item must have been enroll()'d first.
 const active = exports.active = function(item) {
-  insert(item, false);
+  L.append(queuedTimers, [item, TimerWrap.now()]);
+  if (!insertNTScheduled) {
+    process.nextTick(insertNT);
+  }
 };
 
 // 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);
+  insert(item, true, TimerWrap.now());
 };
 
 
+function insertNT() {
+  insertNTScheduled = false;
+  while (!L.isEmpty(queuedTimers)) {
+    const timer = L.shift(queuedTimers);
+    insert(timer[0], timer[0]._unrefed, timer[1]);
+  }
+}
+
+
 // The underlying logic for scheduling or re-scheduling a timer.
 //
 // Appends a timer onto the end of an existing timers list, or creates a new
 // TimerWrap backed list if one does not already exist for the specified timeout
 // duration.
-function insert(item, unrefed) {
+function insert(item, unrefed, now) {
   const msecs = item._idleTimeout;
   if (msecs < 0 || msecs === undefined) return;
 
-  item._idleStart = TimerWrap.now();
+  item._idleStart = now;
 
   const lists = unrefed === true ? unrefedLists : refedLists;
 
@@ -455,46 +469,59 @@ function Timeout(after) {
   this._idleStart = null;
   this._onTimeout = null;
   this._repeat = null;
+  this._unrefed = false;
 }
 
-
-function unrefdHandle() {
+function ownHandle() {
   this.owner._onTimeout();
   if (!this.owner._repeat)
     this.owner.close();
 }
 
+function createOwnHandle() {
+  var now = TimerWrap.now();
+  if (!this._idleStart) this._idleStart = now;
+  var delay = this._idleStart + this._idleTimeout - now;
+  if (delay < 0) delay = 0;
+
+  // Prevent running cb again when unref() is called during the same cb
+  if (this._called && !this._repeat) {
+    unenroll(this);
+    return;
+  }
+
+  var handle = reuse(this);
+
+  this._handle = handle || new TimerWrap();
+  this._handle.owner = this;
+  this._handle[kOnTimeout] = ownHandle;
+  this._handle.start(delay);
+  this._handle.domain = this.domain;
+}
 
 Timeout.prototype.unref = function() {
+  this._unrefed = true;
   if (this._handle) {
     this._handle.unref();
-  } else if (typeof this._onTimeout === 'function') {
-    var now = TimerWrap.now();
-    if (!this._idleStart) this._idleStart = now;
-    var delay = this._idleStart + this._idleTimeout - now;
-    if (delay < 0) delay = 0;
-
-    // Prevent running cb again when unref() is called during the same cb
-    if (this._called && !this._repeat) {
-      unenroll(this);
-      return;
-    }
-
-    var handle = reuse(this);
-
-    this._handle = handle || new TimerWrap();
-    this._handle.owner = this;
-    this._handle[kOnTimeout] = unrefdHandle;
-    this._handle.start(delay);
-    this._handle.domain = this.domain;
+  } else if (typeof this._onTimeout === 'function' &&
+      this._idleNext !== this/* Has been inserted */ &&
+      this._idleNext) {
+    createOwnHandle.call(this);
     this._handle.unref();
   }
   return this;
 };
 
 Timeout.prototype.ref = function() {
-  if (this._handle)
+  this._unrefed = false;
+  if (this._handle) {
     this._handle.ref();
+  } else if (typeof this._onTimeout === 'function' &&
+      this._idleNext !== this/* Has been inserted */ &&
+      this._idleNext) { // TODO: not guarenteed correct for ref()
+    createOwnHandle.call(this);
+    this._handle.ref();
+  }
   return this;
 };
 

test/parallel/test-handle-wrap-isrefed.js

@@ -91,11 +91,13 @@ function makeAssert(message) {
 {
   const assert = makeAssert('hasRef() not working on timer_wrap');
   const timer = setTimeout(() => {}, 500);
-  timer.unref();
-  assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('hasRef'), true);
-  assert(timer._handle.hasRef(), false);
-  timer.ref();
-  assert(timer._handle.hasRef(), true);
-  timer._handle.close(
-      common.mustCall(() => assert(timer._handle.hasRef(), false)));
+  setImmediate(() => {
+    timer.unref();
+    assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('hasRef'), true);
+    assert(timer._handle.hasRef(), false);
+    timer.ref();
+    assert(timer._handle.hasRef(), true);
+    timer._handle.close(
+        common.mustCall(() => assert(timer._handle.hasRef(), false)));
+  });
 }

test/parallel/test-timers-active.js

@@ -12,11 +12,13 @@ var legitTimers = [
 legitTimers.forEach(function(legit) {
   const savedTimeout = legit._idleTimeout;
   active(legit);
-  // active() should mutate these objects
-  assert(legit._idleTimeout === savedTimeout);
-  assert(Number.isInteger(legit._idleStart));
-  assert(legit._idleNext);
-  assert(legit._idlePrev);
+  process.nextTick(() => {
+    // active() should mutate these objects
+    assert(legit._idleTimeout === savedTimeout);
+    assert(Number.isInteger(legit._idleStart));
+    assert(legit._idleNext);
+    assert(legit._idlePrev);
+  });
 });
 
 
@@ -29,6 +31,8 @@ var bogusTimers = [
 bogusTimers.forEach(function(bogus) {
   const savedTimeout = bogus._idleTimeout;
   active(bogus);
-  // active() should not mutate these objects
-  assert.deepStrictEqual(bogus, {_idleTimeout: savedTimeout});
+  process.nextTick(() => {
+    // active() should not mutate these objects
+    assert.deepStrictEqual(bogus, {_idleTimeout: savedTimeout});
+  });
 });

test/parallel/test-timers-same-timeout-wrong-list-deleted.js

@@ -56,25 +56,27 @@ const handle1 = setTimeout(common.mustCall(function() {
     }));
   }), 10);
 
-  // Make sure our timers got added to the list.
-  const activeHandles = process._getActiveHandles();
-  const activeTimers = activeHandles.filter(function(handle) {
-    return handle instanceof Timer;
-  });
-  const shortTimer = activeTimers.find(function(handle) {
-    return handle._list.msecs === 10;
-  });
-  const longTimers = activeTimers.filter(function(handle) {
-    return handle._list.msecs === TIMEOUT;
-  });
+  setImmediate(() => {
+    // Make sure our timers got added to the list.
+    const activeHandles = process._getActiveHandles();
+    const activeTimers = activeHandles.filter(function(handle) {
+      return handle instanceof Timer;
+    });
+    const shortTimer = activeTimers.find(function(handle) {
+      return handle._list.msecs === 10;
+    });
+    const longTimers = activeTimers.filter(function(handle) {
+      return handle._list.msecs === TIMEOUT;
+    });
 
-  // Make sure our clearTimeout succeeded. One timer finished and
-  // the other was canceled, so none should be active.
-  assert.equal(activeTimers.length, 3, 'There are 3 timers in the list.');
-  assert(shortTimer instanceof Timer, 'The shorter timer is in the list.');
-  assert.equal(longTimers.length, 2, 'Both longer timers are in the list.');
+    // Make sure our clearTimeout succeeded. One timer finished and
+    // the other was canceled, so none should be active.
+    assert.equal(activeTimers.length, 3, 'There are 3 timers in the list.');
+    assert(shortTimer instanceof Timer, 'The shorter timer is in the list.');
+    assert.equal(longTimers.length, 2, 'Both longer timers are in the list.');
 
-  // When this callback completes, `listOnTimeout` should now look at the
-  // correct list and refrain from removing the new TIMEOUT list which
-  // contains the reference to the newer timer.
+    // When this callback completes, `listOnTimeout` should now look at the
+    // correct list and refrain from removing the new TIMEOUT list which
+    // contains the reference to the newer timer.
+  });
 }), TIMEOUT);

test/parallel/test-timers-unref-after-nt.js

@@ -0,0 +1,12 @@
+'use strict';
+const common = require('../common');
+
+const timer = setTimeout(() => {
+  common.fail('nope');
+}, 200);
+
+setImmediate(() => {
+  timer.unref();
+});
+
+setTimeout(common.fail, 500).unref();

test/parallel/test-timers-unref-leak.js

@@ -8,14 +8,16 @@ var closed = 0;
 var timeout = setTimeout(function() {
   called++;
 }, 10);
-timeout.unref();
 
 // Wrap `close` method to check if the handle was closed
-var close = timeout._handle.close;
-timeout._handle.close = function() {
-  closed++;
-  return close.apply(this, arguments);
-};
+process.nextTick(() => {
+  timeout.unref();
+  var close = timeout._handle.close;
+  timeout._handle.close = function() {
+    closed++;
+    return close.apply(this, arguments);
+  };
+});
 
 // Just to keep process alive and let previous timer's handle die
 setTimeout(function() {