Skip to content

Commit

Permalink
timers: use timerListMap and outstandingQueue to count timers
Browse files Browse the repository at this point in the history
The additional objects that were getting added and deleted from the
activeTimersMap object were slowing down the rest of the timers code,
so this change falls back to traversing the timerListMap and
outstandingQueue objects to count the active timers inside
process.getActiveResourcesInfo().

Fixes: nodejs#41219

Signed-off-by: Darshan Sen <[email protected]>
  • Loading branch information
RaisinTen committed Dec 18, 2021
1 parent 587b167 commit 206ac18
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 30 deletions.
32 changes: 26 additions & 6 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@
setupPrepareStackTrace();

const {
Array,
ArrayPrototypeConcat,
ArrayPrototypeFilter,
ArrayPrototypeMap,
ArrayPrototypeFill,
ArrayPrototypeForEach,
FunctionPrototypeCall,
JSONParse,
ObjectDefineProperty,
Expand Down Expand Up @@ -156,13 +157,32 @@ const rawMethods = internalBinding('process_methods');
process._getActiveHandles = rawMethods._getActiveHandles;

process.getActiveResourcesInfo = function() {
var timeoutCount = 0;
ArrayPrototypeForEach(ObjectValues(internalTimers.timerListMap),
(list) => {
var timer = list._idlePrev === list ?
null : list._idlePrev;
while (timer !== null) {
timeoutCount += timer.hasRef();
timer = timer._idlePrev === list ?
null : timer._idlePrev;
}
});

var immediateCount = 0;
const queue = internalTimers.outstandingQueue.head !== null ?
internalTimers.outstandingQueue : internalTimers.immediateQueue;
var immediate = queue.head;
while (immediate !== null) {
immediateCount += immediate.hasRef();
immediate = immediate._idleNext;
}

return ArrayPrototypeConcat(
rawMethods._getActiveRequestsInfo(),
rawMethods._getActiveHandlesInfo(),
ArrayPrototypeMap(
ArrayPrototypeFilter(ObjectValues(internalTimers.activeTimersMap),
({ resource }) => resource.hasRef()),
({ type }) => type));
ArrayPrototypeFill(new Array(timeoutCount), 'Timeout'),
ArrayPrototypeFill(new Array(immediateCount), 'Immediate'));
};

// TODO(joyeecheung): remove these
Expand Down
20 changes: 5 additions & 15 deletions lib/internal/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,10 @@ const kRefed = Symbol('refed');
// Create a single linked list instance only once at startup
const immediateQueue = new ImmediateList();

// Object map containing timers
//
// - key = asyncId
// - value = { type, resource }
const activeTimersMap = ObjectCreate(null);
// If an uncaught exception was thrown during execution of immediateQueue,
// this queue will store all remaining Immediates that need to run upon
// resolution of all error handling (if process is still alive).
const outstandingQueue = new ImmediateList();

let nextExpiry = Infinity;
let refCount = 0;
Expand All @@ -166,7 +165,6 @@ function initAsyncResource(resource, type) {
resource[trigger_async_id_symbol] = getDefaultTriggerAsyncId();
if (initHooksExist())
emitInit(asyncId, type, triggerAsyncId, resource);
activeTimersMap[asyncId] = { type, resource };
}

// Timer constructor function.
Expand Down Expand Up @@ -420,11 +418,6 @@ function setPosition(node, pos) {
}

function getTimerCallbacks(runNextTicks) {
// If an uncaught exception was thrown during execution of immediateQueue,
// this queue will store all remaining Immediates that need to run upon
// resolution of all error handling (if process is still alive).
const outstandingQueue = new ImmediateList();

function processImmediate() {
const queue = outstandingQueue.head !== null ?
outstandingQueue : immediateQueue;
Expand Down Expand Up @@ -478,7 +471,6 @@ function getTimerCallbacks(runNextTicks) {

if (destroyHooksExist())
emitDestroy(asyncId);
delete activeTimersMap[asyncId];

outstandingQueue.head = immediate = immediate._idleNext;
}
Expand Down Expand Up @@ -551,7 +543,6 @@ function getTimerCallbacks(runNextTicks) {

if (destroyHooksExist())
emitDestroy(asyncId);
delete activeTimersMap[asyncId];
}
continue;
}
Expand Down Expand Up @@ -580,7 +571,6 @@ function getTimerCallbacks(runNextTicks) {

if (destroyHooksExist())
emitDestroy(asyncId);
delete activeTimersMap[asyncId];
}
}

Expand Down Expand Up @@ -661,6 +651,7 @@ module.exports = {
setUnrefTimeout,
getTimerDuration,
immediateQueue,
outstandingQueue,
getTimerCallbacks,
immediateInfoFields: {
kCount,
Expand All @@ -670,7 +661,6 @@ module.exports = {
active,
unrefActive,
insert,
activeTimersMap,
timerListMap,
timerListQueue,
decRefCount,
Expand Down
3 changes: 0 additions & 3 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ const {
kRefed,
kHasPrimitive,
getTimerDuration,
activeTimersMap,
timerListMap,
timerListQueue,
immediateQueue,
Expand Down Expand Up @@ -88,7 +87,6 @@ function unenroll(item) {
// Fewer checks may be possible, but these cover everything.
if (destroyHooksExist() && item[async_id_symbol] !== undefined)
emitDestroy(item[async_id_symbol]);
delete activeTimersMap[item[async_id_symbol]];

L.remove(item);

Expand Down Expand Up @@ -331,7 +329,6 @@ function clearImmediate(immediate) {
if (destroyHooksExist() && immediate[async_id_symbol] !== undefined) {
emitDestroy(immediate[async_id_symbol]);
}
delete activeTimersMap[immediate[async_id_symbol]];

immediate._onImmediate = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ assert.strictEqual(process.getActiveResourcesInfo().filter(

let count = 0;
const interval = setInterval(common.mustCall(() => {
// TODO(RaisinTen): This should be the correct behaviour.
// assert.strictEqual(process.getActiveResourcesInfo().filter(
// (type) => type === 'Timeout').length, 1);
assert.strictEqual(process.getActiveResourcesInfo().filter(
(type) => type === 'Timeout').length, 1);
(type) => type === 'Timeout').length, 0);
++count;
if (count === 3) {
clearInterval(interval);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ const assert = require('assert');
assert.strictEqual(process.getActiveResourcesInfo().filter(
(type) => type === 'Timeout').length, 0);

const timeout = setTimeout(common.mustCall(() => {
setTimeout(common.mustCall(function () {
// TODO(RaisinTen): This should be the correct behaviour.
// assert.strictEqual(process.getActiveResourcesInfo().filter(
// (type) => type === 'Timeout').length, 1);
assert.strictEqual(process.getActiveResourcesInfo().filter(
(type) => type === 'Timeout').length, 1);
clearTimeout(timeout);
(type) => type === 'Timeout').length, 0);
clearTimeout(this);
assert.strictEqual(process.getActiveResourcesInfo().filter(
(type) => type === 'Timeout').length, 0);
}), 0);
Expand All @@ -24,14 +27,14 @@ const assert = require('assert');
assert.strictEqual(process.getActiveResourcesInfo().filter(
(type) => type === 'Immediate').length, 0);

const immediate = setImmediate(common.mustCall(() => {
setImmediate(common.mustCall(function () {
// TODO(RaisinTen): Change this test to the following when the Immediate is
// destroyed and unrefed after the callback gets executed.
// assert.strictEqual(process.getActiveResourcesInfo().filter(
// (type) => type === 'Immediate').length, 1);
assert.strictEqual(process.getActiveResourcesInfo().filter(
(type) => type === 'Immediate').length, 0);
clearImmediate(immediate);
clearImmediate(this);
assert.strictEqual(process.getActiveResourcesInfo().filter(
(type) => type === 'Immediate').length, 0);
}));
Expand Down

0 comments on commit 206ac18

Please sign in to comment.