Skip to content

Commit 03e7945

Browse files
mcollinanodejs-github-bot
authored andcommitted
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 <hello@matteocollina.com> PR-URL: #64265 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 2e8f4d7 commit 03e7945

3 files changed

Lines changed: 123 additions & 61 deletions

File tree

lib/internal/http2/compat.js

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -300,11 +300,12 @@ function onStreamCloseRequest() {
300300
req.emit('close');
301301
}
302302

303-
function onStreamTimeout(kind) {
304-
return function onStreamTimeout() {
305-
const obj = this[kind];
306-
obj.emit('timeout');
307-
};
303+
function onStreamTimeoutRequest() {
304+
this[kRequest].emit('timeout');
305+
}
306+
307+
function onStreamTimeoutResponse() {
308+
this[kResponse].emit('timeout');
308309
}
309310

310311
class Http2ServerRequest extends Readable {
@@ -332,7 +333,7 @@ class Http2ServerRequest extends Readable {
332333
stream.on('error', onStreamError);
333334
stream.on('aborted', onStreamAbortedRequest);
334335
stream.on('close', onStreamCloseRequest);
335-
stream.on('timeout', onStreamTimeout(kRequest));
336+
stream.on('timeout', onStreamTimeoutRequest);
336337
this.on('pause', onRequestPause);
337338
this.on('resume', onRequestResume);
338339
}
@@ -488,7 +489,7 @@ class Http2ServerResponse extends Stream {
488489
stream.on('aborted', onStreamAbortedResponse);
489490
stream.on('close', onStreamCloseResponse);
490491
stream.on('wantTrailers', onStreamTrailersReady);
491-
stream.on('timeout', onStreamTimeout(kResponse));
492+
stream.on('timeout', onStreamTimeoutResponse);
492493
}
493494

494495
// User land modules such as finalhandler just check truthiness of this

lib/internal/http2/core.js

Lines changed: 94 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -559,29 +559,19 @@ function sessionListenerRemoved(name) {
559559
}
560560

561561
// Also keep track of listeners for the Http2Stream instances, as some events
562-
// are emitted on those objects.
563-
function streamListenerAdded(name) {
564-
const session = this[kSession];
565-
if (!session) return;
566-
switch (name) {
567-
case 'priority':
568-
session[kNativeFields][kSessionPriorityListenerCount]++;
569-
break;
570-
case 'frameError':
571-
session[kNativeFields][kSessionFrameErrorListenerCount]++;
572-
break;
573-
}
574-
}
575-
576-
function streamListenerRemoved(name) {
577-
const session = this[kSession];
562+
// are emitted on those objects. Instead of subscribing to 'newListener' and
563+
// 'removeListener' (which makes every listener add/remove on every stream
564+
// emit an extra tracking event), Http2Stream overrides the EventEmitter
565+
// methods and updates the counts directly.
566+
function trackStreamListener(stream, name, delta) {
567+
const session = stream[kSession];
578568
if (!session) return;
579569
switch (name) {
580570
case 'priority':
581-
session[kNativeFields][kSessionPriorityListenerCount]--;
571+
session[kNativeFields][kSessionPriorityListenerCount] += delta;
582572
break;
583573
case 'frameError':
584-
session[kNativeFields][kSessionFrameErrorListenerCount]--;
574+
session[kNativeFields][kSessionFrameErrorListenerCount] += delta;
585575
break;
586576
}
587577
}
@@ -595,6 +585,20 @@ function onPing(payload) {
595585
session.emit('ping', payload);
596586
}
597587

588+
function streamNaturalCloseSettled(stream) {
589+
return (stream._readableState.endEmitted ||
590+
!!stream._readableState.errored) &&
591+
(stream._writableState.finished ||
592+
!!stream._writableState.errored);
593+
}
594+
595+
// Shared 'end'/'finish'/'error' listener for the natural-close path of
596+
// onStreamClose(). Named and reused to avoid per-stream closures.
597+
function maybeDestroyNaturalClose() {
598+
if (!this.destroyed && streamNaturalCloseSettled(this))
599+
this.destroy();
600+
}
601+
598602
// Fired by C++ when nghttp2's on_stream_close fires. `peerReset` is
599603
// true when the peer sent a RST_STREAM frame - peer RST_STREAM(NO_ERROR)
600604
// is otherwise indistinguishable from a clean END_STREAM exchange at
@@ -655,22 +659,14 @@ function onStreamClose(code, peerReset) {
655659
// errored readable won't fire 'end' - and on a Duplex a writable
656660
// error propagates to readable.errored, blocking 'end' too. Treat
657661
// either side's errored state as settled.
658-
const readDone = () => stream._readableState.endEmitted ||
659-
!!stream._readableState.errored;
660-
const writeDone = () => stream._writableState.finished ||
661-
!!stream._writableState.errored;
662-
if (readDone() && writeDone()) {
662+
if (streamNaturalCloseSettled(stream)) {
663663
stream.destroy();
664664
return true;
665665
}
666666

667-
const maybeDestroy = () => {
668-
if (!stream.destroyed && readDone() && writeDone())
669-
stream.destroy();
670-
};
671-
stream.once('end', maybeDestroy);
672-
stream.once('finish', maybeDestroy);
673-
stream.once('error', maybeDestroy);
667+
stream.on('end', maybeDestroyNaturalClose);
668+
stream.on('finish', maybeDestroyNaturalClose);
669+
stream.on('error', maybeDestroyNaturalClose);
674670
if (stream[kSession][kType] === NGHTTP2_SESSION_SERVER &&
675671
!stream[kState].didRead &&
676672
stream.readableFlowing === null) {
@@ -2015,12 +2011,14 @@ function streamOnPause() {
20152011
this[kHandle].readStop();
20162012
}
20172013

2014+
function streamOnFinishMaybeDestroy() {
2015+
this[kMaybeDestroy]();
2016+
}
2017+
20182018
function afterShutdown(status) {
20192019
const stream = this.handle[kOwner];
20202020
if (stream) {
2021-
stream.on('finish', () => {
2022-
stream[kMaybeDestroy]();
2023-
});
2021+
stream.on('finish', streamOnFinishMaybeDestroy);
20242022
}
20252023
// Currently this status value is unused
20262024
this.callback();
@@ -2132,7 +2130,7 @@ function finishCloseStream(code) {
21322130
// An Http2Stream is a Duplex stream that is backed by a
21332131
// node::http2::Http2Stream handle implementing StreamBase.
21342132
class Http2Stream extends Duplex {
2135-
constructor(session, options) {
2133+
constructor(session, options, hasHandle = false) {
21362134
options.allowHalfOpen = true;
21372135
options.decodeStrings = false;
21382136
options.autoDestroy = false;
@@ -2144,7 +2142,10 @@ class Http2Stream extends Duplex {
21442142
// been assigned.
21452143
this.cork();
21462144
this[kSession] = session;
2147-
session[kState].pendingStreams.add(this);
2145+
// Streams constructed with their native handle already available (e.g.
2146+
// server streams) are initialized immediately and never become pending.
2147+
if (!hasHandle)
2148+
session[kState].pendingStreams.add(this);
21482149

21492150
// Allow our logic for determining whether any reads have happened to
21502151
// work in all situations. This is similar to what we do in _http_incoming.
@@ -2166,9 +2167,53 @@ class Http2Stream extends Duplex {
21662167
this[kProxySocket] = null;
21672168

21682169
this.on('pause', streamOnPause);
2170+
}
21692171

2170-
this.on('newListener', streamListenerAdded);
2171-
this.on('removeListener', streamListenerRemoved);
2172+
addListener(name, listener) {
2173+
const ret = super.addListener(name, listener);
2174+
if (name === 'priority' || name === 'frameError')
2175+
trackStreamListener(this, name, 1);
2176+
return ret;
2177+
}
2178+
2179+
on(name, listener) {
2180+
const ret = super.on(name, listener);
2181+
if (name === 'priority' || name === 'frameError')
2182+
trackStreamListener(this, name, 1);
2183+
return ret;
2184+
}
2185+
2186+
prependListener(name, listener) {
2187+
const ret = super.prependListener(name, listener);
2188+
if (name === 'priority' || name === 'frameError')
2189+
trackStreamListener(this, name, 1);
2190+
return ret;
2191+
}
2192+
2193+
removeListener(name, listener) {
2194+
if (name === 'priority' || name === 'frameError') {
2195+
const before = this.listenerCount(name);
2196+
const ret = super.removeListener(name, listener);
2197+
if (this.listenerCount(name) !== before)
2198+
trackStreamListener(this, name, -1);
2199+
return ret;
2200+
}
2201+
return super.removeListener(name, listener);
2202+
}
2203+
2204+
removeAllListeners(name) {
2205+
let priority = 0;
2206+
let frameError = 0;
2207+
if (name === undefined || name === 'priority')
2208+
priority = this.listenerCount('priority');
2209+
if (name === undefined || name === 'frameError')
2210+
frameError = this.listenerCount('frameError');
2211+
const ret = super.removeAllListeners(name);
2212+
if (priority !== 0)
2213+
trackStreamListener(this, 'priority', -priority);
2214+
if (frameError !== 0)
2215+
trackStreamListener(this, 'frameError', -frameError);
2216+
return ret;
21722217
}
21732218

21742219
[kUpdateTimer]() {
@@ -2562,9 +2607,7 @@ class Http2Stream extends Duplex {
25622607
// will destroy if it has been closed and there are no other open or
25632608
// pending streams. Delay with setImmediate so we don't do it on the
25642609
// nghttp2 stack.
2565-
setImmediate(() => {
2566-
session[kMaybeDestroy]();
2567-
});
2610+
setImmediate(sessionMaybeDestroy, session);
25682611
if (err) {
25692612
if (session[kType] === NGHTTP2_SESSION_CLIENT) {
25702613
if (onClientStreamErrorChannel.hasSubscribers) {
@@ -2615,6 +2658,8 @@ class Http2Stream extends Duplex {
26152658
}
26162659
}
26172660

2661+
Http2Stream.prototype.off = Http2Stream.prototype.removeListener;
2662+
26182663
// TODO(aduh95): remove this in future semver-major
26192664
Http2Stream.prototype.priority = deprecate(function priority(options) {
26202665
if (this.destroyed)
@@ -2649,6 +2694,10 @@ function callStreamClose(stream) {
26492694
stream.close();
26502695
}
26512696

2697+
function sessionMaybeDestroy(session) {
2698+
session[kMaybeDestroy]();
2699+
}
2700+
26522701
function prepareResponseHeaders(stream, headersParam, options) {
26532702
let headers;
26542703
let statusCode;
@@ -2961,12 +3010,14 @@ function afterOpen(session, options, headers, streamOptions, err, fd) {
29613010

29623011
class ServerHttp2Stream extends Http2Stream {
29633012
constructor(session, handle, id, options, headers) {
2964-
super(session, options);
3013+
super(session, options, true);
29653014
handle.owner = this;
29663015
this[kInit](id, handle);
29673016
this[kProtocol] = headers[HTTP2_HEADER_SCHEME];
29683017
this[kAuthority] = getAuthority(headers);
2969-
this.once('finish', autoDrainReadable);
3018+
// 'finish' is only emitted once, so a regular listener is safe here and
3019+
// avoids allocating a once() wrapper for every stream.
3020+
this.on('finish', autoDrainReadable);
29703021
}
29713022

29723023
// True if the remote peer accepts push streams
@@ -3307,7 +3358,7 @@ ServerHttp2Stream.prototype[kProceed] = ServerHttp2Stream.prototype.respond;
33073358

33083359
class ClientHttp2Stream extends Http2Stream {
33093360
constructor(session, handle, id, options) {
3310-
super(session, options);
3361+
super(session, options, id !== undefined);
33113362
this[kState].flags |= STREAM_FLAGS_HEADERS_SENT;
33123363
if (id !== undefined)
33133364
this[kInit](id, handle);

lib/internal/http2/util.js

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -770,14 +770,16 @@ function buildNgHeaderString(arrayOrMap,
770770
let pseudoHeaders = '';
771771
let count = 0;
772772

773-
const singles = new SafeSet();
773+
let singles;
774774
const sensitiveHeaders = arrayOrMap[kSensitiveHeaders] || emptyArray;
775-
const neverIndex = sensitiveHeaders.map((v) => v.toLowerCase());
775+
const neverIndex = sensitiveHeaders.length === 0 ?
776+
emptyArray : sensitiveHeaders.map((v) => v.toLowerCase());
776777

777778
function processHeader(key, value) {
778779
key = key.toLowerCase();
780+
const isSingleValueField = kSingleValueFields.has(key);
779781
const isStrictSingleValueField = strictSingleValueFields &&
780-
kSingleValueFields.has(key);
782+
isSingleValueField;
781783
let isArray = ArrayIsArray(value);
782784
if (isArray) {
783785
switch (value.length) {
@@ -795,11 +797,15 @@ function buildNgHeaderString(arrayOrMap,
795797
value = String(value);
796798
}
797799
if (isStrictSingleValueField) {
798-
if (singles.has(key))
800+
if (singles === undefined) {
801+
singles = [key];
802+
} else if (singles.includes(key)) {
799803
throw new ERR_HTTP2_HEADER_SINGLE_VALUE(key);
800-
singles.add(key);
804+
} else {
805+
singles.push(key);
806+
}
801807
}
802-
const flags = neverIndex.includes(key) ?
808+
const flags = neverIndex.length !== 0 && neverIndex.includes(key) ?
803809
kNeverIndexFlag :
804810
kNoHeaderFlags;
805811
if (key[0] === ':') {
@@ -810,11 +816,15 @@ function buildNgHeaderString(arrayOrMap,
810816
count++;
811817
return;
812818
}
813-
if (!checkIsHttpToken(key)) {
814-
throw new ERR_INVALID_HTTP_TOKEN('Header name', key);
815-
}
816-
if (isIllegalConnectionSpecificHeader(key, value)) {
817-
throw new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key);
819+
// Well-known single-value fields are all valid HTTP tokens and none of
820+
// them is a connection-specific header, so both checks can be skipped.
821+
if (!isSingleValueField) {
822+
if (!checkIsHttpToken(key)) {
823+
throw new ERR_INVALID_HTTP_TOKEN('Header name', key);
824+
}
825+
if (isIllegalConnectionSpecificHeader(key, value)) {
826+
throw new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key);
827+
}
818828
}
819829
if (isArray) {
820830
for (let j = 0; j < value.length; ++j) {

0 commit comments

Comments
 (0)