timers: reuse Timeout objects in setStreamTimeout#64254
Open
mcollina wants to merge 1 commit into
Open
Conversation
Rearm the stream's existing Timeout object instead of allocating a new Timeout and a bound callback on every setTimeout() call. The HTTP server disarms and rearms the keep-alive timeout twice per request, so this saves two allocations and the associated async resource churn on every request over a keep-alive connection. The async resource is re-initialized on reuse, so async_hooks observes the same init/destroy sequence as before. A microbenchmark of the setTimeout(msecs)/setTimeout(0) cycle improves by 2.2x (205.9ns to 92.8ns), and benchmark/http/simple.js (type=buffer len=1024 chunks=1 chunkedEnc=0 c=50) improves by 3.70% (t=2.15, 40 samples per binary with server and load generator pinned to disjoint CPU sets). Assisted-by: Claude Fable 5 (Claude Code) Signed-off-by: Matteo Collina <hello@matteocollina.com>
nodejs-github-bot
added
needs-ci
anonrig
approved these changes
Jul 2, 2026
mertcanaltin
reviewed
Jul 2, 2026
Comment on lines
+431
to
+434
| const args = timer._timerArgs; | ||
| if (args === undefined || args[0] !== arg) { | ||
| timer._timerArgs = [arg]; | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| const args = timer._timerArgs; | |
| if (args === undefined || args[0] !== arg) { | |
| timer._timerArgs = [arg]; | |
| } | |
| const args = timer._timerArgs; | |
| if (args === undefined) { | |
| timer._timerArgs = [arg]; | |
| } else { | |
| args[0] = arg; | |
| } |
Nit: Instead of assigning a new array, update _timerArgs directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rearm the stream's existing
Timeoutobject instead of allocating a newTimeoutand a bound callback on everysetTimeout()call. This is the internal reuse path anticipated by the TODO inunenroll()(lib/timers.js).The HTTP server disarms the socket timeout at request start and rearms the keep-alive timeout on response finish, so every request over a keep-alive connection currently pays a
Timeoutallocation, a.bind()closure allocation, and the associated timer-list/async-resource churn. With this change the socket's clearedTimeoutobject is kept referenced inthis[kTimeout]and reused on the next rearm. The async resource is re-initialized on reuse (via the existinginsertGuardedpath), so async_hooks observes the same init/destroy sequence and AsyncLocalStorage context capture is unchanged.Performance
A microbenchmark of the
setTimeout(msecs)/setTimeout(0)cycle on anet.Socketimproves by 2.2x (205.9 ns → 92.8 ns per cycle).End-to-end,
benchmark/http/simple.js(type=buffer len=1024 chunks=1 chunkedEnc=0 c=50, wrk) improves by +3.70% (t=2.15, 40 samples per binary, server and wrk pinned to disjoint CPU sets).