http: improve http parser performance

Co-Authored-by: Jonas Badalic <[email protected]>

Author: anonrig

benchmark/http/bench-parser.js

@@ -23,14 +23,14 @@ function main({ len, n }) {
     bench.start();
     for (let i = 0; i < n; i++) {
       parser.execute(header, 0, header.length);
-      parser.initialize(REQUEST, {});
+      parser.initialize(REQUEST);
     }
     bench.end(n);
   }
 
   function newParser(type) {
     const parser = new HTTPParser();
-    parser.initialize(type, {});
+    parser.initialize(type);
 
     parser.headers = [];
 

lib/_http_client.js

@@ -130,13 +130,6 @@ function validateHost(host, name) {
   return host;
 }
 
-class HTTPClientAsyncResource {
-  constructor(type, req) {
-    this.type = type;
-    this.req = req;
-  }
-}
-
 // When proxying a HTTP request, the following needs to be done:
 // https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2
 // 1. Rewrite the request path to absolute-form.
@@ -880,7 +873,6 @@ function tickOnSocket(req, socket) {
   const lenient = req.insecureHTTPParser === undefined ?
     isLenient() : req.insecureHTTPParser;
   parser.initialize(HTTPParser.RESPONSE,
-                    new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
                     req.maxHeaderSize || 0,
                     lenient ? kLenientAll : kLenientNone);
   parser.socket = socket;

lib/_http_common.js

@@ -187,10 +187,6 @@ function freeParser(parser, req, socket) {
       // Make sure the parser's stack has unwound before deleting the
       // corresponding C++ object through .close().
       setImmediate(closeParserInstance, parser);
-    } else {
-      // Since the Parser destructor isn't going to run the destroy() callbacks
-      // it needs to be triggered manually.
-      parser.free();
     }
   }
   if (req) {

lib/_http_server.js

@@ -187,13 +187,6 @@ const kConnectionsCheckingInterval = Symbol('http.server.connectionsCheckingInte
 
 const HTTP_SERVER_TRACE_EVENT_NAME = 'http.server.request';
 
-class HTTPServerAsyncResource {
-  constructor(type, socket) {
-    this.type = type;
-    this.socket = socket;
-  }
-}
-
 function ServerResponse(req, options) {
   OutgoingMessage.call(this, options);
 
@@ -705,7 +698,6 @@ function connectionListenerInternal(server, socket) {
   // https://github.com/nodejs/node/pull/21313
   parser.initialize(
     HTTPParser.REQUEST,
-    new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket),
     server.maxHeaderSize || 0,
     lenient ? kLenientAll : kLenientNone,
     server[kConnections],

src/async_wrap.h

@@ -49,8 +49,6 @@ namespace node {
   V(HTTP2STREAM)                                                               \
   V(HTTP2PING)                                                                 \
   V(HTTP2SETTINGS)                                                             \
-  V(HTTPINCOMINGMESSAGE)                                                       \
-  V(HTTPCLIENTREQUEST)                                                         \
   V(LOCKS)                                                                     \
   V(JSSTREAM)                                                                  \
   V(JSUDPWRAP)                                                                 \

src/node_http_parser.cc

@@ -23,7 +23,6 @@
 #include "node_buffer.h"
 #include "util.h"
 
-#include "async_wrap-inl.h"
 #include "env-inl.h"
 #include "llhttp.h"
 #include "memory_tracker-inl.h"
@@ -250,17 +249,16 @@ class ConnectionsList : public BaseObject {
     std::set<Parser*, ParserComparator> active_connections_;
 };
 
-class Parser : public AsyncWrap, public StreamListener {
+class Parser : public BaseObject, public StreamListener {
   friend class ConnectionsList;
   friend struct ParserComparator;
 
  public:
   Parser(BindingData* binding_data, Local<Object> wrap)
-      : AsyncWrap(binding_data->env(), wrap),
+      : BaseObject(binding_data->env(), wrap),
         current_buffer_len_(0),
         current_buffer_data_(nullptr),
-        binding_data_(binding_data) {
-  }
+        binding_data_(binding_data) {}
 
   SET_NO_MEMORY_INFO()
   SET_MEMORY_INFO_NAME(Parser)
@@ -289,13 +287,7 @@ class Parser : public AsyncWrap, public StreamListener {
     Local<Value> cb = object()->Get(env()->context(), kOnMessageBegin)
                               .ToLocalChecked();
     if (cb->IsFunction()) {
-      InternalCallbackScope callback_scope(
-        this, InternalCallbackScope::kSkipTaskQueues);
-
-      MaybeLocal<Value> r = cb.As<Function>()->Call(
-        env()->context(), object(), 0, nullptr);
-
-      if (r.IsEmpty()) callback_scope.MarkAsFailed();
+      USE(cb.As<Function>()->Call(env()->context(), object(), 0, nullptr));
     }
 
     return 0;
@@ -442,14 +434,8 @@ class Parser : public AsyncWrap, public StreamListener {
 
     argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade);
 
-    MaybeLocal<Value> head_response;
-    {
-      InternalCallbackScope callback_scope(
-          this, InternalCallbackScope::kSkipTaskQueues);
-      head_response = cb.As<Function>()->Call(
-          env()->context(), object(), arraysize(argv), argv);
-      if (head_response.IsEmpty()) callback_scope.MarkAsFailed();
-    }
+    auto head_response = cb.As<Function>()->Call(
+        env()->context(), object(), arraysize(argv), argv);
 
     int64_t val;
 
@@ -478,9 +464,10 @@ class Parser : public AsyncWrap, public StreamListener {
 
     Local<Value> buffer = Buffer::Copy(env, at, length).ToLocalChecked();
 
-    MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 1, &buffer);
+    v8::TryCatch try_catch(env->isolate());
+    USE(cb.As<Function>()->Call(env->context(), object(), 1, &buffer));
 
-    if (r.IsEmpty()) {
+    if (try_catch.HasCaught()) {
       got_exception_ = true;
       llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
       return HPE_USER;
@@ -516,15 +503,10 @@ class Parser : public AsyncWrap, public StreamListener {
     if (!cb->IsFunction())
       return 0;
 
-    MaybeLocal<Value> r;
-    {
-      InternalCallbackScope callback_scope(
-          this, InternalCallbackScope::kSkipTaskQueues);
-      r = cb.As<Function>()->Call(env()->context(), object(), 0, nullptr);
-      if (r.IsEmpty()) callback_scope.MarkAsFailed();
-    }
+    v8::TryCatch try_catch(env()->isolate());
+    USE(cb.As<Function>()->Call(env()->context(), object(), 0, nullptr));
 
-    if (r.IsEmpty()) {
+    if (try_catch.HasCaught()) {
       got_exception_ = true;
       return -1;
     }
@@ -571,17 +553,6 @@ class Parser : public AsyncWrap, public StreamListener {
     delete parser;
   }
 
-  // TODO(@anonrig): Add V8 Fast API
-  static void Free(const FunctionCallbackInfo<Value>& args) {
-    Parser* parser;
-    ASSIGN_OR_RETURN_UNWRAP(&parser, args.This());
-
-    // Since the Parser destructor isn't going to run the destroy() callbacks
-    // it needs to be triggered manually.
-    parser->EmitTraceEventDestroy();
-    parser->EmitDestroy();
-  }
-
   // TODO(@anonrig): Add V8 Fast API
   static void Remove(const FunctionCallbackInfo<Value>& args) {
     Parser* parser;
@@ -639,25 +610,24 @@ class Parser : public AsyncWrap, public StreamListener {
     ConnectionsList* connectionsList = nullptr;
 
     CHECK(args[0]->IsInt32());
-    CHECK(args[1]->IsObject());
 
-    if (args.Length() > 2) {
-      CHECK(args[2]->IsNumber());
+    if (args.Length() > 1) {
+      CHECK(args[1]->IsNumber());
       max_http_header_size =
-          static_cast<uint64_t>(args[2].As<Number>()->Value());
+          static_cast<uint64_t>(args[1].As<Number>()->Value());
     }
     if (max_http_header_size == 0) {
       max_http_header_size = env->options()->max_http_header_size;
     }
 
-    if (args.Length() > 3) {
-      CHECK(args[3]->IsInt32());
-      lenient_flags = args[3].As<Int32>()->Value();
+    if (args.Length() > 2) {
+      CHECK(args[2]->IsInt32());
+      lenient_flags = args[2].As<Int32>()->Value();
     }
 
-    if (args.Length() > 4 && !args[4]->IsNullOrUndefined()) {
-      CHECK(args[4]->IsObject());
-      ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[4]);
+    if (args.Length() > 3 && !args[3]->IsNullOrUndefined()) {
+      CHECK(args[3]->IsObject());
+      ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[3]);
     }
 
     llhttp_type_t type =
@@ -669,13 +639,6 @@ class Parser : public AsyncWrap, public StreamListener {
     // Should always be called from the same context.
     CHECK_EQ(env, parser->env());
 
-    AsyncWrap::ProviderType provider =
-        (type == HTTP_REQUEST ?
-            AsyncWrap::PROVIDER_HTTPINCOMINGMESSAGE
-            : AsyncWrap::PROVIDER_HTTPCLIENTREQUEST);
-
-    parser->set_provider_type(provider);
-    parser->AsyncReset(args[1].As<Object>());
     parser->Init(type, max_http_header_size, lenient_flags);
 
     if (connectionsList != nullptr) {
@@ -802,7 +765,13 @@ class Parser : public AsyncWrap, public StreamListener {
     current_buffer_len_ = nread;
     current_buffer_data_ = buf.base;
 
-    MakeCallback(cb.As<Function>(), 1, &ret);
+    v8::TryCatch try_catch(env()->isolate());
+    USE(cb.As<Function>()->Call(env()->context(), object(), 1, &ret));
+
+    if (try_catch.HasCaught()) {
+      got_exception_ = true;
+      return;
+    }
 
     current_buffer_len_ = 0;
     current_buffer_data_ = nullptr;
@@ -917,12 +886,11 @@ class Parser : public AsyncWrap, public StreamListener {
       url_.ToString(env())
     };
 
-    MaybeLocal<Value> r = MakeCallback(cb.As<Function>(),
-                                       arraysize(argv),
-                                       argv);
+    v8::TryCatch try_catch(env()->isolate());
+    USE(cb.As<Function>()->Call(
+        env()->context(), object(), arraysize(argv), argv));
 
-    if (r.IsEmpty())
-      got_exception_ = true;
+    if (try_catch.HasCaught()) got_exception_ = true;
 
     url_.Reset();
     have_flushed_ = true;
@@ -1287,9 +1255,7 @@ void CreatePerIsolateProperties(IsolateData* isolate_data,
   t->Set(FIXED_ONE_BYTE_STRING(isolate, "kLenientAll"),
          Integer::NewFromUnsigned(isolate, kLenientAll));
 
-  t->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data));
   SetProtoMethod(isolate, t, "close", Parser::Close);
-  SetProtoMethod(isolate, t, "free", Parser::Free);
   SetProtoMethod(isolate, t, "remove", Parser::Remove);
   SetProtoMethod(isolate, t, "execute", Parser::Execute);
   SetProtoMethod(isolate, t, "finish", Parser::Finish);
@@ -1358,7 +1324,6 @@ void CreatePerContextProperties(Local<Object> target,
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(Parser::New);
   registry->Register(Parser::Close);
-  registry->Register(Parser::Free);
   registry->Register(Parser::Remove);
   registry->Register(Parser::Execute);
   registry->Register(Parser::Finish);

test/async-hooks/test-graph.http.js

@@ -36,13 +36,7 @@ process.on('exit', () => {
       { type: 'TCPCONNECTWRAP',
         id: 'tcpconnect:1',
         triggerAsyncId: 'tcp:1' },
-      { type: 'HTTPCLIENTREQUEST',
-        id: 'httpclientrequest:1',
-        triggerAsyncId: 'tcpserver:1' },
       { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' },
-      { type: 'HTTPINCOMINGMESSAGE',
-        id: 'httpincomingmessage:1',
-        triggerAsyncId: 'tcp:2' },
       { type: 'Timeout',
         id: 'timeout:1',
         triggerAsyncId: null },