Skip to content

Commit bedf22f

Browse files
committed
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 <hello@matteocollina.com>
1 parent 331609b commit bedf22f

2 files changed

Lines changed: 24 additions & 2 deletions

File tree

doc/api/http2.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4410,6 +4410,9 @@ added: v8.4.0
44104410
This method adds HTTP trailing headers (a header but at the end of the
44114411
message) to the response.
44124412

4413+
Trailers must be added before calling [`response.end()`][]; trailers added
4414+
afterwards are silently dropped.
4415+
44134416
Attempting to set a header field name or value that contains invalid characters
44144417
will result in a [`TypeError`][] being thrown.
44154418

lib/internal/http2/compat.js

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,10 @@ function onStreamCloseRequest() {
300300
req.emit('close');
301301
}
302302

303+
// Shared between all Http2ServerRequest instances created without explicit
304+
// options; the Readable constructor only reads from it.
305+
const kDefaultRequestOptions = { autoDestroy: false };
306+
303307
function onStreamTimeoutRequest() {
304308
this[kRequest].emit('timeout');
305309
}
@@ -310,7 +314,9 @@ function onStreamTimeoutResponse() {
310314

311315
class Http2ServerRequest extends Readable {
312316
constructor(stream, headers, options, rawHeaders) {
313-
super({ autoDestroy: false, ...options });
317+
super(options === undefined ?
318+
kDefaultRequestOptions :
319+
{ autoDestroy: false, ...options });
314320
this[kState] = {
315321
closed: false,
316322
didRead: false,
@@ -473,6 +479,8 @@ class Http2ServerResponse extends Stream {
473479
this[kState] = {
474480
closed: false,
475481
ending: false,
482+
finishing: false,
483+
hasTrailers: false,
476484
destroyed: false,
477485
headRequest: false,
478486
sendDate: true,
@@ -584,6 +592,7 @@ class Http2ServerResponse extends Stream {
584592
name = name.trim().toLowerCase();
585593
assertValidHeader(name, value);
586594
this[kTrailers][name] = value;
595+
this[kState].hasTrailers = true;
587596
}
588597

589598
addTrailers(headers) {
@@ -839,6 +848,12 @@ class Http2ServerResponse extends Stream {
839848
return this;
840849
}
841850

851+
// If the headers have not been flushed yet, they will be flushed below
852+
// as part of ending the response. In that case there is no further
853+
// opportunity to add trailers, so the trailers round trip can be
854+
// skipped entirely when none have been registered.
855+
state.finishing = true;
856+
842857
if (chunk !== null && chunk !== undefined)
843858
this.write(chunk, encoding);
844859

@@ -898,7 +913,11 @@ class Http2ServerResponse extends Stream {
898913
headers[HTTP2_HEADER_STATUS] = state.statusCode;
899914
const options = {
900915
endStream: state.ending,
901-
waitForTrailers: true,
916+
// Only wait for trailers if some have been registered, or if the
917+
// headers are flushed before the response is ended (in which case
918+
// trailers may still be added during streaming). Trailers added after
919+
// end() are dropped, matching HTTP/1 addTrailers() semantics.
920+
waitForTrailers: state.hasTrailers || !state.finishing,
902921
sendDate: state.sendDate,
903922
};
904923
this[kStream].respond(headers, options);

0 commit comments

Comments
 (0)