From 9af04ad684a666888f76024876d7b74eee60d2f7 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 18 Jan 2019 15:31:46 +0100 Subject: http2: improve compat performance This bunch of commits help me improve the performance of a http2 server by 8-10%. The benchmarks reports several 1-2% improvements in various areas. PR-URL: https://github.com/nodejs/node/pull/25567 Reviewed-By: Benedikt Meurer Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- benchmark/http2/compat.js | 35 +++++++++++++++++++++++++++++++++++ lib/internal/http2/compat.js | 4 +--- lib/internal/http2/core.js | 26 ++++++++++++++++++++++---- lib/internal/http2/util.js | 8 ++++++-- src/stream_wrap.cc | 20 +++++++++++++++++++- 5 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 benchmark/http2/compat.js diff --git a/benchmark/http2/compat.js b/benchmark/http2/compat.js new file mode 100644 index 0000000000..5d06ccf317 --- /dev/null +++ b/benchmark/http2/compat.js @@ -0,0 +1,35 @@ +'use strict'; + +const common = require('../common.js'); +const path = require('path'); +const fs = require('fs'); +const file = path.join(path.resolve(__dirname, '../fixtures'), 'alice.html'); + +const bench = common.createBenchmark(main, { + requests: [100, 1000, 5000], + streams: [1, 10, 20, 40, 100, 200], + clients: [2], + benchmarker: ['h2load'] +}, { flags: ['--no-warnings'] }); + +function main({ requests, streams, clients }) { + const http2 = require('http2'); + const server = http2.createServer(); + server.on('request', (req, res) => { + const out = fs.createReadStream(file); + res.setHeader('content-type', 'text/html'); + out.pipe(res); + out.on('error', (err) => { + res.destroy(); + }); + }); + server.listen(common.PORT, () => { + bench.http({ + path: '/', + requests, + maxConcurrentStreams: streams, + clients, + threads: clients + }, () => { server.close(); }); + }); +} diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 8043bd492a..da88f4d880 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -18,18 +18,16 @@ const { ERR_INVALID_HTTP_TOKEN } = require('internal/errors').codes; const { validateString } = require('internal/validators'); -const { kSocket } = require('internal/http2/util'); +const { kSocket, kRequest, kProxySocket } = require('internal/http2/util'); const kBeginSend = Symbol('begin-send'); const kState = Symbol('state'); const kStream = Symbol('stream'); -const kRequest = Symbol('request'); const kResponse = Symbol('response'); const kHeaders = Symbol('headers'); const kRawHeaders = Symbol('rawHeaders'); const kTrailers = Symbol('trailers'); const kRawTrailers = Symbol('rawTrailers'); -const kProxySocket = Symbol('proxySocket'); const kSetHeader = Symbol('setHeader'); const kAborted = Symbol('aborted'); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 8e84eadbff..0a566ed9e6 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -96,6 +96,8 @@ const { getStreamState, isPayloadMeaningless, kSocket, + kRequest, + kProxySocket, mapToHeaders, NghttpError, sessionName, @@ -117,6 +119,8 @@ const { const { kTimeout } = require('internal/timers'); const { isArrayBufferView } = require('internal/util/types'); +const hasOwnProperty = Object.prototype.hasOwnProperty; + const { FileHandle } = internalBinding('fs'); const binding = internalBinding('http2'); const { @@ -155,7 +159,6 @@ const kOwner = owner_symbol; const kOrigin = Symbol('origin'); const kProceed = Symbol('proceed'); const kProtocol = Symbol('protocol'); -const kProxySocket = Symbol('proxy-socket'); const kRemoteSettings = Symbol('remote-settings'); const kSelectPadding = Symbol('select-padding'); const kSentHeaders = Symbol('sent-headers'); @@ -1622,6 +1625,10 @@ class Http2Stream extends Duplex { endAfterHeaders: false }; + // Fields used by the compat API to avoid megamorphisms. + this[kRequest] = null; + this[kProxySocket] = null; + this.on('pause', streamOnPause); } @@ -2001,9 +2008,20 @@ class Http2Stream extends Duplex { } } -function processHeaders(headers) { - assertIsObject(headers, 'headers'); - headers = Object.assign(Object.create(null), headers); +function processHeaders(oldHeaders) { + assertIsObject(oldHeaders, 'headers'); + const headers = Object.create(null); + + if (oldHeaders !== null && oldHeaders !== undefined) { + const hop = hasOwnProperty.bind(oldHeaders); + // This loop is here for performance reason. Do not change. + for (var key in oldHeaders) { + if (hop(key)) { + headers[key] = oldHeaders[key]; + } + } + } + const statusCode = headers[HTTP2_HEADER_STATUS] = headers[HTTP2_HEADER_STATUS] | 0 || HTTP_STATUS_OK; diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index f62d936025..0a3faa2355 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -10,6 +10,8 @@ const { } = require('internal/errors').codes; const kSocket = Symbol('socket'); +const kProxySocket = Symbol('proxySocket'); +const kRequest = Symbol('request'); const { NGHTTP2_SESSION_CLIENT, @@ -499,12 +501,12 @@ class NghttpError extends Error { } } -function assertIsObject(value, name, types = 'Object') { +function assertIsObject(value, name, types) { if (value !== undefined && (value === null || typeof value !== 'object' || Array.isArray(value))) { - const err = new ERR_INVALID_ARG_TYPE(name, types, value); + const err = new ERR_INVALID_ARG_TYPE(name, types || 'Object', value); Error.captureStackTrace(err, assertIsObject); throw err; } @@ -592,6 +594,8 @@ module.exports = { getStreamState, isPayloadMeaningless, kSocket, + kProxySocket, + kRequest, mapToHeaders, NghttpError, sessionName, diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index e957cb1725..d126f90eef 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -64,11 +64,29 @@ void LibuvStreamWrap::Initialize(Local target, }; Local sw = FunctionTemplate::New(env->isolate(), is_construct_call_callback); - sw->InstanceTemplate()->SetInternalFieldCount(StreamReq::kStreamReqField + 1); + sw->InstanceTemplate()->SetInternalFieldCount( + StreamReq::kStreamReqField + 1 + 3); Local wrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "ShutdownWrap"); sw->SetClassName(wrapString); + + // we need to set handle and callback to null, + // so that those fields are created and functions + // do not become megamorphic + // Fields: + // - oncomplete + // - callback + // - handle + sw->InstanceTemplate()->Set( + FIXED_ONE_BYTE_STRING(env->isolate(), "oncomplete"), + v8::Null(env->isolate())); + sw->InstanceTemplate()->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "callback"), + v8::Null(env->isolate())); + sw->InstanceTemplate()->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "handle"), + v8::Null(env->isolate())); + sw->Inherit(AsyncWrap::GetConstructorTemplate(env)); + target->Set(env->context(), wrapString, sw->GetFunction(env->context()).ToLocalChecked()).FromJust(); -- cgit v1.2.3