timers: do less work in insert

Most of the code in insert is only applicable to scheduling non-timers or re-scheduling timers. We can skip most of it in the case of setTimeout, setInterval & setUnrefTimeout. PR-URL: #27345 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jan 14, 2020 · f108a5f · f108a5f
1 parent 84e8e1a
commit f108a5f
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 33 deletions.
diff --git a/lib/internal/timers.js b/lib/internal/timers.js
@@ -155,7 +155,7 @@ function initAsyncResource(resource, type) {
 
 // Timer constructor function.
 // The entire prototype is defined in lib/timers.js
-function Timeout(callback, after, args, isRepeat) {
+function Timeout(callback, after, args, isRepeat, isRefed) {
   after *= 1; // Coalesce to number or NaN
   if (!(after >= 1 && after <= TIMEOUT_MAX)) {
     if (after > TIMEOUT_MAX) {
@@ -179,7 +179,9 @@ function Timeout(callback, after, args, isRepeat) {
   this._repeat = isRepeat ? after : null;
   this._destroyed = false;
 
-  this[kRefed] = null;
+  if (isRefed)
+    incRefCount();
+  this[kRefed] = isRefed;
 
   initAsyncResource(this, 'Timeout');
 }
@@ -295,27 +297,43 @@ function decRefCount() {
 // Schedule or re-schedule a timer.
 // The item must have been enroll()'d first.
 function active(item) {
-  insert(item, true, getLibuvNow());
+  insertGuarded(item, true);
 }
 
 // Internal APIs that need timeouts should use `unrefActive()` instead of
 // `active()` so that they do not unnecessarily keep the process open.
 function unrefActive(item) {
-  insert(item, false, getLibuvNow());
+  insertGuarded(item, false);
 }
 
 // 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
 // list if one does not already exist for the specified timeout duration.
-function insert(item, refed, start) {
-  let msecs = item._idleTimeout;
+function insertGuarded(item, refed, start) {
+  const msecs = item._idleTimeout;
   if (msecs < 0 || msecs === undefined)
     return;
 
-  // Truncate so that accuracy of sub-millisecond timers is not assumed.
-  msecs = MathTrunc(msecs);
+  insert(item, msecs, start);
+
+  if (!item[async_id_symbol] || item._destroyed) {
+    item._destroyed = false;
+    initAsyncResource(item, 'Timeout');
+  }
+
+  if (refed === !item[kRefed]) {
+    if (refed)
+      incRefCount();
+    else
+      decRefCount();
+  }
+  item[kRefed] = refed;
+}
 
+function insert(item, msecs, start = getLibuvNow()) {
+  // Truncate so that accuracy of sub-milisecond timers is not assumed.
+  msecs = MathTrunc(msecs);
   item._idleStart = start;
 
   // Use an existing list if there is one, otherwise we need to make a new one.
@@ -332,19 +350,6 @@ function insert(item, refed, start) {
     }
   }
 
-  if (!item[async_id_symbol] || item._destroyed) {
-    item._destroyed = false;
-    initAsyncResource(item, 'Timeout');
-  }
-
-  if (refed === !item[kRefed]) {
-    if (refed)
-      incRefCount();
-    else
-      decRefCount();
-  }
-  item[kRefed] = refed;
-
   L.append(list, item);
 }
 
@@ -354,8 +359,8 @@ function setUnrefTimeout(callback, after) {
     throw new ERR_INVALID_CALLBACK(callback);
   }
 
-  const timer = new Timeout(callback, after, undefined, false);
-  unrefActive(timer);
+  const timer = new Timeout(callback, after, undefined, false, false);
+  insert(timer, timer._idleTimeout);
 
   return timer;
 }
@@ -540,16 +545,14 @@ function getTimerCallbacks(runNextTicks) {
       } finally {
         if (timer._repeat && timer._idleTimeout !== -1) {
           timer._idleTimeout = timer._repeat;
-          if (start === undefined)
-            start = getLibuvNow();
-          insert(timer, timer[kRefed], start);
+          insert(timer, timer._idleTimeout, start);
         } else if (!timer._idleNext && !timer._idlePrev) {
           if (timer[kRefed])
             refCount--;
           timer[kRefed] = null;
 
           if (destroyHooksExist() && !timer._destroyed) {
-            emitDestroy(timer[async_id_symbol]);
+            emitDestroy(asyncId);
           }
           timer._destroyed = true;
         }
@@ -598,6 +601,7 @@ module.exports = {
   },
   active,
   unrefActive,
+  insert,
   timerListMap,
   timerListQueue,
   decRefCount,

diff --git a/lib/timers.js b/lib/timers.js
@@ -46,7 +46,8 @@ const {
   timerListQueue,
   immediateQueue,
   active,
-  unrefActive
+  unrefActive,
+  insert
 } = require('internal/timers');
 const {
   promisify: { custom: customPromisify },
@@ -142,16 +143,17 @@ function setTimeout(callback, after, arg1, arg2, arg3) {
       break;
   }
 
-  const timeout = new Timeout(callback, after, args, false);
-  active(timeout);
+  const timeout = new Timeout(callback, after, args, false, true);
+  insert(timeout, timeout._idleTimeout);
 
   return timeout;
 }
 
 setTimeout[customPromisify] = function(after, value) {
   const args = value !== undefined ? [value] : value;
   return new Promise((resolve) => {
-    active(new Timeout(resolve, after, args, false));
+    const timeout = new Timeout(resolve, after, args, false, true);
+    insert(timeout, timeout._idleTimeout);
   });
 };
 
@@ -188,8 +190,8 @@ function setInterval(callback, repeat, arg1, arg2, arg3) {
       break;
   }
 
-  const timeout = new Timeout(callback, repeat, args, true);
-  active(timeout);
+  const timeout = new Timeout(callback, repeat, args, true, true);
+  insert(timeout, timeout._idleTimeout);
 
   return timeout;
 }