Skip to content

process: handle rejections only when needed#62919

Open
gurgunday wants to merge 1 commit intonodejs:mainfrom
gurgunday:perf/event-loop-skip
Open

process: handle rejections only when needed#62919
gurgunday wants to merge 1 commit intonodejs:mainfrom
gurgunday:perf/event-loop-skip

Conversation

@gurgunday
Copy link
Copy Markdown
Member

@gurgunday gurgunday commented Apr 23, 2026

After:

./node benchmark/timers/timers-timeout-nexttick.js
timers/timers-timeout-nexttick.js n=50000: 2,807,740.108387753
timers/timers-timeout-nexttick.js n=5000000: 9,041,959.898481071

Before:

./node benchmark/timers/timers-timeout-nexttick.js
timers/timers-timeout-nexttick.js n=50000: 2,127,372.8770254664
timers/timers-timeout-nexttick.js n=5000000: 6,926,259.012697572
  • Skips noop processPromiseRejections() calls in processTicksAndRejections() when there is no pending rejection work

We already do a check before entering the loop through runNextTicks():

// Should be in sync with RunNextTicksNative in node_task_queue.cc
function runNextTicks() {
if (!hasTickScheduled() && !hasRejectionToWarn())
runMicrotasks();
if (!hasTickScheduled() && !hasRejectionToWarn())
return;
processTicksAndRejections();
}

But once we are already inside processTicksAndRejections() because of tick work, the internal do..while loop still calls processPromiseRejections() on every iteration (where we have a Map instantiation, method calls, etc.) without checking if we should even call it:

} while (!queue.isEmpty() || processPromiseRejections());

function processPromiseRejections() {
let maybeScheduledTicksOrMicrotasks = !asyncHandledRejections.isEmpty();
while (!asyncHandledRejections.isEmpty()) {
const { promise, warning } = asyncHandledRejections.shift();
if (!process.emit('rejectionHandled', promise)) {
process.emitWarning(warning);
}
}
let needPop = true;
let promiseAsyncId;
const pending = pendingUnhandledRejections;
pendingUnhandledRejections = new SafeMap();
for (const { 0: promise, 1: promiseInfo } of pending.entries()) {
maybeUnhandledPromises.set(promise, promiseInfo);
promiseInfo.warned = true;
// We need to check if async_hooks are enabled
// don't use enabledHooksExist as a Promise could
// come from a vm.* context and not have an async id
promiseAsyncId = promise[kAsyncIdSymbol];
if (promiseAsyncId !== undefined) {
pushAsyncContext(
promiseAsyncId,
promise[kTriggerAsyncIdSymbol],
promise,
);
}
const { contextFrame } = promiseInfo;
const priorContextFrame = AsyncContextFrame.exchange(contextFrame);
try {
needPop = unhandledRejectionsMode(promise, promiseInfo, promiseAsyncId);
} finally {
AsyncContextFrame.set(priorContextFrame);
needPop &&
promiseAsyncId !== undefined &&
popAsyncContext(promiseAsyncId);
}
maybeScheduledTicksOrMicrotasks = true;
}
return maybeScheduledTicksOrMicrotasks ||
pendingUnhandledRejections.size !== 0;
}

Now the name hasRejectionToWarn is inaccurate because we use it for any type of unhandled rejection:

/**
* @param {Promise} promise
* @param {Error} reason
*/
function unhandledRejection(promise, reason) {
pendingUnhandledRejections.set(promise, {
reason,
uid: ++lastPromiseId,
warned: false,
domain: process.domain,
contextFrame: AsyncContextFrame.current(),
});
setHasRejectionToWarn(true);
}

So skipping covers all types of rejections

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Apr 23, 2026
@gurgunday gurgunday added the performance Issues and PRs related to the performance of Node.js. label Apr 23, 2026
@gurgunday
Copy link
Copy Markdown
Member Author

gurgunday commented Apr 23, 2026

Cc @nodejs/performance since they weren't tagged

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.63%. Comparing base (1266013) to head (a02e782).
⚠️ Report is 109 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62919      +/-   ##
==========================================
- Coverage   89.68%   89.63%   -0.05%     
==========================================
  Files         706      706              
  Lines      218223   219206     +983     
  Branches    41768    42006     +238     
==========================================
+ Hits       195703   196488     +785     
- Misses      14426    14610     +184     
- Partials     8094     8108      +14     
Files with missing lines Coverage Δ
lib/internal/process/task_queues.js 100.00% <100.00%> (ø)

... and 68 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 25, 2026
@gurgunday gurgunday added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 25, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 25, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js. process Issues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants