Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timers: use only a single TimerWrap instance #20555

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions benchmark/timers/timers-cancel-unpooled.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,26 @@ const assert = require('assert');

const bench = common.createBenchmark(main, {
n: [1e6],
direction: ['start', 'end']
});

function main({ n }) {
function main({ n, direction }) {

const timersList = [];
for (var i = 0; i < n; i++) {
timersList.push(setTimeout(cb, i + 1));
}

var j;
bench.start();
for (var j = 0; j < n + 1; j++) {
clearTimeout(timersList[j]);
if (direction === 'start') {
for (j = 0; j < n; j++) {
clearTimeout(timersList[j]);
}
} else {
for (j = n - 1; j >= 0; j--) {
clearTimeout(timersList[j]);
}
}
bench.end(n);
}
Expand Down
16 changes: 12 additions & 4 deletions benchmark/timers/timers-insert-unpooled.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,26 @@ const assert = require('assert');

const bench = common.createBenchmark(main, {
n: [1e6],
direction: ['start', 'end']
});

function main({ n }) {
function main({ direction, n }) {
const timersList = [];

var i;
bench.start();
for (var i = 0; i < n; i++) {
timersList.push(setTimeout(cb, i + 1));
if (direction === 'start') {
for (i = 1; i <= n; i++) {
timersList.push(setTimeout(cb, i));
}
} else {
for (i = n; i > 0; i--) {
timersList.push(setTimeout(cb, i));
}
}
bench.end(n);

for (var j = 0; j < n + 1; j++) {
for (var j = 0; j < n; j++) {
clearTimeout(timersList[j]);
}
}
Expand Down
18 changes: 18 additions & 0 deletions benchmark/util/priority-queue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

const common = require('../common');

const bench = common.createBenchmark(main, {
n: [1e6]
}, { flags: ['--expose-internals'] });

function main({ n, type }) {
const PriorityQueue = require('internal/priority_queue');
const queue = new PriorityQueue();
bench.start();
for (var i = 0; i < n; i++)
queue.insert(Math.random() * 1e7 | 0);

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

for (i = 0; i < n; i++)
queue.shift();
bench.end(n);
}
111 changes: 111 additions & 0 deletions lib/internal/priority_queue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
'use strict';

const kCompare = Symbol('compare');
const kHeap = Symbol('heap');
const kSetPosition = Symbol('setPosition');
const kSize = Symbol('size');

// The PriorityQueue is a basic implementation of a binary heap that accepts
// a custom sorting function via its constructor. This function is passed
// the two nodes to compare, similar to the native Array#sort. Crucially
// this enables priority queues that are based on a comparison of more than
// just a single criteria.

module.exports = class PriorityQueue {
constructor(comparator, setPosition) {
if (comparator !== undefined)
this[kCompare] = comparator;
if (setPosition !== undefined)
this[kSetPosition] = setPosition;

this[kHeap] = new Array(64);
this[kSize] = 0;
}

[kCompare](a, b) {
return a - b;
}

insert(value) {
const heap = this[kHeap];
let pos = ++this[kSize];

if (heap.length === pos)
heap.length *= 2;

const compare = this[kCompare];
const setPosition = this[kSetPosition];
while (pos > 1) {
const parent = heap[pos / 2 | 0];
if (compare(parent, value) <= 0)
break;
heap[pos] = parent;
if (setPosition !== undefined)
setPosition(parent, pos);
pos = pos / 2 | 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new position is actually calculated twice. I guess it does not change much but it could be better to cache the entry above and to set it down here.

}
heap[pos] = value;
if (setPosition !== undefined)
setPosition(value, pos);
}

peek() {
return this[kHeap][1];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not this[kHeap][0];? Is 0 unused?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes the math slightly more complicated to start binary heaps at 0, so it's unused.

}

percolateDown(pos) {
const compare = this[kCompare];
const setPosition = this[kSetPosition];
const heap = this[kHeap];
const size = this[kSize];
const item = heap[pos];

while (pos * 2 <= size) {
let childIndex = pos * 2 + 1;
if (childIndex > size || compare(heap[pos * 2], heap[childIndex]) < 0)
childIndex = pos * 2;
const child = heap[childIndex];
if (compare(item, child) <= 0)
break;
if (setPosition !== undefined)
setPosition(child, pos);
heap[pos] = child;
pos = childIndex;
}
heap[pos] = item;
if (setPosition !== undefined)
setPosition(item, pos);
}

removeAt(pos) {
const heap = this[kHeap];
const size = --this[kSize];
heap[pos] = heap[size + 1];
heap[size + 1] = undefined;

if (size > 0)
this.percolateDown(1);
}

remove(value) {
const heap = this[kHeap];
const pos = heap.indexOf(value);
if (pos < 1)
return false;

this.removeAt(pos);

return true;
}

shift() {
const heap = this[kHeap];
const value = heap[1];
if (value === undefined)
return;

this.removeAt(1);

return value;
}
};
22 changes: 8 additions & 14 deletions lib/internal/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ const {
// Timeout values > TIMEOUT_MAX are set to 1.
const TIMEOUT_MAX = 2 ** 31 - 1;

const unrefedSymbol = Symbol('unrefed');
const kRefed = Symbol('refed');

module.exports = {
TIMEOUT_MAX,
kTimeout: Symbol('timeout'), // For hiding Timeouts on other internals.
async_id_symbol,
trigger_async_id_symbol,
Timeout,
kRefed,
initAsyncResource,
setUnrefTimeout,
validateTimerDuration
Expand All @@ -50,7 +51,7 @@ function initAsyncResource(resource, type) {

// Timer constructor function.
// The entire prototype is defined in lib/timers.js
function Timeout(callback, after, args, isRepeat, isUnrefed) {
function Timeout(callback, after, args, isRepeat) {
after *= 1; // coalesce to number or NaN
if (!(after >= 1 && after <= TIMEOUT_MAX)) {
if (after > TIMEOUT_MAX) {
Expand All @@ -62,7 +63,6 @@ function Timeout(callback, after, args, isRepeat, isUnrefed) {
after = 1; // schedule on next tick, follows browser behavior
}

this._called = false;
this._idleTimeout = after;
this._idlePrev = this;
this._idleNext = this;
Expand All @@ -75,22 +75,16 @@ function Timeout(callback, after, args, isRepeat, isUnrefed) {
this._repeat = isRepeat ? after : null;
this._destroyed = false;

this[unrefedSymbol] = isUnrefed;
this[kRefed] = null;

initAsyncResource(this, 'Timeout');
}

Timeout.prototype.refresh = function() {
if (this._handle) {
// Would be more ideal with uv_timer_again(), however that API does not
// cause libuv's sorted timers data structure (a binary heap at the time
// of writing) to re-sort itself. This causes ordering inconsistencies.
this._handle.start(this._idleTimeout);
} else if (this[unrefedSymbol]) {
getTimers()._unrefActive(this);
} else {
if (this[kRefed])
getTimers().active(this);
}
else
getTimers()._unrefActive(this);

return this;
};
Expand Down Expand Up @@ -122,7 +116,7 @@ function setUnrefTimeout(callback, after, arg1, arg2, arg3) {
break;
}

const timer = new Timeout(callback, after, args, false, true);
const timer = new Timeout(callback, after, args, false);
getTimers()._unrefActive(timer);

return timer;
Expand Down
Loading