From 331609b86dc57b4e8c4a161872a9fe61b8d6ed27 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 2 Jul 2026 19:42:54 +0200 Subject: [PATCH 1/6] http2: reduce per-request allocations Cut several sources of per-stream/per-request overhead on the hot path: - Track 'priority'/'frameError' stream listeners by overriding the EventEmitter methods on Http2Stream instead of subscribing to 'newListener'/'removeListener', which made every listener add and remove on every stream emit an extra tracking event. - Replace the per-call SafeSet and sensitive-header mapping in buildNgHeaderString with a lazily allocated array and an empty-array fast path, and skip the HTTP token regex and connection-specific header checks for well-known single-value header names. - Replace per-call closures with shared named handlers in onStreamClose, afterShutdown and Http2Stream._destroy. - Skip the pendingStreams Set add/delete for streams that are created with their native handle already available (all server streams). - Hoist the per-request onStreamTimeout closure factories in the compat layer to module-level handlers, and avoid a once() wrapper allocation per server stream. h2load, 1 KiB response payload, -c 4 -m 100, mean of 6 alternating runs: core API 60.2k -> 69.3k req/s (+15%), compat API 43.6k -> 46.2k req/s (+5.9%). Signed-off-by: Matteo Collina --- lib/internal/http2/compat.js | 15 ++-- lib/internal/http2/core.js | 137 ++++++++++++++++++++++++----------- lib/internal/http2/util.js | 32 +++++--- 3 files changed, 123 insertions(+), 61 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 7b9524ef855988..9f13cad794333a 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -300,11 +300,12 @@ function onStreamCloseRequest() { req.emit('close'); } -function onStreamTimeout(kind) { - return function onStreamTimeout() { - const obj = this[kind]; - obj.emit('timeout'); - }; +function onStreamTimeoutRequest() { + this[kRequest].emit('timeout'); +} + +function onStreamTimeoutResponse() { + this[kResponse].emit('timeout'); } class Http2ServerRequest extends Readable { @@ -332,7 +333,7 @@ class Http2ServerRequest extends Readable { stream.on('error', onStreamError); stream.on('aborted', onStreamAbortedRequest); stream.on('close', onStreamCloseRequest); - stream.on('timeout', onStreamTimeout(kRequest)); + stream.on('timeout', onStreamTimeoutRequest); this.on('pause', onRequestPause); this.on('resume', onRequestResume); } @@ -488,7 +489,7 @@ class Http2ServerResponse extends Stream { stream.on('aborted', onStreamAbortedResponse); stream.on('close', onStreamCloseResponse); stream.on('wantTrailers', onStreamTrailersReady); - stream.on('timeout', onStreamTimeout(kResponse)); + stream.on('timeout', onStreamTimeoutResponse); } // User land modules such as finalhandler just check truthiness of this diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 3fe6380732a482..2d2ff7cd56fc3c 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -559,29 +559,19 @@ function sessionListenerRemoved(name) { } // Also keep track of listeners for the Http2Stream instances, as some events -// are emitted on those objects. -function streamListenerAdded(name) { - const session = this[kSession]; - if (!session) return; - switch (name) { - case 'priority': - session[kNativeFields][kSessionPriorityListenerCount]++; - break; - case 'frameError': - session[kNativeFields][kSessionFrameErrorListenerCount]++; - break; - } -} - -function streamListenerRemoved(name) { - const session = this[kSession]; +// are emitted on those objects. Instead of subscribing to 'newListener' and +// 'removeListener' (which makes every listener add/remove on every stream +// emit an extra tracking event), Http2Stream overrides the EventEmitter +// methods and updates the counts directly. +function trackStreamListener(stream, name, delta) { + const session = stream[kSession]; if (!session) return; switch (name) { case 'priority': - session[kNativeFields][kSessionPriorityListenerCount]--; + session[kNativeFields][kSessionPriorityListenerCount] += delta; break; case 'frameError': - session[kNativeFields][kSessionFrameErrorListenerCount]--; + session[kNativeFields][kSessionFrameErrorListenerCount] += delta; break; } } @@ -595,6 +585,20 @@ function onPing(payload) { session.emit('ping', payload); } +function streamNaturalCloseSettled(stream) { + return (stream._readableState.endEmitted || + !!stream._readableState.errored) && + (stream._writableState.finished || + !!stream._writableState.errored); +} + +// Shared 'end'/'finish'/'error' listener for the natural-close path of +// onStreamClose(). Named and reused to avoid per-stream closures. +function maybeDestroyNaturalClose() { + if (!this.destroyed && streamNaturalCloseSettled(this)) + this.destroy(); +} + // Fired by C++ when nghttp2's on_stream_close fires. `peerReset` is // true when the peer sent a RST_STREAM frame - peer RST_STREAM(NO_ERROR) // is otherwise indistinguishable from a clean END_STREAM exchange at @@ -655,22 +659,14 @@ function onStreamClose(code, peerReset) { // errored readable won't fire 'end' - and on a Duplex a writable // error propagates to readable.errored, blocking 'end' too. Treat // either side's errored state as settled. - const readDone = () => stream._readableState.endEmitted || - !!stream._readableState.errored; - const writeDone = () => stream._writableState.finished || - !!stream._writableState.errored; - if (readDone() && writeDone()) { + if (streamNaturalCloseSettled(stream)) { stream.destroy(); return true; } - const maybeDestroy = () => { - if (!stream.destroyed && readDone() && writeDone()) - stream.destroy(); - }; - stream.once('end', maybeDestroy); - stream.once('finish', maybeDestroy); - stream.once('error', maybeDestroy); + stream.on('end', maybeDestroyNaturalClose); + stream.on('finish', maybeDestroyNaturalClose); + stream.on('error', maybeDestroyNaturalClose); if (stream[kSession][kType] === NGHTTP2_SESSION_SERVER && !stream[kState].didRead && stream.readableFlowing === null) { @@ -2015,12 +2011,14 @@ function streamOnPause() { this[kHandle].readStop(); } +function streamOnFinishMaybeDestroy() { + this[kMaybeDestroy](); +} + function afterShutdown(status) { const stream = this.handle[kOwner]; if (stream) { - stream.on('finish', () => { - stream[kMaybeDestroy](); - }); + stream.on('finish', streamOnFinishMaybeDestroy); } // Currently this status value is unused this.callback(); @@ -2132,7 +2130,7 @@ function finishCloseStream(code) { // An Http2Stream is a Duplex stream that is backed by a // node::http2::Http2Stream handle implementing StreamBase. class Http2Stream extends Duplex { - constructor(session, options) { + constructor(session, options, hasHandle = false) { options.allowHalfOpen = true; options.decodeStrings = false; options.autoDestroy = false; @@ -2144,7 +2142,10 @@ class Http2Stream extends Duplex { // been assigned. this.cork(); this[kSession] = session; - session[kState].pendingStreams.add(this); + // Streams constructed with their native handle already available (e.g. + // server streams) are initialized immediately and never become pending. + if (!hasHandle) + session[kState].pendingStreams.add(this); // Allow our logic for determining whether any reads have happened to // work in all situations. This is similar to what we do in _http_incoming. @@ -2166,9 +2167,53 @@ class Http2Stream extends Duplex { this[kProxySocket] = null; this.on('pause', streamOnPause); + } - this.on('newListener', streamListenerAdded); - this.on('removeListener', streamListenerRemoved); + addListener(name, listener) { + const ret = super.addListener(name, listener); + if (name === 'priority' || name === 'frameError') + trackStreamListener(this, name, 1); + return ret; + } + + on(name, listener) { + const ret = super.on(name, listener); + if (name === 'priority' || name === 'frameError') + trackStreamListener(this, name, 1); + return ret; + } + + prependListener(name, listener) { + const ret = super.prependListener(name, listener); + if (name === 'priority' || name === 'frameError') + trackStreamListener(this, name, 1); + return ret; + } + + removeListener(name, listener) { + if (name === 'priority' || name === 'frameError') { + const before = this.listenerCount(name); + const ret = super.removeListener(name, listener); + if (this.listenerCount(name) !== before) + trackStreamListener(this, name, -1); + return ret; + } + return super.removeListener(name, listener); + } + + removeAllListeners(name) { + let priority = 0; + let frameError = 0; + if (name === undefined || name === 'priority') + priority = this.listenerCount('priority'); + if (name === undefined || name === 'frameError') + frameError = this.listenerCount('frameError'); + const ret = super.removeAllListeners(name); + if (priority !== 0) + trackStreamListener(this, 'priority', -priority); + if (frameError !== 0) + trackStreamListener(this, 'frameError', -frameError); + return ret; } [kUpdateTimer]() { @@ -2562,9 +2607,7 @@ class Http2Stream extends Duplex { // will destroy if it has been closed and there are no other open or // pending streams. Delay with setImmediate so we don't do it on the // nghttp2 stack. - setImmediate(() => { - session[kMaybeDestroy](); - }); + setImmediate(sessionMaybeDestroy, session); if (err) { if (session[kType] === NGHTTP2_SESSION_CLIENT) { if (onClientStreamErrorChannel.hasSubscribers) { @@ -2615,6 +2658,8 @@ class Http2Stream extends Duplex { } } +Http2Stream.prototype.off = Http2Stream.prototype.removeListener; + // TODO(aduh95): remove this in future semver-major Http2Stream.prototype.priority = deprecate(function priority(options) { if (this.destroyed) @@ -2649,6 +2694,10 @@ function callStreamClose(stream) { stream.close(); } +function sessionMaybeDestroy(session) { + session[kMaybeDestroy](); +} + function prepareResponseHeaders(stream, headersParam, options) { let headers; let statusCode; @@ -2961,12 +3010,14 @@ function afterOpen(session, options, headers, streamOptions, err, fd) { class ServerHttp2Stream extends Http2Stream { constructor(session, handle, id, options, headers) { - super(session, options); + super(session, options, true); handle.owner = this; this[kInit](id, handle); this[kProtocol] = headers[HTTP2_HEADER_SCHEME]; this[kAuthority] = getAuthority(headers); - this.once('finish', autoDrainReadable); + // 'finish' is only emitted once, so a regular listener is safe here and + // avoids allocating a once() wrapper for every stream. + this.on('finish', autoDrainReadable); } // True if the remote peer accepts push streams @@ -3307,7 +3358,7 @@ ServerHttp2Stream.prototype[kProceed] = ServerHttp2Stream.prototype.respond; class ClientHttp2Stream extends Http2Stream { constructor(session, handle, id, options) { - super(session, options); + super(session, options, id !== undefined); this[kState].flags |= STREAM_FLAGS_HEADERS_SENT; if (id !== undefined) this[kInit](id, handle); diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 25adc8f9697d82..9d35d58784252f 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -770,14 +770,16 @@ function buildNgHeaderString(arrayOrMap, let pseudoHeaders = ''; let count = 0; - const singles = new SafeSet(); + let singles; const sensitiveHeaders = arrayOrMap[kSensitiveHeaders] || emptyArray; - const neverIndex = sensitiveHeaders.map((v) => v.toLowerCase()); + const neverIndex = sensitiveHeaders.length === 0 ? + emptyArray : sensitiveHeaders.map((v) => v.toLowerCase()); function processHeader(key, value) { key = key.toLowerCase(); + const isSingleValueField = kSingleValueFields.has(key); const isStrictSingleValueField = strictSingleValueFields && - kSingleValueFields.has(key); + isSingleValueField; let isArray = ArrayIsArray(value); if (isArray) { switch (value.length) { @@ -795,11 +797,15 @@ function buildNgHeaderString(arrayOrMap, value = String(value); } if (isStrictSingleValueField) { - if (singles.has(key)) + if (singles === undefined) { + singles = [key]; + } else if (singles.includes(key)) { throw new ERR_HTTP2_HEADER_SINGLE_VALUE(key); - singles.add(key); + } else { + singles.push(key); + } } - const flags = neverIndex.includes(key) ? + const flags = neverIndex.length !== 0 && neverIndex.includes(key) ? kNeverIndexFlag : kNoHeaderFlags; if (key[0] === ':') { @@ -810,11 +816,15 @@ function buildNgHeaderString(arrayOrMap, count++; return; } - if (!checkIsHttpToken(key)) { - throw new ERR_INVALID_HTTP_TOKEN('Header name', key); - } - if (isIllegalConnectionSpecificHeader(key, value)) { - throw new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key); + // Well-known single-value fields are all valid HTTP tokens and none of + // them is a connection-specific header, so both checks can be skipped. + if (!isSingleValueField) { + if (!checkIsHttpToken(key)) { + throw new ERR_INVALID_HTTP_TOKEN('Header name', key); + } + if (isIllegalConnectionSpecificHeader(key, value)) { + throw new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key); + } } if (isArray) { for (let j = 0; j < value.length; ++j) { From bedf22f4d164b195e69c3e5e289f3639274c73bb Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 2 Jul 2026 19:50:55 +0200 Subject: [PATCH 2/6] http2: skip trailers round trip for compat responses The compat layer always responded with waitForTrailers set, so every response paid for a wantTrailers C++ -> JS callback, an empty sendTrailers() submission scheduled through setImmediate(), and an extra empty DATA frame on the wire, even though the vast majority of responses never register any trailers. When the headers are flushed as part of response.end() and no trailers have been registered, there is no further opportunity to add trailers, so waitForTrailers can be skipped altogether. Headers flushed early (writeHead, write, flushHeaders) keep the previous behavior so trailers can still be added while streaming. Trailers added after response.end() are now silently dropped, matching the HTTP/1 response.addTrailers() semantics. Also reuse a shared options object for Http2ServerRequest instances created without explicit options. h2load, 1 KiB response payload, -c 4 -m 100, mean of 6 alternating runs: compat API 43.1k -> 49.9k req/s (+15.7% cumulative vs main). Signed-off-by: Matteo Collina --- doc/api/http2.md | 3 +++ lib/internal/http2/compat.js | 23 +++++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index 9e9c3f8d3f0232..16f3c1d2fe4542 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -4410,6 +4410,9 @@ added: v8.4.0 This method adds HTTP trailing headers (a header but at the end of the message) to the response. +Trailers must be added before calling [`response.end()`][]; trailers added +afterwards are silently dropped. + Attempting to set a header field name or value that contains invalid characters will result in a [`TypeError`][] being thrown. diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 9f13cad794333a..1a158ad8943a38 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -300,6 +300,10 @@ function onStreamCloseRequest() { req.emit('close'); } +// Shared between all Http2ServerRequest instances created without explicit +// options; the Readable constructor only reads from it. +const kDefaultRequestOptions = { autoDestroy: false }; + function onStreamTimeoutRequest() { this[kRequest].emit('timeout'); } @@ -310,7 +314,9 @@ function onStreamTimeoutResponse() { class Http2ServerRequest extends Readable { constructor(stream, headers, options, rawHeaders) { - super({ autoDestroy: false, ...options }); + super(options === undefined ? + kDefaultRequestOptions : + { autoDestroy: false, ...options }); this[kState] = { closed: false, didRead: false, @@ -473,6 +479,8 @@ class Http2ServerResponse extends Stream { this[kState] = { closed: false, ending: false, + finishing: false, + hasTrailers: false, destroyed: false, headRequest: false, sendDate: true, @@ -584,6 +592,7 @@ class Http2ServerResponse extends Stream { name = name.trim().toLowerCase(); assertValidHeader(name, value); this[kTrailers][name] = value; + this[kState].hasTrailers = true; } addTrailers(headers) { @@ -839,6 +848,12 @@ class Http2ServerResponse extends Stream { return this; } + // If the headers have not been flushed yet, they will be flushed below + // as part of ending the response. In that case there is no further + // opportunity to add trailers, so the trailers round trip can be + // skipped entirely when none have been registered. + state.finishing = true; + if (chunk !== null && chunk !== undefined) this.write(chunk, encoding); @@ -898,7 +913,11 @@ class Http2ServerResponse extends Stream { headers[HTTP2_HEADER_STATUS] = state.statusCode; const options = { endStream: state.ending, - waitForTrailers: true, + // Only wait for trailers if some have been registered, or if the + // headers are flushed before the response is ended (in which case + // trailers may still be added during streaming). Trailers added after + // end() are dropped, matching HTTP/1 addTrailers() semantics. + waitForTrailers: state.hasTrailers || !state.finishing, sendDate: state.sendDate, }; this[kStream].respond(headers, options); From 47e843405fca35f92da6d8884efc732c8080eb7c Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 2 Jul 2026 20:52:08 +0200 Subject: [PATCH 3/6] http2: avoid per-write closures in kWriteGeneric Every _write()/_writev() on an Http2Stream allocated four closures and an anonymous nextTick callback to coordinate the write callback with the end-of-stream check. Since the stream machinery dispatches at most one write at a time, that coordination state can live on the stream's kState object instead, with shared named functions for the end check and completion logic. When trailers are pending the writable side cannot be shut down early anyway, so the end-of-stream check tick is now skipped entirely for those writes. Also pre-initialize the kState fields that used to be added dynamically (shutdownWritableCalled, fd) so hot-path stores no longer transition the object shape. h2load, 1 KiB response payload, -c 4 -m 100, mean of 6 alternating runs vs main: core API 61.0k -> 70.7k req/s (+15.9% cumulative), compat API 43.7k -> 50.4k req/s (+15.3% cumulative). Signed-off-by: Matteo Collina --- lib/internal/http2/core.js | 106 +++++++++++++++++++++++++------------ 1 file changed, 71 insertions(+), 35 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 2d2ff7cd56fc3c..7b873f0a0d801a 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -2043,6 +2043,47 @@ function shutdownWritable(callback) { return afterShutdown.call(req, 0); } +// Completes one of the two halves of a dispatched write (the write callback +// itself and the end-of-stream check); the stream machinery callback runs +// once both have finished. The state lives on stream[kState] because only a +// single write may be in flight at any given time. +function finishWrite(stream) { + const state = stream[kState]; + if (--state.writePending !== 0) + return; + const cb = state.writeCb; + state.writeCb = null; + const err = aggregateTwoErrors(state.endErr, state.writeErr); + state.writeErr = null; + state.endErr = null; + // writeGeneric does not destroy on error and + // we cannot enable autoDestroy, + // so make sure to destroy on error. + if (err) { + stream.destroy(err); + } + cb(err); +} + +// Runs on the tick after a write was dispatched: if the write turned out to +// be the last chunk of an ending writable, shut the writable side down right +// away so the final DATA frame can include the END_STREAM flag. +function endCheckNT(stream) { + const state = stream[kState]; + if (state.writeErr || + !stream._writableState.ending || + stream._writableState.buffered.length || + (state.flags & STREAM_FLAGS_HAS_TRAILERS)) { + finishWrite(stream); + return; + } + debugStreamObj(stream, 'shutting down writable on last write'); + shutdownWritable.call(stream, (err) => { + state.endErr = err; + finishWrite(stream); + }); +} + function finishSendTrailers(stream, headersList) { // The stream might be destroyed and in that case // there is nothing to do. @@ -2160,6 +2201,12 @@ class Http2Stream extends Duplex { writeQueueSize: 0, trailersReady: false, endAfterHeaders: false, + writeCb: null, + writeErr: null, + endErr: null, + writePending: 0, + shutdownWritableCalled: false, + fd: -1, }; // Fields used by the compat API to avoid megamorphisms. @@ -2391,45 +2438,34 @@ class Http2Stream extends Duplex { if (!this.headersSent) this[kProceed](); - let req; + // The stream machinery dispatches at most one _write()/_writev() at a + // time, so the coordination state between the write callback and the + // end-of-stream check below can live on the stream state instead of + // being captured by per-write closures. + const state = this[kState]; + state.writeCb = cb; + state.writeErr = null; + state.endErr = null; + + if (state.flags & STREAM_FLAGS_HAS_TRAILERS) { + // Trailers are pending, so the writable side cannot be shut down + // early anyway; there is no point in scheduling the end check. + state.writePending = 1; + } else { + state.writePending = 2; + // Shutdown write stream right after last chunk is sent + // so final DATA frame can include END_STREAM flag + process.nextTick(endCheckNT, this); + } - let waitingForWriteCallback = true; - let waitingForEndCheck = true; - let writeCallbackErr; - let endCheckCallbackErr; - const done = () => { - if (waitingForEndCheck || waitingForWriteCallback) return; - const err = aggregateTwoErrors(endCheckCallbackErr, writeCallbackErr); - // writeGeneric does not destroy on error and - // we cannot enable autoDestroy, - // so make sure to destroy on error. - if (err) { - this.destroy(err); - } - cb(err); - }; + // This is invoked both as a method on the write req and as a plain + // call, so the stream has to be captured here. const writeCallback = (err) => { - waitingForWriteCallback = false; - writeCallbackErr = err; - done(); - }; - const endCheckCallback = (err) => { - waitingForEndCheck = false; - endCheckCallbackErr = err; - done(); + state.writeErr = err; + finishWrite(this); }; - // Shutdown write stream right after last chunk is sent - // so final DATA frame can include END_STREAM flag - process.nextTick(() => { - if (writeCallbackErr || - !this._writableState.ending || - this._writableState.buffered.length || - (this[kState].flags & STREAM_FLAGS_HAS_TRAILERS)) - return endCheckCallback(); - debugStreamObj(this, 'shutting down writable on last write'); - shutdownWritable.call(this, endCheckCallback); - }); + let req; if (writev) req = writevGeneric(this, data, writeCallback); else From 2739f6310d41f3ba93dab9afcbfcc163f4c5bed6 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 2 Jul 2026 21:54:14 +0200 Subject: [PATCH 4/6] http2: finish empty trailers natively for compat streams When the compat layer flushes response headers before the response is ended (writeHead(), write(), flushHeaders()), it must keep waitForTrailers so that trailers can still be added while streaming. As a result, every such response paid for a wantTrailers C++ -> JS callback, an empty sendTrailers() with its setImmediate(), and a trailers() call back into C++, even though most responses never register any trailers. Introduce STREAM_OPTION_AUTO_EMPTY_TRAILERS: when set and no trailers have been handed to the native side by the time the final DATA frame is sent, the stream is finished directly in C++ with the same empty DATA frame carrying END_STREAM that the JS path would have produced, without calling into JS at all. The compat layer enables this mode whenever it responds with waitForTrailers and no trailers registered yet; a later setTrailer() call flips the stream back to JS-managed trailers through a new disableAutoTrailers() binding, so streaming trailers keep working unchanged. The wire format is identical in all cases. h2load -c 4 -m 100, 1 KiB payload, mean of 8 alternating runs against the previous commit: compat writeHead()+end() 47.8k -> 50.2k req/s (+5.0%); multi-write streaming responses +1%. Signed-off-by: Matteo Collina --- lib/internal/http2/compat.js | 27 ++++-- lib/internal/http2/core.js | 19 ++++ lib/internal/http2/util.js | 4 + src/node_http2.cc | 87 ++++++++++++++----- src/node_http2.h | 23 ++++- ...ompat-serverresponse-trailers-streaming.js | 70 +++++++++++++++ 6 files changed, 203 insertions(+), 27 deletions(-) create mode 100644 test/parallel/test-http2-compat-serverresponse-trailers-streaming.js diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 1a158ad8943a38..d9c123ee846b90 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -52,6 +52,8 @@ const { validateObject, } = require('internal/validators'); const { + kAutoEmptyTrailers, + kDisableAutoTrailers, kSocket, kRequest, kProxySocket, @@ -592,7 +594,15 @@ class Http2ServerResponse extends Stream { name = name.trim().toLowerCase(); assertValidHeader(name, value); this[kTrailers][name] = value; - this[kState].hasTrailers = true; + const state = this[kState]; + if (!state.hasTrailers) { + state.hasTrailers = true; + // If the response headers were already flushed with auto-empty + // trailers enabled, tell the stream to hand the trailers back to JS. + const stream = this[kStream]; + if (stream.headersSent) + stream[kDisableAutoTrailers](); + } } addTrailers(headers) { @@ -911,13 +921,18 @@ class Http2ServerResponse extends Stream { const state = this[kState]; const headers = this[kHeaders]; headers[HTTP2_HEADER_STATUS] = state.statusCode; + // Only wait for trailers if some have been registered, or if the + // headers are flushed before the response is ended (in which case + // trailers may still be added during streaming). Trailers added after + // end() are dropped, matching HTTP/1 addTrailers() semantics. + const waitForTrailers = state.hasTrailers || !state.finishing; const options = { endStream: state.ending, - // Only wait for trailers if some have been registered, or if the - // headers are flushed before the response is ended (in which case - // trailers may still be added during streaming). Trailers added after - // end() are dropped, matching HTTP/1 addTrailers() semantics. - waitForTrailers: state.hasTrailers || !state.finishing, + waitForTrailers, + // When no trailers have been registered yet, let the native side + // finish the stream on its own if none show up by the time the last + // DATA frame is sent (see setTrailer()). + [kAutoEmptyTrailers]: waitForTrailers && !state.hasTrailers, sendDate: state.sendDate, }; this[kStream].respond(headers, options); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7b873f0a0d801a..59f9bd5dd87178 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -150,6 +150,8 @@ const { getStreamState, isPayloadMeaningless, kAuthority, + kAutoEmptyTrailers, + kDisableAutoTrailers, kSensitiveHeaders, kStrictSingleValueFields, kSocket, @@ -334,6 +336,7 @@ const { STREAM_OPTION_EMPTY_PAYLOAD, STREAM_OPTION_GET_TRAILERS, + STREAM_OPTION_AUTO_EMPTY_TRAILERS, } = constants; const STREAM_FLAGS_PENDING = 0x0; @@ -2520,6 +2523,17 @@ class Http2Stream extends Duplex { } } + // Called by the compat layer when trailers are registered after the + // response headers were already sent with auto-empty trailers enabled + // (see STREAM_OPTION_AUTO_EMPTY_TRAILERS); switches the stream back to + // JS-managed trailers. + [kDisableAutoTrailers]() { + const handle = this[kHandle]; + if (this.destroyed || handle === undefined) + return; + handle.disableAutoTrailers(); + } + sendTrailers(headers) { if (this.destroyed || this.closed) throw new ERR_HTTP2_INVALID_STREAM(); @@ -3187,6 +3201,11 @@ class ServerHttp2Stream extends Http2Stream { if (options.waitForTrailers) { streamOptions |= STREAM_OPTION_GET_TRAILERS; state.flags |= STREAM_FLAGS_HAS_TRAILERS; + // Internal mode used by the compat layer: if no trailers have been + // registered by the time the final DATA frame is sent, the native + // side finishes the stream without calling back into JS at all. + if (options[kAutoEmptyTrailers]) + streamOptions |= STREAM_OPTION_AUTO_EMPTY_TRAILERS; } const { diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 9d35d58784252f..159ba5bd531524 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -38,6 +38,8 @@ const { } = require('internal/errors'); const kAuthority = Symbol('authority'); +const kAutoEmptyTrailers = Symbol('autoEmptyTrailers'); +const kDisableAutoTrailers = Symbol('disableAutoTrailers'); const kSensitiveHeaders = Symbol('sensitiveHeaders'); const kStrictSingleValueFields = Symbol('strictSingleValueFields'); const kSocket = Symbol('socket'); @@ -991,6 +993,8 @@ module.exports = { getStreamState, isPayloadMeaningless, kAuthority, + kAutoEmptyTrailers, + kDisableAutoTrailers, kSensitiveHeaders, kStrictSingleValueFields, kSocket, diff --git a/src/node_http2.cc b/src/node_http2.cc index fca840c0fa03ad..710afee2da845c 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2308,6 +2308,9 @@ Http2Stream::Http2Stream(Http2Session* session, if (options & STREAM_OPTION_GET_TRAILERS) set_has_trailers(); + if (options & STREAM_OPTION_AUTO_EMPTY_TRAILERS) + set_auto_empty_trailers(); + PushStreamListener(&stream_listener_); if (options & STREAM_OPTION_EMPTY_PAYLOAD) @@ -2435,6 +2438,9 @@ int Http2Stream::SubmitResponse(const Http2Headers& headers, int options) { if (options & STREAM_OPTION_GET_TRAILERS) set_has_trailers(); + if (options & STREAM_OPTION_AUTO_EMPTY_TRAILERS) + set_auto_empty_trailers(); + if (!is_writable()) options |= STREAM_OPTION_EMPTY_PAYLOAD; @@ -2468,39 +2474,67 @@ int Http2Stream::SubmitInfo(const Http2Headers& headers) { } void Http2Stream::OnTrailers() { + CHECK(!this->is_destroyed()); + set_has_trailers(false); + if (!auto_empty_trailers()) { + EmitWantTrailers(); + return; + } + // The JS side has not registered any trailers, so the stream can be + // finished without calling into JS at all. The empty DATA frame cannot be + // submitted synchronously because OnTrailers() runs from inside the data + // source read callback for the final DATA frame; defer it to the next + // turn of the event loop, just like the JS sendTrailers() path does. + Debug(this, "auto-submitting empty trailers"); + env()->SetImmediate( + [self = BaseObjectPtr(this)](Environment* env) { + if (self->is_destroyed()) return; + // Hand control back to the JS side if trailers were registered in + // the meantime or if submitting the empty DATA frame failed. + if (!self->auto_empty_trailers() || self->SubmitEmptyTrailers() != 0) + self->EmitWantTrailers(); + }); +} + +void Http2Stream::EmitWantTrailers() { Debug(this, "let javascript know we are ready for trailers"); CHECK(!this->is_destroyed()); Isolate* isolate = env()->isolate(); HandleScope scope(isolate); Local context = env()->context(); Context::Scope context_scope(context); - set_has_trailers(false); MakeCallback(env()->http2session_on_stream_trailers_function(), 0, nullptr); } -// Submit informational headers for a stream. +// Sending an empty trailers frame poses problems in Safari, Edge & IE. +// Instead we can just send an empty data frame with NGHTTP2_FLAG_END_STREAM +// to indicate that the stream is ready to be closed. +int Http2Stream::SubmitEmptyTrailers() { + CHECK(!this->is_destroyed()); + Http2Scope h2scope(this); + Debug(this, "sending empty trailers"); + Http2Stream::Provider::Stream prov(this, 0); + int ret = nghttp2_submit_data( + session_->session(), + NGHTTP2_FLAG_END_STREAM, + id_, + *prov); + CHECK_NE(ret, NGHTTP2_ERR_NOMEM); + return ret; +} + +// Submit trailing headers for a stream. int Http2Stream::SubmitTrailers(const Http2Headers& headers) { CHECK(!this->is_destroyed()); + if (headers.length() == 0) + return SubmitEmptyTrailers(); Http2Scope h2scope(this); Debug(this, "sending %d trailers", headers.length()); - int ret; - // Sending an empty trailers frame poses problems in Safari, Edge & IE. - // Instead we can just send an empty data frame with NGHTTP2_FLAG_END_STREAM - // to indicate that the stream is ready to be closed. - if (headers.length() == 0) { - Http2Stream::Provider::Stream prov(this, 0); - ret = nghttp2_submit_data( - session_->session(), - NGHTTP2_FLAG_END_STREAM, - id_, - *prov); - } else { - ret = nghttp2_submit_trailer( - session_->session(), - id_, - headers.data(), - headers.length()); - } + int ret = nghttp2_submit_trailer( + session_->session(), + id_, + headers.data(), + headers.length()); CHECK_NE(ret, NGHTTP2_ERR_NOMEM); return ret; } @@ -3110,6 +3144,15 @@ void Http2Stream::Trailers(const FunctionCallbackInfo& args) { stream->SubmitTrailers(Http2Headers(env, headers))); } +// Called by the JS layer when trailers are registered after the response +// headers were already submitted with STREAM_OPTION_AUTO_EMPTY_TRAILERS set, +// so that the trailers are handed back to JS instead of being auto-emptied. +void Http2Stream::DisableAutoTrailers(const FunctionCallbackInfo& args) { + Http2Stream* stream; + ASSIGN_OR_RETURN_UNWRAP(&stream, args.This()); + stream->set_auto_empty_trailers(false); +} + // Grab the numeric id of the Http2Stream void Http2Stream::GetID(const FunctionCallbackInfo& args) { Http2Stream* stream; @@ -3547,6 +3590,10 @@ void Initialize(Local target, SetProtoMethod(isolate, stream, "pushPromise", Http2Stream::PushPromise); SetProtoMethod(isolate, stream, "info", Http2Stream::Info); SetProtoMethod(isolate, stream, "trailers", Http2Stream::Trailers); + SetProtoMethod(isolate, + stream, + "disableAutoTrailers", + Http2Stream::DisableAutoTrailers); SetProtoMethod(isolate, stream, "respond", Http2Stream::Respond); SetProtoMethod(isolate, stream, "rstStream", Http2Stream::RstStream); SetProtoMethod(isolate, stream, "refreshState", Http2Stream::RefreshState); diff --git a/src/node_http2.h b/src/node_http2.h index c9957cb559b323..670c5b5db81f70 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -57,6 +57,10 @@ constexpr int STREAM_OPTION_EMPTY_PAYLOAD = 0x1; // Stream might have trailing headers constexpr int STREAM_OPTION_GET_TRAILERS = 0x2; +// Stream may finish with an empty DATA frame carrying END_STREAM without +// calling back into JS, unless trailers are registered before then +constexpr int STREAM_OPTION_AUTO_EMPTY_TRAILERS = 0x4; + // Http2Stream internal states constexpr int kStreamStateNone = 0x0; constexpr int kStreamStateShut = 0x1; @@ -66,6 +70,7 @@ constexpr int kStreamStateClosed = 0x8; constexpr int kStreamStateDestroyed = 0x10; constexpr int kStreamStateTrailers = 0x20; constexpr int kStreamStatePeerReset = 0x40; +constexpr int kStreamStateAutoEmptyTrailers = 0x80; // Http2Session internal states constexpr int kSessionStateNone = 0x0; @@ -310,7 +315,9 @@ class Http2Stream : public AsyncWrap, // Submit trailing headers for this stream int SubmitTrailers(const Http2Headers& headers); + int SubmitEmptyTrailers(); void OnTrailers(); + void EmitWantTrailers(); // Submit a PRIORITY frame for this stream int SubmitPriority(const Http2Priority& priority, bool silent = false); @@ -368,6 +375,17 @@ class Http2Stream : public AsyncWrap, flags_ &= ~kStreamStateTrailers; } + bool auto_empty_trailers() const { + return flags_ & kStreamStateAutoEmptyTrailers; + } + + void set_auto_empty_trailers(bool on = true) { + if (on) + flags_ |= kStreamStateAutoEmptyTrailers; + else + flags_ &= ~kStreamStateAutoEmptyTrailers; + } + void set_closed() { flags_ |= kStreamStateClosed; } @@ -463,6 +481,8 @@ class Http2Stream : public AsyncWrap, static void RefreshState(const v8::FunctionCallbackInfo& args); static void Info(const v8::FunctionCallbackInfo& args); static void Trailers(const v8::FunctionCallbackInfo& args); + static void DisableAutoTrailers( + const v8::FunctionCallbackInfo& args); static void Respond(const v8::FunctionCallbackInfo& args); static void RstStream(const v8::FunctionCallbackInfo& args); @@ -1119,7 +1139,8 @@ class Origins { V(NGHTTP2_ERR_STREAM_CLOSED) \ V(NGHTTP2_ERR_NOMEM) \ V(STREAM_OPTION_EMPTY_PAYLOAD) \ - V(STREAM_OPTION_GET_TRAILERS) + V(STREAM_OPTION_GET_TRAILERS) \ + V(STREAM_OPTION_AUTO_EMPTY_TRAILERS) #define HTTP2_ERROR_CODES(V) \ V(NGHTTP2_NO_ERROR) \ diff --git a/test/parallel/test-http2-compat-serverresponse-trailers-streaming.js b/test/parallel/test-http2-compat-serverresponse-trailers-streaming.js new file mode 100644 index 00000000000000..b15cc4680b00bc --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-trailers-streaming.js @@ -0,0 +1,70 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const h2 = require('http2'); + +// Regression test for the auto-empty-trailers optimization: when the +// response headers are flushed before the response is ended (streaming +// mode), trailers registered afterwards must still be sent, and responses +// that never register trailers must complete normally. + +const server = h2.createServer(); +server.listen(0, common.mustCall(() => { + const port = server.address().port; + server.on('request', (request, response) => { + if (request.url === '/trailers') { + response.writeHead(200); + response.write('hello'); + // Trailers registered after the headers were already flushed. + response.setTrailer('x-checksum', 'abc'); + response.addTrailers({ 'x-count': 2 }); + response.end('world'); + } else { + response.writeHead(200); + response.write('no'); + response.end('trailers'); + } + }); + + const client = h2.connect(`http://localhost:${port}`); + + { + const request = client.request({ ':path': '/trailers' }); + let body = ''; + request.setEncoding('utf8'); + request.on('data', (chunk) => body += chunk); + request.on('trailers', common.mustCall((trailers) => { + assert.strictEqual(trailers['x-checksum'], 'abc'); + assert.strictEqual(trailers['x-count'], '2'); + })); + request.on('end', common.mustCall(() => { + assert.strictEqual(body, 'helloworld'); + maybeClose(); + })); + request.end(); + } + + { + const request = client.request({ ':path': '/plain' }); + let body = ''; + request.setEncoding('utf8'); + request.on('data', (chunk) => body += chunk); + request.on('trailers', common.mustNotCall()); + request.on('end', common.mustCall(() => { + assert.strictEqual(body, 'notrailers'); + maybeClose(); + })); + request.end(); + } + + let remaining = 2; + function maybeClose() { + if (--remaining === 0) { + client.close(); + server.close(); + } + } +})); From bfd97c5b6769437262066f4a92142c71b253e0a9 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 3 Jul 2026 10:18:24 +0200 Subject: [PATCH 5/6] http2: reduce scheduled callbacks per request Two per-request scheduling eliminations: - The end-of-stream check that lets the final DATA frame carry the END_STREAM flag was scheduled with process.nextTick() on every write. When the write is dispatched from inside end() - the common case of end(chunk) - the check can instead run synchronously once end() returns and the writable state has settled. An end() override marks the stream while the base method runs, and [kWriteGeneric] hands the check back to it instead of scheduling a tick. Writes not tied to end() keep the nextTick behavior. - Every stream destruction scheduled a setImmediate() to ask the session to clean itself up, but Http2Session[kMaybeDestroy] is a no-op unless the session is closed and has no remaining streams. Gate the setImmediate() on that condition: session.close() runs its own check, and the native side notifies again through ongracefulclosecomplete once pending data is flushed. The wire format is unchanged (verified byte-identical h2load traffic), and the END_STREAM merge is preserved. h2load -c 4 -m 100, 1 KiB payload, alternating runs vs the previous commit: consistently around +1% (within run-to-run noise on any single set, positive across 42 paired samples). Signed-off-by: Matteo Collina --- lib/internal/http2/core.js | 39 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 59f9bd5dd87178..f864dc1b93e692 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -2208,6 +2208,8 @@ class Http2Stream extends Duplex { writeErr: null, endErr: null, writePending: 0, + endInProgress: false, + endCheckOwed: false, shutdownWritableCalled: false, fd: -1, }; @@ -2454,6 +2456,12 @@ class Http2Stream extends Duplex { // Trailers are pending, so the writable side cannot be shut down // early anyway; there is no point in scheduling the end check. state.writePending = 1; + } else if (state.endInProgress) { + // This write was dispatched from inside end(), which makes it the + // final chunk; end() runs the end-of-stream check synchronously once + // the stream machinery has settled, avoiding a nextTick per write. + state.writePending = 2; + state.endCheckOwed = true; } else { state.writePending = 2; // Shutdown write stream right after last chunk is sent @@ -2494,6 +2502,25 @@ class Http2Stream extends Duplex { this[kWriteGeneric](true, data, '', cb); } + end(chunk, encoding, cb) { + const state = this[kState]; + // Any write dispatched while end() runs is the final chunk. Marking + // that lets [kWriteGeneric] hand its end-of-stream check back to us to + // run synchronously below (once the writable state has settled) + // instead of scheduling a nextTick for it. + state.endInProgress = true; + try { + super.end(chunk, encoding, cb); + } finally { + state.endInProgress = false; + if (state.endCheckOwed) { + state.endCheckOwed = false; + endCheckNT(this); + } + } + return this; + } + _final(cb) { if (this.pending) { this.once('ready', () => this._final(cb)); @@ -2656,8 +2683,16 @@ class Http2Stream extends Duplex { // gives the session the opportunity to clean itself up. The session // will destroy if it has been closed and there are no other open or // pending streams. Delay with setImmediate so we don't do it on the - // nghttp2 stack. - setImmediate(sessionMaybeDestroy, session); + // nghttp2 stack. When the session is not closed (or other streams are + // still around), [kMaybeDestroy] would be a no-op, so skip scheduling + // it altogether: session.close() runs its own check, and the native + // side notifies again through ongracefulclosecomplete once all pending + // data has been flushed. + if (session.closed && + sessionState.streams.size === 0 && + sessionState.pendingStreams.size === 0) { + setImmediate(sessionMaybeDestroy, session); + } if (err) { if (session[kType] === NGHTTP2_SESSION_CLIENT) { if (onClientStreamErrorChannel.hasSubscribers) { From 77ecd83cfd1ba46eea76c8e93a6aab5e5d6649bc Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 3 Jul 2026 10:47:40 +0200 Subject: [PATCH 6/6] http2: avoid copying the options in respond() respond() copied the user-provided options object on every call just so it could normalize and locally flip options.endStream, and prepareResponseHeadersObject() then looked the :status and date fields up again on the dictionary-mode null-prototype headers copy it had just built. Use a local variable for endStream and pick up :status/date while copying the headers instead. No measurable throughput change on its own; this removes an object clone and several dictionary-mode property lookups per response. Signed-off-by: Matteo Collina --- lib/internal/http2/core.js | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index f864dc1b93e692..d65e9fe5a7623c 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -2813,31 +2813,35 @@ function prepareResponseHeaders(stream, headersParam, options) { function prepareResponseHeadersObject(oldHeaders, options) { assertIsObject(oldHeaders, 'headers', ['Object', 'Array']); const headers = { __proto__: null }; + let statusCode; + let hasDate = false; if (oldHeaders !== null && oldHeaders !== undefined) { // This loop is here for performance reason. Do not change. + // The :status and date fields are picked up while copying so they do + // not have to be looked up again on the null-prototype copy. for (const key in oldHeaders) { if (ObjectHasOwn(oldHeaders, key)) { - headers[key] = oldHeaders[key]; + const value = oldHeaders[key]; + headers[key] = value; + if (key === HTTP2_HEADER_STATUS) + statusCode = value; + else if (key === HTTP2_HEADER_DATE) + hasDate = value != null; } } headers[kSensitiveHeaders] = oldHeaders[kSensitiveHeaders]; } - const statusCode = - headers[HTTP2_HEADER_STATUS] = - headers[HTTP2_HEADER_STATUS] | 0 || HTTP_STATUS_OK; + statusCode = headers[HTTP2_HEADER_STATUS] = statusCode | 0 || HTTP_STATUS_OK; - if (options.sendDate == null || options.sendDate) { - headers[HTTP2_HEADER_DATE] ??= utcDate(); + if (!hasDate && (options.sendDate == null || options.sendDate)) { + headers[HTTP2_HEADER_DATE] = utcDate(); } validatePreparedResponseHeaders(headers, statusCode); - return { - headers, - statusCode: headers[HTTP2_HEADER_STATUS], - }; + return { headers, statusCode }; } function prepareResponseHeadersArray(headers, options) { @@ -3222,15 +3226,17 @@ class ServerHttp2Stream extends Http2Stream { const state = this[kState]; assertIsObject(options, 'options'); - options = { ...options }; + // The options are only read, never mutated, so the user-provided object + // can be used directly instead of copying it. + options ??= kEmptyObject; debugStreamObj(this, 'initiating response'); this[kUpdateTimer](); - options.endStream = !!options.endStream; + const endStream = !!options.endStream; let streamOptions = 0; - if (options.endStream) + if (endStream) streamOptions |= STREAM_OPTION_EMPTY_PAYLOAD; if (options.waitForTrailers) { @@ -3253,12 +3259,11 @@ class ServerHttp2Stream extends Http2Stream { // Close the writable side if the endStream option is set or status // is one of known codes with no payload, or it's a head request - if (!!options.endStream || + if (endStream || statusCode === HTTP_STATUS_NO_CONTENT || statusCode === HTTP_STATUS_RESET_CONTENT || statusCode === HTTP_STATUS_NOT_MODIFIED || this.headRequest === true) { - options.endStream = true; this.end(); }