Skip to content

Commit 0d50c70

Browse files
jasnellMylesBorins
authored andcommitted
http2: multiple style and performance updates
* move CHECK statements into DEBUG checks * improve performance by removing branches * Several if checks were left in while the code was being developed. Now that the core API has stablized more, the checks are largely unnecessary and can be removed, yielding a significant boost in performance. * refactor flow control for proper backpressure * use std::queue for inbound headers * use std::queue for outbound data * remove now unnecessary FreeHeaders function * expand comments and miscellaneous edits * add a couple of misbehaving flow control tests PR-URL: #16239 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent 5d34f2f commit 0d50c70

File tree

7 files changed

+555
-365
lines changed

7 files changed

+555
-365
lines changed

lib/internal/http2/core.js

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -279,19 +279,13 @@ function onSessionRead(nread, buf, handle) {
279279
assert(stream !== undefined,
280280
'Internal HTTP/2 Failure. Stream does not exist. Please ' +
281281
'report this as a bug in Node.js');
282-
const state = stream[kState];
283282
_unrefActive(owner); // Reset the session timeout timer
284283
_unrefActive(stream); // Reset the stream timeout timer
284+
if (nread >= 0 && !stream.destroyed)
285+
return stream.push(buf);
285286

286-
if (nread >= 0 && !stream.destroyed) {
287-
if (!stream.push(buf)) {
288-
this.streamReadStop(id);
289-
state.reading = false;
290-
}
291-
} else {
292-
// Last chunk was received. End the readable side.
293-
stream.push(null);
294-
}
287+
// Last chunk was received. End the readable side.
288+
stream.push(null);
295289
}
296290

297291
// Called when the remote peer settings have been updated.
@@ -1205,9 +1199,7 @@ function streamOnResume() {
12051199
return;
12061200
}
12071201
const session = this[kSession];
1208-
const state = this[kState];
1209-
if (session && !state.reading) {
1210-
state.reading = true;
1202+
if (session) {
12111203
assert(session[kHandle].streamReadStart(this[kID]) === undefined,
12121204
`HTTP/2 Stream ${this[kID]} does not exist. Please report this as ` +
12131205
'a bug in Node.js');
@@ -1216,9 +1208,7 @@ function streamOnResume() {
12161208

12171209
function streamOnPause() {
12181210
const session = this[kSession];
1219-
const state = this[kState];
1220-
if (session && state.reading) {
1221-
state.reading = false;
1211+
if (session) {
12221212
assert(session[kHandle].streamReadStop(this[kID]) === undefined,
12231213
`HTTP/2 Stream ${this[kID]} does not exist. Please report this as ' +
12241214
'a bug in Node.js`);
@@ -1412,12 +1402,8 @@ class Http2Stream extends Duplex {
14121402
return;
14131403
}
14141404
_unrefActive(this);
1415-
const state = this[kState];
1416-
if (state.reading)
1417-
return;
1418-
state.reading = true;
1419-
assert(this[kSession][kHandle].streamReadStart(this[kID]) === undefined,
1420-
`HTTP/2 Stream ${this[kID]} does not exist. Please report this as ` +
1405+
assert(this[kSession][kHandle].flushData(this[kID]) === undefined,
1406+
'HTTP/2 Stream #{this[kID]} does not exist. Please report this as ' +
14211407
'a bug in Node.js');
14221408
}
14231409

0 commit comments

Comments
 (0)