Skip to content

Commit

Permalink
http2: improve perf of passing headers to C++
Browse files Browse the repository at this point in the history 
By passing a single string rather than many small ones and
a single block allocation for passing headers, save expensive
interactions with JS values and memory allocations.

PR-URL: #14723
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
@addaleax
addaleax committed Aug 14, 2017
1 parent 66788fc commit 92c37fe
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 81 deletions.
47 changes: 47 additions & 0 deletions benchmark/http2/headers.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict';

const common = require('../common.js');
const PORT = common.PORT;

var bench = common.createBenchmark(main, {
n: [1e3],
nheaders: [100, 1000],
}, { flags: ['--expose-http2', '--no-warnings'] });

function main(conf) {
const n = +conf.n;
const nheaders = +conf.nheaders;
const http2 = require('http2');
const server = http2.createServer();

const headersObject = { ':path': '/' };
for (var i = 0; i < nheaders; i++) {
headersObject[`foo${i}`] = `some header value ${i}`;
}

server.on('stream', (stream) => {
stream.respond();
stream.end('Hi!');
});
server.listen(PORT, () => {
const client = http2.connect(`http://localhost:${PORT}/`);

function doRequest(remaining) {
const req = client.request(headersObject);
req.end();
req.on('data', () => {});
req.on('end', () => {
if (remaining > 0) {
doRequest(remaining - 1);
} else {
bench.end(n);
server.close();
client.destroy();
}
});
}

bench.start();
doRequest(n);
});
}
14 changes: 9 additions & 5 deletions lib/internal/http2/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,8 @@ function assertValidPseudoHeaderTrailer(key) {

function mapToHeaders(map,
assertValuePseudoHeader = assertValidPseudoHeader) {
const ret = [];
let ret = '';
let count = 0;
const keys = Object.keys(map);
const singles = new Set();
for (var i = 0; i < keys.length; i++) {
Expand All @@ -402,7 +403,8 @@ function mapToHeaders(map,
const err = assertValuePseudoHeader(key);
if (err !== undefined)
return err;
ret.unshift([key, String(value)]);
ret = `${key}\0${String(value)}\0${ret}`;
count++;
} else {
if (kSingleValueHeaders.has(key)) {
if (singles.has(key))
Expand All @@ -415,16 +417,18 @@ function mapToHeaders(map,
if (isArray) {
for (var k = 0; k < value.length; k++) {
val = String(value[k]);
ret.push([key, val]);
ret += `${key}\0${val}\0`;
}
count += value.length;
} else {
val = String(value);
ret.push([key, val]);
ret += `${key}\0${val}\0`;
count++;
}
}
}

return ret;
return [ret, count];
}

class NghttpError extends Error {
Expand Down
65 changes: 65 additions & 0 deletions src/node_http2.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ using v8::Boolean;
using v8::Context;
using v8::Function;
using v8::Integer;
using v8::String;
using v8::Uint32;
using v8::Undefined;

namespace http2 {
Expand Down Expand Up @@ -1075,6 +1077,69 @@ void Http2Session::Unconsume() {
}


Headers::Headers(Isolate* isolate,
Local<Context> context,
Local<Array> headers) {
CHECK_EQ(headers->Length(), 2);
Local<Value> header_string = headers->Get(context, 0).ToLocalChecked();
Local<Value> header_count = headers->Get(context, 1).ToLocalChecked();
CHECK(header_string->IsString());
CHECK(header_count->IsUint32());
count_ = header_count.As<Uint32>()->Value();
int header_string_len = header_string.As<String>()->Length();

if (count_ == 0) {
CHECK_EQ(header_string_len, 0);
return;
}

// Allocate a single buffer with count_ nghttp2_nv structs, followed
// by the raw header data as passed from JS. This looks like:
// | possible padding | nghttp2_nv | nghttp2_nv | ... | header contents |
buf_.AllocateSufficientStorage((alignof(nghttp2_nv) - 1) +
count_ * sizeof(nghttp2_nv) +
header_string_len);
// Make sure the start address is aligned appropriately for an nghttp2_nv*.
char* start = reinterpret_cast<char*>(
ROUND_UP(reinterpret_cast<uintptr_t>(*buf_), alignof(nghttp2_nv)));
char* header_contents = start + (count_ * sizeof(nghttp2_nv));
nghttp2_nv* const nva = reinterpret_cast<nghttp2_nv*>(start);

CHECK_LE(header_contents + header_string_len, *buf_ + buf_.length());
CHECK_EQ(header_string.As<String>()
->WriteOneByte(reinterpret_cast<uint8_t*>(header_contents),
0, header_string_len,
String::NO_NULL_TERMINATION),
header_string_len);

size_t n = 0;
char* p;
for (p = header_contents; p < header_contents + header_string_len; n++) {
if (n >= count_) {
// This can happen if a passed header contained a null byte. In that
// case, just provide nghttp2 with an invalid header to make it reject
// the headers list.
static uint8_t zero = '\0';
nva[0].name = nva[0].value = &zero;
nva[0].namelen = nva[0].valuelen = 1;
count_ = 1;
return;
}

nva[n].flags = NGHTTP2_NV_FLAG_NONE;
nva[n].name = reinterpret_cast<uint8_t*>(p);
nva[n].namelen = strlen(p);
p += nva[n].namelen + 1;
nva[n].value = reinterpret_cast<uint8_t*>(p);
nva[n].valuelen = strlen(p);
p += nva[n].valuelen + 1;
}

CHECK_EQ(p, header_contents + header_string_len);
CHECK_EQ(n, count_);
}


void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
Expand Down
46 changes: 6 additions & 40 deletions src/node_http2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,54 +515,20 @@ class ExternalHeader :

class Headers {
public:
Headers(Isolate* isolate, Local<Context> context, Local<Array> headers) {
headers_.AllocateSufficientStorage(headers->Length());
Local<Value> item;
Local<Array> header;

for (size_t n = 0; n < headers->Length(); n++) {
item = headers->Get(context, n).ToLocalChecked();
CHECK(item->IsArray());
header = item.As<Array>();
Local<Value> key = header->Get(context, 0).ToLocalChecked();
Local<Value> value = header->Get(context, 1).ToLocalChecked();
CHECK(key->IsString());
CHECK(value->IsString());
size_t keylen = StringBytes::StorageSize(isolate, key, ASCII);
size_t valuelen = StringBytes::StorageSize(isolate, value, ASCII);
headers_[n].flags = NGHTTP2_NV_FLAG_NONE;
Local<Value> flag = header->Get(context, 2).ToLocalChecked();
if (flag->BooleanValue(context).ToChecked())
headers_[n].flags |= NGHTTP2_NV_FLAG_NO_INDEX;
uint8_t* buf = Malloc<uint8_t>(keylen + valuelen);
headers_[n].name = buf;
headers_[n].value = buf + keylen;
headers_[n].namelen =
StringBytes::Write(isolate,
reinterpret_cast<char*>(headers_[n].name),
keylen, key, ASCII);
headers_[n].valuelen =
StringBytes::Write(isolate,
reinterpret_cast<char*>(headers_[n].value),
valuelen, value, ASCII);
}
}

~Headers() {
for (size_t n = 0; n < headers_.length(); n++)
free(headers_[n].name);
}
Headers(Isolate* isolate, Local<Context> context, Local<Array> headers);
~Headers() {}

nghttp2_nv* operator*() {
return *headers_;
return reinterpret_cast<nghttp2_nv*>(*buf_);
}

size_t length() const {
return headers_.length();
return count_;
}

private:
MaybeStackBuffer<nghttp2_nv> headers_;
size_t count_;
MaybeStackBuffer<char, 3000> buf_;
};

} // namespace http2
Expand Down
56 changes: 20 additions & 36 deletions test/parallel/test-http2-util-headers-list.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,11 @@ const {
'BAR': [1]
};

assert.deepStrictEqual(mapToHeaders(headers), [
[ ':path', 'abc' ],
[ ':status', '200' ],
[ 'abc', '1' ],
[ 'xyz', '1' ],
[ 'xyz', '2' ],
[ 'xyz', '3' ],
[ 'xyz', '4' ],
[ 'bar', '1' ]
]);
assert.deepStrictEqual(
mapToHeaders(headers),
[ [ ':path', 'abc', ':status', '200', 'abc', '1', 'xyz', '1', 'xyz', '2',
'xyz', '3', 'xyz', '4', 'bar', '1', '' ].join('\0'), 8 ]
);
}

{
Expand All @@ -103,15 +98,11 @@ const {
'xyz': [1, 2, 3, 4]
};

assert.deepStrictEqual(mapToHeaders(headers), [
[ ':status', '200' ],
[ ':path', 'abc' ],
[ 'abc', '1' ],
[ 'xyz', '1' ],
[ 'xyz', '2' ],
[ 'xyz', '3' ],
[ 'xyz', '4' ]
]);
assert.deepStrictEqual(
mapToHeaders(headers),
[ [ ':status', '200', ':path', 'abc', 'abc', '1', 'xyz', '1', 'xyz', '2',
'xyz', '3', 'xyz', '4', '' ].join('\0'), 7 ]
);
}

{
Expand All @@ -124,15 +115,11 @@ const {
[Symbol('test')]: 1 // Symbol keys are ignored
};

assert.deepStrictEqual(mapToHeaders(headers), [
[ ':status', '200' ],
[ ':path', 'abc' ],
[ 'abc', '1' ],
[ 'xyz', '1' ],
[ 'xyz', '2' ],
[ 'xyz', '3' ],
[ 'xyz', '4' ]
]);
assert.deepStrictEqual(
mapToHeaders(headers),
[ [ ':status', '200', ':path', 'abc', 'abc', '1', 'xyz', '1', 'xyz', '2',
'xyz', '3', 'xyz', '4', '' ].join('\0'), 7 ]
);
}

{
Expand All @@ -144,14 +131,11 @@ const {
headers.foo = [];
headers[':status'] = 200;

assert.deepStrictEqual(mapToHeaders(headers), [
[ ':status', '200' ],
[ ':path', 'abc' ],
[ 'xyz', '1' ],
[ 'xyz', '2' ],
[ 'xyz', '3' ],
[ 'xyz', '4' ]
]);
assert.deepStrictEqual(
mapToHeaders(headers),
[ [ ':status', '200', ':path', 'abc', 'xyz', '1', 'xyz', '2', 'xyz', '3',
'xyz', '4', '' ].join('\0'), 6 ]
);
}

// The following are not allowed to have multiple values
Expand Down

0 comments on commit 92c37fe

Please sign in to comment.