Skip to content

Commit 086741d

Browse files
mcollinarichardlau
authored andcommitted
timers: reuse Timeout objects in setStreamTimeout
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> PR-URL: #64254 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent efbbb9a commit 086741d

2 files changed

Lines changed: 40 additions & 2 deletions

File tree

lib/internal/stream_base_commons.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const {
2222
const { owner_symbol } = require('internal/async_hooks').symbols;
2323
const {
2424
kTimeout,
25-
setUnrefTimeout,
25+
reuseOrCreateUnrefTimeout,
2626
getTimerDuration,
2727
} = require('internal/timers');
2828
const { isUint8Array } = require('internal/util/types');
@@ -233,6 +233,10 @@ function onStreamRead(arrayBuffer) {
233233
}
234234
}
235235

236+
function onStreamTimeout(stream) {
237+
stream._onTimeout();
238+
}
239+
236240
function setStreamTimeout(msecs, callback) {
237241
if (this.destroyed)
238242
return this;
@@ -244,6 +248,8 @@ function setStreamTimeout(msecs, callback) {
244248

245249
// Attempt to clear an existing timer in both cases -
246250
// even if it will be rescheduled we don't want to leak an existing timer.
251+
// The cleared Timeout stays referenced in this[kTimeout] so that it can be
252+
// reused the next time a timeout is set, e.g. on keep-alive connections.
247253
clearTimeout(this[kTimeout]);
248254

249255
if (msecs === 0) {
@@ -252,7 +258,8 @@ function setStreamTimeout(msecs, callback) {
252258
this.removeListener('timeout', callback);
253259
}
254260
} else {
255-
this[kTimeout] = setUnrefTimeout(this._onTimeout.bind(this), msecs);
261+
this[kTimeout] =
262+
reuseOrCreateUnrefTimeout(this[kTimeout], onStreamTimeout, msecs, this);
256263
if (this[kSession]) this[kSession][kUpdateTimer]();
257264
if (this[kBoundSession]) this[kBoundSession][kUpdateTimer]();
258265

lib/internal/timers.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,36 @@ function setUnrefTimeout(callback, after) {
417417
return timer;
418418
}
419419

420+
// Just like setUnrefTimeout() but reuses `timer` if it is a Timeout that is
421+
// no longer scheduled (fired or cleared), avoiding the allocation of a new
422+
// Timeout on rearm. This is the internal reuse path anticipated by the
423+
// TODO in unenroll() (lib/timers.js), used by hot paths such as the
424+
// keep-alive timeout handling in the HTTP server. `arg` is passed to
425+
// `callback` when the timer fires.
426+
function reuseOrCreateUnrefTimeout(timer, callback, after, arg) {
427+
if (timer !== undefined && timer !== null &&
428+
timer._destroyed && timer._repeat === null && !timer[kHasPrimitive]) {
429+
timer._idleTimeout = after;
430+
timer._onTimeout = callback;
431+
const args = timer._timerArgs;
432+
if (args === undefined) {
433+
timer._timerArgs = [arg];
434+
} else {
435+
args[0] = arg;
436+
}
437+
// Re-inserts the timer and re-initializes its async resource, so
438+
// async_hooks observes the same init/destroy sequence as if a new
439+
// Timeout had been allocated.
440+
unrefActive(timer);
441+
return timer;
442+
}
443+
444+
timer = new Timeout(callback, after, [arg], false, false);
445+
insert(timer, timer._idleTimeout);
446+
447+
return timer;
448+
}
449+
420450
// Type checking used by timers.enroll() and Socket#setTimeout()
421451
function getTimerDuration(msecs, name) {
422452
validateNumber(msecs, name);
@@ -704,6 +734,7 @@ module.exports = {
704734
kHasPrimitive,
705735
initAsyncResource,
706736
setUnrefTimeout,
737+
reuseOrCreateUnrefTimeout,
707738
getTimerDuration,
708739
immediateQueue,
709740
getTimerCallbacks,

0 commit comments

Comments
 (0)