timers: run nextTicks after each immediate and timer

In order to better match the browser behaviour, run nextTicks (and
subsequently the microtask queue) after each individual Timer and
Immediate, rather than after the whole list is processed. The
current behaviour is somewhat of a performance micro-optimization
and also partly dictated by how timer handles were implemented.

PR-URL: https://github.com/nodejs/node/pull/22842
Fixes: https://github.com/nodejs/node/issues/22257
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

1 files changed, 76 insertions, 99 deletions

diff --git a/lib/timers.js b/lib/timers.js
index 9070ade70e..575dcf25f6 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -254,16 +254,17 @@ function processTimers(now) {
   debug('process timer lists %d', now);
   nextExpiry = Infinity;
 
-  let list, ran;
+  let list;
+  let ranAtLeastOneList = false;
   while (list = queue.peek()) {
     if (list.expiry > now) {
       nextExpiry = list.expiry;
       return refCount > 0 ? nextExpiry : -nextExpiry;
     }
-    if (ran)
+    if (ranAtLeastOneList)
       runNextTicks();
     else
-      ran = true;
+      ranAtLeastOneList = true;
     listOnTimeout(list, now);
   }
   return 0;
@@ -275,6 +276,7 @@ function listOnTimeout(list, now) {
   debug('timeout callback %d', msecs);
 
   var diff, timer;
+  let ranAtLeastOneTimer = false;
   while (timer = L.peek(list)) {
     diff = now - timer._idleStart;
 
@@ -288,6 +290,11 @@ function listOnTimeout(list, now) {
       return;
     }
 
+    if (ranAtLeastOneTimer)
+      runNextTicks();
+    else
+      ranAtLeastOneTimer = true;
+
     // The actual logic for when a timeout happens.
     L.remove(timer);
 
@@ -307,7 +314,33 @@ function listOnTimeout(list, now) {
 
     emitBefore(asyncId, timer[trigger_async_id_symbol]);
 
-    tryOnTimeout(timer);
+    let start;
+    if (timer._repeat)
+      start = getLibuvNow();
+
+    try {
+      const args = timer._timerArgs;
+      if (!args)
+        timer._onTimeout();
+      else
+        Reflect.apply(timer._onTimeout, timer, args);
+    } finally {
+      if (timer._repeat && timer._idleTimeout !== -1) {
+        timer._idleTimeout = timer._repeat;
+        if (start === undefined)
+          start = getLibuvNow();
+        insert(timer, timer[kRefed], start);
+      } else {
+        if (timer[kRefed])
+          refCount--;
+        timer[kRefed] = null;
+
+        if (destroyHooksExist() && !timer._destroyed) {
+          emitDestroy(timer[async_id_symbol]);
+          timer._destroyed = true;
+        }
+      }
+    }
 
     emitAfter(asyncId);
   }
@@ -327,30 +360,6 @@ function listOnTimeout(list, now) {
 }
 
 
-// An optimization so that the try/finally only de-optimizes (since at least v8
-// 4.7) what is in this smaller function.
-function tryOnTimeout(timer, start) {
-  if (start === undefined && timer._repeat)
-    start = getLibuvNow();
-  try {
-    ontimeout(timer);
-  } finally {
-    if (timer._repeat) {
-      rearm(timer, start);
-    } else {
-      if (timer[kRefed])
-        refCount--;
-      timer[kRefed] = null;
-
-      if (destroyHooksExist() && !timer._destroyed) {
-        emitDestroy(timer[async_id_symbol]);
-        timer._destroyed = true;
-      }
-    }
-  }
-}
-
-
 // Remove a timer. Cancels the timeout and resets the relevant timer properties.
 function unenroll(item) {
   // Fewer checks may be possible, but these cover everything.
@@ -456,26 +465,6 @@ setTimeout[internalUtil.promisify.custom] = function(after, value) {
 exports.setTimeout = setTimeout;
 
 
-function ontimeout(timer) {
-  const args = timer._timerArgs;
-  if (typeof timer._onTimeout !== 'function')
-    return Promise.resolve(timer._onTimeout, args[0]);
-  if (!args)
-    timer._onTimeout();
-  else
-    Reflect.apply(timer._onTimeout, timer, args);
-}
-
-function rearm(timer, start) {
-  // Do not re-arm unenroll'd or closed timers.
-  if (timer._idleTimeout === -1)
-    return;
-
-  timer._idleTimeout = timer._repeat;
-  insert(timer, timer[kRefed], start);
-}
-
-
 const clearTimeout = exports.clearTimeout = function clearTimeout(timer) {
   if (timer && timer._onTimeout) {
     timer._onTimeout = null;
@@ -601,75 +590,63 @@ function processImmediate() {
   const queue = outstandingQueue.head !== null ?
     outstandingQueue : immediateQueue;
   var immediate = queue.head;
-  const tail = queue.tail;
 
   // Clear the linked list early in case new `setImmediate()` calls occur while
   // immediate callbacks are executed
-  queue.head = queue.tail = null;
-
-  let count = 0;
-  let refCount = 0;
+  if (queue !== outstandingQueue) {
+    queue.head = queue.tail = null;
+    immediateInfo[kHasOutstanding] = 1;
+  }
 
+  let prevImmediate;
+  let ranAtLeastOneImmediate = false;
   while (immediate !== null) {
-    immediate._destroyed = true;
+    if (ranAtLeastOneImmediate)
+      runNextTicks();
+    else
+      ranAtLeastOneImmediate = true;
 
-    const asyncId = immediate[async_id_symbol];
-    emitBefore(asyncId, immediate[trigger_async_id_symbol]);
+    // It's possible for this current Immediate to be cleared while executing
+    // the next tick queue above, which means we need to use the previous
+    // Immediate's _idleNext which is guaranteed to not have been cleared.
+    if (immediate._destroyed) {
+      outstandingQueue.head = immediate = prevImmediate._idleNext;
+      continue;
+    }
 
-    count++;
+    immediate._destroyed = true;
+
+    immediateInfo[kCount]--;
     if (immediate[kRefed])
-      refCount++;
+      immediateInfo[kRefCount]--;
     immediate[kRefed] = null;
 
-    tryOnImmediate(immediate, tail, count, refCount);
+    prevImmediate = immediate;
 
-    emitAfter(asyncId);
+    const asyncId = immediate[async_id_symbol];
+    emitBefore(asyncId, immediate[trigger_async_id_symbol]);
 
-    immediate = immediate._idleNext;
-  }
+    try {
+      const argv = immediate._argv;
+      if (!argv)
+        immediate._onImmediate();
+      else
+        Reflect.apply(immediate._onImmediate, immediate, argv);
+    } finally {
+      immediate._onImmediate = null;
 
-  immediateInfo[kCount] -= count;
-  immediateInfo[kRefCount] -= refCount;
-  immediateInfo[kHasOutstanding] = 0;
-}
+      if (destroyHooksExist())
+        emitDestroy(asyncId);
 
-// An optimization so that the try/finally only de-optimizes (since at least v8
-// 4.7) what is in this smaller function.
-function tryOnImmediate(immediate, oldTail, count, refCount) {
-  var threw = true;
-  try {
-    // make the actual call outside the try/finally to allow it to be optimized
-    runCallback(immediate);
-    threw = false;
-  } finally {
-    immediate._onImmediate = null;
-
-    if (destroyHooksExist()) {
-      emitDestroy(immediate[async_id_symbol]);
+      outstandingQueue.head = immediate = immediate._idleNext;
     }
 
-    if (threw) {
-      immediateInfo[kCount] -= count;
-      immediateInfo[kRefCount] -= refCount;
-
-      if (immediate._idleNext !== null) {
-        // Handle any remaining Immediates after error handling has resolved,
-        // assuming we're still alive to do so.
-        outstandingQueue.head = immediate._idleNext;
-        outstandingQueue.tail = oldTail;
-        immediateInfo[kHasOutstanding] = 1;
-      }
-    }
+    emitAfter(asyncId);
   }
-}
 
-function runCallback(timer) {
-  const argv = timer._argv;
-  if (typeof timer._onImmediate !== 'function')
-    return Promise.resolve(timer._onImmediate, argv[0]);
-  if (!argv)
-    return timer._onImmediate();
-  Reflect.apply(timer._onImmediate, timer, argv);
+  if (queue === outstandingQueue)
+    outstandingQueue.head = null;
+  immediateInfo[kHasOutstanding] = 0;
 }