Skip to content

Commit

Permalink
http2: multiple style and performance updates
Browse files Browse the repository at this point in the history
* 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 <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
jasnell committed Oct 23, 2017
1 parent 82b1660 commit f16b9c1
Show file tree
Hide file tree
Showing 7 changed files with 555 additions and 365 deletions.
30 changes: 8 additions & 22 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,19 +279,13 @@ function onSessionRead(nread, buf, handle) {
assert(stream !== undefined,
'Internal HTTP/2 Failure. Stream does not exist. Please ' +
'report this as a bug in Node.js');
const state = stream[kState];
_unrefActive(owner); // Reset the session timeout timer
_unrefActive(stream); // Reset the stream timeout timer
if (nread >= 0 && !stream.destroyed)
return stream.push(buf);

if (nread >= 0 && !stream.destroyed) {
if (!stream.push(buf)) {
this.streamReadStop(id);
state.reading = false;
}
} else {
// Last chunk was received. End the readable side.
stream.push(null);
}
// Last chunk was received. End the readable side.
stream.push(null);
}

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

function streamOnPause() {
const session = this[kSession];
const state = this[kState];
if (session && state.reading) {
state.reading = false;
if (session) {
assert(session[kHandle].streamReadStop(this[kID]) === undefined,
`HTTP/2 Stream ${this[kID]} does not exist. Please report this as ' +
'a bug in Node.js`);
Expand Down Expand Up @@ -1412,12 +1402,8 @@ class Http2Stream extends Duplex {
return;
}
_unrefActive(this);
const state = this[kState];
if (state.reading)
return;
state.reading = true;
assert(this[kSession][kHandle].streamReadStart(this[kID]) === undefined,
`HTTP/2 Stream ${this[kID]} does not exist. Please report this as ` +
assert(this[kSession][kHandle].flushData(this[kID]) === undefined,
'HTTP/2 Stream #{this[kID]} does not exist. Please report this as ' +
'a bug in Node.js');
}

Expand Down
Loading

0 comments on commit f16b9c1

Please sign in to comment.