Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: incremental improvements #15693

Closed
wants to merge 4 commits into from
Closed

http2: incremental improvements #15693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
146fae6
http2: simplify TypeName
jasnell Sep 29, 2017
5832766
http2: refactor method arguments to avoid bools
jasnell Sep 29, 2017
9540585
http2: eliminate dead code
jasnell Sep 29, 2017
0b943c6
http2: making sending to the socket more efficient
jasnell Sep 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions benchmark/http2/write.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,27 @@ const PORT = common.PORT;
const bench = common.createBenchmark(main, {
streams: [100, 200, 1000],
length: [64 * 1024, 128 * 1024, 256 * 1024, 1024 * 1024],
size: [100000]
}, { flags: ['--no-warnings'] });

function main(conf) {
const m = +conf.streams;
const l = +conf.length;
const s = +conf.size;
const http2 = require('http2');
const server = http2.createServer();
server.on('stream', (stream) => {
stream.respond();
stream.write('ü'.repeat(l));
stream.end();
let written = 0;
function write() {
stream.write('ü'.repeat(s));
written += s;
if (written < l)
setImmediate(write);
else
stream.end();
}
write();
});
server.listen(PORT, () => {
bench.http({
Expand Down
67 changes: 37 additions & 30 deletions lib/internal/http2/core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ const {
HTTP_STATUS_OK,
HTTP_STATUS_NO_CONTENT,
HTTP_STATUS_NOT_MODIFIED,
HTTP_STATUS_SWITCHING_PROTOCOLS
HTTP_STATUS_SWITCHING_PROTOCOLS,

STREAM_OPTION_EMPTY_PAYLOAD,
STREAM_OPTION_GET_TRAILERS
} = constants;

// Top level to avoid creating a closure
Expand Down Expand Up @@ -412,20 +415,22 @@ function requestOnConnect(headers, options) {
return;
}

let getTrailers = false;
let streamOptions = 0;
if (options.endStream)
streamOptions |= STREAM_OPTION_EMPTY_PAYLOAD;

if (typeof options.getTrailers === 'function') {
getTrailers = true;
streamOptions |= STREAM_OPTION_GET_TRAILERS;
this[kState].getTrailers = options.getTrailers;
}

// ret will be either the reserved stream ID (if positive)
// or an error code (if negative)
const ret = handle.submitRequest(headersList,
!!options.endStream,
streamOptions,
options.parent | 0,
options.weight | 0,
!!options.exclusive,
getTrailers);
!!options.exclusive);

// In an error condition, one of three possible response codes will be
// possible:
Expand Down Expand Up @@ -1563,7 +1568,7 @@ function processHeaders(headers) {
}

function processRespondWithFD(fd, headers, offset = 0, length = -1,
getTrailers = false) {
streamOptions = 0) {
const session = this[kSession];
const state = this[kState];
state.headersSent = true;
Expand All @@ -1573,7 +1578,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1,

const handle = session[kHandle];
const ret =
handle.submitFile(this[kID], fd, headers, offset, length, getTrailers);
handle.submitFile(this[kID], fd, headers, offset, length, streamOptions);
let err;
switch (ret) {
case NGHTTP2_ERR_NOMEM:
Expand All @@ -1588,7 +1593,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1,
}
}

function doSendFD(session, options, fd, headers, getTrailers, err, stat) {
function doSendFD(session, options, fd, headers, streamOptions, err, stat) {
if (this.destroyed || session.destroyed) {
abort(this);
return;
Expand Down Expand Up @@ -1617,10 +1622,10 @@ function doSendFD(session, options, fd, headers, getTrailers, err, stat) {
processRespondWithFD.call(this, fd, headersList,
statOptions.offset,
statOptions.length,
getTrailers);
streamOptions);
}

function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
function doSendFileFD(session, options, fd, headers, streamOptions, err, stat) {
if (this.destroyed || session.destroyed) {
abort(this);
return;
Expand Down Expand Up @@ -1674,10 +1679,10 @@ function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
processRespondWithFD.call(this, fd, headersList,
options.offset,
options.length,
getTrailers);
streamOptions);
}

function afterOpen(session, options, headers, getTrailers, err, fd) {
function afterOpen(session, options, headers, streamOptions, err, fd) {
const state = this[kState];
const onError = options.onError;
if (this.destroyed || session.destroyed) {
Expand All @@ -1695,7 +1700,8 @@ function afterOpen(session, options, headers, getTrailers, err, fd) {
state.fd = fd;

fs.fstat(fd,
doSendFileFD.bind(this, session, options, fd, headers, getTrailers));
doSendFileFD.bind(this, session, options, fd,
headers, streamOptions));
}

function streamOnError(err) {
Expand Down Expand Up @@ -1779,9 +1785,9 @@ class ServerHttp2Stream extends Http2Stream {
throw headersList;
}

const ret = handle.submitPushPromise(this[kID],
headersList,
options.endStream);
const streamOptions = options.endStream ? STREAM_OPTION_EMPTY_PAYLOAD : 0;

const ret = handle.submitPushPromise(this[kID], headersList, streamOptions);
let err;
switch (ret) {
case NGHTTP2_ERR_NOMEM:
Expand Down Expand Up @@ -1837,14 +1843,17 @@ class ServerHttp2Stream extends Http2Stream {
options = Object.assign(Object.create(null), options);
options.endStream = !!options.endStream;

let getTrailers = false;
let streamOptions = 0;
if (options.endStream)
streamOptions |= STREAM_OPTION_EMPTY_PAYLOAD;

if (options.getTrailers !== undefined) {
if (typeof options.getTrailers !== 'function') {
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
'getTrailers',
options.getTrailers);
}
getTrailers = true;
streamOptions |= STREAM_OPTION_GET_TRAILERS;
state.getTrailers = options.getTrailers;
}

Expand Down Expand Up @@ -1874,10 +1883,7 @@ class ServerHttp2Stream extends Http2Stream {

const handle = session[kHandle];
const ret =
handle.submitResponse(this[kID],
headersList,
options.endStream,
getTrailers);
handle.submitResponse(this[kID], headersList, streamOptions);
let err;
switch (ret) {
case NGHTTP2_ERR_NOMEM:
Expand Down Expand Up @@ -1930,14 +1936,14 @@ class ServerHttp2Stream extends Http2Stream {
options.statCheck);
}

let getTrailers = false;
let streamOptions = 0;
if (options.getTrailers !== undefined) {
if (typeof options.getTrailers !== 'function') {
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
'getTrailers',
options.getTrailers);
}
getTrailers = true;
streamOptions |= STREAM_OPTION_GET_TRAILERS;
state.getTrailers = options.getTrailers;
}

Expand All @@ -1956,7 +1962,8 @@ class ServerHttp2Stream extends Http2Stream {

if (options.statCheck !== undefined) {
fs.fstat(fd,
doSendFD.bind(this, session, options, fd, headers, getTrailers));
doSendFD.bind(this, session, options, fd,
headers, streamOptions));
return;
}

Expand All @@ -1969,7 +1976,7 @@ class ServerHttp2Stream extends Http2Stream {
processRespondWithFD.call(this, fd, headersList,
options.offset,
options.length,
getTrailers);
streamOptions);
}

// Initiate a file response on this Http2Stream. The path is passed to
Expand Down Expand Up @@ -2011,14 +2018,14 @@ class ServerHttp2Stream extends Http2Stream {
options.statCheck);
}

let getTrailers = false;
let streamOptions = 0;
if (options.getTrailers !== undefined) {
if (typeof options.getTrailers !== 'function') {
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
'getTrailers',
options.getTrailers);
}
getTrailers = true;
streamOptions |= STREAM_OPTION_GET_TRAILERS;
state.getTrailers = options.getTrailers;
}

Expand All @@ -2032,7 +2039,7 @@ class ServerHttp2Stream extends Http2Stream {
}

fs.open(path, 'r',
afterOpen.bind(this, session, options, headers, getTrailers));
afterOpen.bind(this, session, options, headers, streamOptions));
}

// Sends a block of informational headers. In theory, the HTTP/2 spec
Expand Down
35 changes: 16 additions & 19 deletions src/node_http2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ Freelist<Nghttp2Stream, FREELIST_MAX> stream_free_list;

Freelist<nghttp2_header_list, FREELIST_MAX> header_free_list;

Freelist<nghttp2_data_chunks_t, FREELIST_MAX>
data_chunks_free_list;

Nghttp2Session::Callbacks Nghttp2Session::callback_struct_saved[2] = {
Callbacks(false),
Callbacks(true)};
Expand Down Expand Up @@ -479,7 +476,7 @@ void Http2Session::SubmitRstStream(const FunctionCallbackInfo<Value>& args) {

void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
// args[0] Array of headers
// args[1] endStream boolean
// args[1] options int
// args[2] parentStream ID (for priority spec)
// args[3] weight (for priority spec)
// args[4] exclusive boolean (for priority spec)
Expand All @@ -492,15 +489,14 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = env->isolate();

Local<Array> headers = args[0].As<Array>();
bool endStream = args[1]->BooleanValue(context).ToChecked();
int options = args[1]->IntegerValue(context).ToChecked();
int32_t parent = args[2]->Int32Value(context).ToChecked();
int32_t weight = args[3]->Int32Value(context).ToChecked();
bool exclusive = args[4]->BooleanValue(context).ToChecked();
bool getTrailers = args[5]->BooleanValue(context).ToChecked();

DEBUG_HTTP2("Http2Session: submitting request: headers: %d, end-stream: %d, "
DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d, "
"parent: %d, weight: %d, exclusive: %d\n", headers->Length(),
endStream, parent, weight, exclusive);
options, parent, weight, exclusive);

nghttp2_priority_spec prispec;
nghttp2_priority_spec_init(&prispec, parent, weight, exclusive ? 1 : 0);
Expand All @@ -509,8 +505,7 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {

int32_t ret = session->Nghttp2Session::SubmitRequest(&prispec,
*list, list.length(),
nullptr, endStream,
getTrailers);
nullptr, options);
DEBUG_HTTP2("Http2Session: request submitted, response: %d\n", ret);
args.GetReturnValue().Set(ret);
}
Expand All @@ -529,11 +524,10 @@ void Http2Session::SubmitResponse(const FunctionCallbackInfo<Value>& args) {

int32_t id = args[0]->Int32Value(context).ToChecked();
Local<Array> headers = args[1].As<Array>();
bool endStream = args[2]->BooleanValue(context).ToChecked();
bool getTrailers = args[3]->BooleanValue(context).ToChecked();
int options = args[2]->IntegerValue(context).ToChecked();

DEBUG_HTTP2("Http2Session: submitting response for stream %d: headers: %d, "
"end-stream: %d\n", id, headers->Length(), endStream);
"options: %d\n", id, headers->Length(), options);

if (!(stream = session->FindStream(id))) {
return args.GetReturnValue().Set(NGHTTP2_ERR_INVALID_STREAM_ID);
Expand All @@ -542,7 +536,7 @@ void Http2Session::SubmitResponse(const FunctionCallbackInfo<Value>& args) {
Headers list(isolate, context, headers);

args.GetReturnValue().Set(
stream->SubmitResponse(*list, list.length(), endStream, getTrailers));
stream->SubmitResponse(*list, list.length(), options));
}

void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {
Expand All @@ -566,7 +560,7 @@ void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {

int64_t offset = args[3]->IntegerValue(context).ToChecked();
int64_t length = args[4]->IntegerValue(context).ToChecked();
bool getTrailers = args[5]->BooleanValue(context).ToChecked();
int options = args[5]->IntegerValue(context).ToChecked();

CHECK_GE(offset, 0);

Expand All @@ -580,7 +574,7 @@ void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {
Headers list(isolate, context, headers);

args.GetReturnValue().Set(stream->SubmitFile(fd, *list, list.length(),
offset, length, getTrailers));
offset, length, options));
}

void Http2Session::SendHeaders(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -719,10 +713,10 @@ void Http2Session::SubmitPushPromise(const FunctionCallbackInfo<Value>& args) {
Nghttp2Stream* parent;
int32_t id = args[0]->Int32Value(context).ToChecked();
Local<Array> headers = args[1].As<Array>();
bool endStream = args[2]->BooleanValue(context).ToChecked();
int options = args[2]->IntegerValue(context).ToChecked();

DEBUG_HTTP2("Http2Session: submitting push promise for stream %d: "
"end-stream: %d, headers: %d\n", id, endStream,
"options: %d, headers: %d\n", id, options,
headers->Length());

if (!(parent = session->FindStream(id))) {
Expand All @@ -732,7 +726,7 @@ void Http2Session::SubmitPushPromise(const FunctionCallbackInfo<Value>& args) {
Headers list(isolate, context, headers);

int32_t ret = parent->SubmitPushPromise(*list, list.length(),
nullptr, endStream);
nullptr, options);
DEBUG_HTTP2("Http2Session: push promise submitted, ret: %d\n", ret);
args.GetReturnValue().Set(ret);
}
Expand Down Expand Up @@ -1250,6 +1244,9 @@ void Initialize(Local<Object> target,
NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_ERR_STREAM_CLOSED);
NODE_DEFINE_CONSTANT(constants, NGHTTP2_ERR_FRAME_SIZE_ERROR);

NODE_DEFINE_HIDDEN_CONSTANT(constants, STREAM_OPTION_EMPTY_PAYLOAD);
NODE_DEFINE_HIDDEN_CONSTANT(constants, STREAM_OPTION_GET_TRAILERS);

NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_NONE);
NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_END_STREAM);
NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_END_HEADERS);
Expand Down
Loading