Skip to content

Commit

Permalink
timers: fix the order of timers under a long loop
Browse files Browse the repository at this point in the history
  • Loading branch information
XadillaX committed Feb 15, 2023
1 parent ecd385e commit cb2f09d
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 2 deletions.
18 changes: 18 additions & 0 deletions lib/internal/priority_queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,24 @@ module.exports = class PriorityQueue {
return this.#heap[1];
}

secondary() {
// As the priority queue is a binary heap, the secondary element is
// always the second or third element in the heap.
switch (this.#size) {
case 0:
case 1:
return undefined;

case 2:
return this.#heap[2] === undefined ? this.#heap[3] : this.#heap[2];

default:
return this.#compare(this.#heap[2], this.#heap[3]) < 0 ?
this.#heap[2] :
this.#heap[3];
}
}

percolateDown(pos) {
const compare = this.#compare;
const setPosition = this.#setPosition;
Expand Down
13 changes: 11 additions & 2 deletions lib/internal/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -520,16 +520,25 @@ 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);

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;
}

Expand Down
13 changes: 13 additions & 0 deletions test/parallel/test-priority-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ const PriorityQueue = require('internal/priority_queue');
queue.insert({ value: 3, position: null });
queue.insert({ value: 4, position: null });
queue.insert({ value: 5, position: null });
assert.strictEqual(queue.secondary().value, 2);

queue.insert({ value: 2, position: null });
assert.strictEqual(queue.secondary().value, 2);
const secondLargest = { value: 10, position: null };
queue.insert(secondLargest);
const largest = { value: 15, position: null };
Expand All @@ -103,10 +105,15 @@ const PriorityQueue = require('internal/priority_queue');
queue.removeAt(6);

assert.strictEqual(queue.shift().value, 1);
assert.strictEqual(queue.secondary().value, 2);
assert.strictEqual(queue.shift().value, 2);
assert.strictEqual(queue.secondary().value, 4);
assert.strictEqual(queue.shift().value, 2);
assert.strictEqual(queue.secondary().value, 15);
assert.strictEqual(queue.shift().value, 4);
assert.strictEqual(queue.secondary(), undefined);
assert.strictEqual(queue.shift().value, 15);
assert.strictEqual(queue.secondary(), undefined);

assert.strictEqual(queue.shift(), undefined);
}
Expand All @@ -123,11 +130,17 @@ const PriorityQueue = require('internal/priority_queue');
const i8 = { value: 8, position: null };

queue.insert({ value: 1, position: null });
assert.strictEqual(queue.secondary(), undefined);
queue.insert({ value: 6, position: null });
assert.strictEqual(queue.secondary().value, 6);
queue.insert({ value: 2, position: null });
assert.strictEqual(queue.secondary().value, 2);
queue.insert(i7);
assert.strictEqual(queue.secondary().value, 2);
queue.insert(i8);
assert.strictEqual(queue.secondary().value, 2);
queue.insert(i3);
assert.strictEqual(queue.secondary().value, 2);

assert.strictEqual(i7.position, 4);
queue.removeAt(4);
Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-timers-timeout-with-long-loop.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use strict';

require('../common');

const assert = require('assert');

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) {
//
}

0 comments on commit cb2f09d

Please sign in to comment.