timers: fix the order of timers under a long loop

Author: XadillaX

lib/internal/priority_queue.js

@@ -15,6 +15,7 @@ module.exports = class PriorityQueue {
   #heap = new Array(64);
   #setPosition;
   #size = 0;
+  #secondary = null;
 
   constructor(comparator, setPosition) {
     if (comparator !== undefined)
@@ -23,7 +24,19 @@ module.exports = class PriorityQueue {
       this.#setPosition = setPosition;
   }
 
-  insert(value) {
+  updateSecondary() {
+    if (this.#size <= 1) {
+      this.#secondary = null;
+      return;
+    }
+
+    const top = this.peek();
+    this.removeAt(1);
+    this.#secondary = this.peek();
+    this.insert(top, false);
+  }
+
+  insert(value, updateSecondary = true) {
     const heap = this.#heap;
     const pos = ++this.#size;
     heap[pos] = value;
@@ -32,12 +45,17 @@ module.exports = class PriorityQueue {
       heap.length *= 2;
 
     this.percolateUp(pos);
+    if (updateSecondary) this.updateSecondary();
   }
 
   peek() {
     return this.#heap[1];
   }
 
+  secondary() {
+    return this.#secondary;
+  }
+
   percolateDown(pos) {
     const compare = this.#compare;
     const setPosition = this.#setPosition;
@@ -82,7 +100,7 @@ module.exports = class PriorityQueue {
       setPosition(item, pos);
   }
 
-  removeAt(pos) {
+  removeAt(pos, updateSecondary = true) {
     const heap = this.#heap;
     const size = --this.#size;
     heap[pos] = heap[size + 1];
@@ -94,15 +112,20 @@ module.exports = class PriorityQueue {
       else
         this.percolateDown(pos);
     }
+
+    if (updateSecondary) {
+      this.updateSecondary();
+    }
   }
 
-  shift() {
+  shift(updateSecondary = true) {
     const heap = this.#heap;
     const value = heap[1];
     if (value === undefined)
       return;
 
     this.removeAt(1);
+    if (updateSecondary) this.updateSecondary();
 
     return value;
   }

lib/internal/timers.js

@@ -520,16 +520,26 @@ function getTimerCallbacks(runNextTicks) {
 
     let ranAtLeastOneTimer = false;
     let timer;
+    const secondaryList = timerListQueue.secondary();
     while ((timer = L.peek(list)) != null) {
       const diff = now - timer._idleStart;
 
       // Check if this loop iteration is too early for the next timer.
       // This happens if there are more timers scheduled for later in the list.
-      if (diff < msecs) {
+      //
+      // Else, if the secondary list is earlier than current timer, then we
+      // return to the main loop to process the secondary list first.
+      if (diff < msecs || secondaryList?.expiry < timer._idleStart + msecs) {
         list.expiry = MathMax(timer._idleStart + msecs, now + 1);
         list.id = timerListId++;
         timerListQueue.percolateDown(1);
-        debug('%d list wait because diff is %d', msecs, diff);
+        timerListQueue.updateSecondary();
+
+        if (diff < msecs) {
+          debug('%d list wait because diff is %d', msecs, diff);
+        } else {
+          debug('%d list wait because it\s no more earliest', msecs);
+        }
         return;
       }
 

test/parallel/test-priority-queue.js

@@ -80,35 +80,35 @@ const PriorityQueue = require('internal/priority_queue');
     return a.value - b.value;
   }, (node, pos) => (node.position = pos));
 
-  queue.insert({ value: 1, position: null });
-  queue.insert({ value: 2, position: null });
-  queue.insert({ value: 3, position: null });
-  queue.insert({ value: 4, position: null });
-  queue.insert({ value: 5, position: null });
+  queue.insert({ value: 1, position: null }, false);
+  queue.insert({ value: 2, position: null }, false);
+  queue.insert({ value: 3, position: null }, false);
+  queue.insert({ value: 4, position: null }, false);
+  queue.insert({ value: 5, position: null }, false);
 
-  queue.insert({ value: 2, position: null });
+  queue.insert({ value: 2, position: null }, false);
   const secondLargest = { value: 10, position: null };
-  queue.insert(secondLargest);
+  queue.insert(secondLargest, false);
   const largest = { value: 15, position: null };
-  queue.insert(largest);
+  queue.insert(largest, false);
 
-  queue.removeAt(5);
+  queue.removeAt(5, false);
   assert.strictEqual(largest.position, 5);
 
   // Check that removing 2nd to last item works fine
-  queue.removeAt(6);
+  queue.removeAt(6, false);
   assert.strictEqual(secondLargest.position, 6);
 
   // Check that removing the last item doesn't throw
-  queue.removeAt(6);
+  queue.removeAt(6, false);
 
-  assert.strictEqual(queue.shift().value, 1);
-  assert.strictEqual(queue.shift().value, 2);
-  assert.strictEqual(queue.shift().value, 2);
-  assert.strictEqual(queue.shift().value, 4);
-  assert.strictEqual(queue.shift().value, 15);
+  assert.strictEqual(queue.shift(false).value, 1);
+  assert.strictEqual(queue.shift(false).value, 2);
+  assert.strictEqual(queue.shift(false).value, 2);
+  assert.strictEqual(queue.shift(false).value, 4);
+  assert.strictEqual(queue.shift(false).value, 15);
 
-  assert.strictEqual(queue.shift(), undefined);
+  assert.strictEqual(queue.shift(false), undefined);
 }
 
 
@@ -122,20 +122,20 @@ const PriorityQueue = require('internal/priority_queue');
   const i7 = { value: 7, position: null };
   const i8 = { value: 8, position: null };
 
-  queue.insert({ value: 1, position: null });
-  queue.insert({ value: 6, position: null });
-  queue.insert({ value: 2, position: null });
-  queue.insert(i7);
-  queue.insert(i8);
-  queue.insert(i3);
+  queue.insert({ value: 1, position: null }, false);
+  queue.insert({ value: 6, position: null }, false);
+  queue.insert({ value: 2, position: null }, false);
+  queue.insert(i7, false);
+  queue.insert(i8, false);
+  queue.insert(i3, false);
 
   assert.strictEqual(i7.position, 4);
-  queue.removeAt(4);
+  queue.removeAt(4, false);
 
   // 3 should percolate up to swap with 6 (up)
   assert.strictEqual(i3.position, 2);
 
-  queue.removeAt(2);
+  queue.removeAt(2, false);
 
   // 8 should swap places with 6 (down)
   assert.strictEqual(i8.position, 4);

test/parallel/test-timers-timeout-with-long-loop.js

@@ -0,0 +1,33 @@
+'use strict';
+
+const assert = require('assert');
+
+const common = require('../common');
+
+const orders = [];
+
+process.on('exit', () => {
+  assert.deepStrictEqual(orders, [ 1, 2, 3 ]);
+});
+
+setTimeout(() => {
+  orders.push(1);
+}, 10);
+
+setTimeout(() => {
+  orders.push(2);
+}, 15);
+
+let now = Date.now();
+while (Date.now() - now < 100) {
+  //
+}
+
+setTimeout(() => {
+  orders.push(3);
+}, 10);
+
+now = Date.now();
+while (Date.now() - now < 100) {
+  //
+}