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: #25567
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

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(); });
+  });
+}

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');
 

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;

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,

src/stream_wrap.cc

@@ -64,11 +64,29 @@ void LibuvStreamWrap::Initialize(Local<Object> target,
   };
   Local<FunctionTemplate> sw =
       FunctionTemplate::New(env->isolate(), is_construct_call_callback);
-  sw->InstanceTemplate()->SetInternalFieldCount(StreamReq::kStreamReqField + 1);
+  sw->InstanceTemplate()->SetInternalFieldCount(
+      StreamReq::kStreamReqField + 1 + 3);
   Local<String> 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();