Skip to content

Commit a7399d4

Browse files
JonasBaJonasBa
committed
http: improve performance by removing async_hooks
1 parent c922bd8 commit a7399d4

18 files changed

+71
-416
lines changed

‎benchmark/http/bench-parser.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@ function main({ len, n }) {
2424
bench.start();
2525
for (let i = 0; i < n; i++) {
2626
parser.execute(header, 0, header.length);
27-
parser.initialize(REQUEST, {});
27+
parser.initialize(REQUEST);
2828
}
2929
bench.end(n);
3030
}
3131

3232
function newParser(type) {
3333
const parser = new HTTPParser();
34-
parser.initialize(type, {});
34+
parser.initialize(type);
3535

3636
parser.headers = [];
3737

‎lib/_http_client.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,6 @@ function tickOnSocket(req, socket) {
824824
const lenient = req.insecureHTTPParser === undefined ?
825825
isLenient() : req.insecureHTTPParser;
826826
parser.initialize(HTTPParser.RESPONSE,
827-
new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
828827
req.maxHeaderSize || 0,
829828
lenient ? kLenientAll : kLenientNone);
830829
parser.socket = socket;

‎lib/_http_server.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,6 @@ function connectionListenerInternal(server, socket) {
687687
// https://github.com/nodejs/node/pull/21313
688688
parser.initialize(
689689
HTTPParser.REQUEST,
690-
new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket),
691690
server.maxHeaderSize || 0,
692691
lenient ? kLenientAll : kLenientNone,
693692
server[kConnections],

‎src/async_wrap.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ namespace node {
4949
V(HTTP2STREAM) \
5050
V(HTTP2PING) \
5151
V(HTTP2SETTINGS) \
52-
V(HTTPINCOMINGMESSAGE) \
53-
V(HTTPCLIENTREQUEST) \
5452
V(JSSTREAM) \
5553
V(JSUDPWRAP) \
5654
V(MESSAGEPORT) \

‎src/node_http_parser.cc

Lines changed: 25 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include "node_buffer.h"
2424
#include "util.h"
2525

26-
#include "async_wrap-inl.h"
2726
#include "env-inl.h"
2827
#include "llhttp.h"
2928
#include "memory_tracker-inl.h"
@@ -250,17 +249,16 @@ class ConnectionsList : public BaseObject {
250249
std::set<Parser*, ParserComparator> active_connections_;
251250
};
252251

253-
class Parser : public AsyncWrap, public StreamListener {
252+
class Parser : public BaseObject, public StreamListener {
254253
friend class ConnectionsList;
255254
friend struct ParserComparator;
256255

257256
public:
258257
Parser(BindingData* binding_data, Local<Object> wrap)
259-
: AsyncWrap(binding_data->env(), wrap),
258+
: BaseObject(binding_data->env(), wrap),
260259
current_buffer_len_(0),
261260
current_buffer_data_(nullptr),
262-
binding_data_(binding_data) {
263-
}
261+
binding_data_(binding_data) {}
264262

265263
SET_NO_MEMORY_INFO()
266264
SET_MEMORY_INFO_NAME(Parser)
@@ -289,13 +287,10 @@ class Parser : public AsyncWrap, public StreamListener {
289287
Local<Value> cb = object()->Get(env()->context(), kOnMessageBegin)
290288
.ToLocalChecked();
291289
if (cb->IsFunction()) {
292-
InternalCallbackScope callback_scope(
293-
this, InternalCallbackScope::kSkipTaskQueues);
294-
295-
MaybeLocal<Value> r = cb.As<Function>()->Call(
296-
env()->context(), object(), 0, nullptr);
290+
MaybeLocal<Value> r =
291+
cb.As<Function>()->Call(env()->context(), object(), 0, nullptr);
297292

298-
if (r.IsEmpty()) callback_scope.MarkAsFailed();
293+
// if (r.IsEmpty()) got_exception_ = true;
299294
}
300295

301296
return 0;
@@ -442,14 +437,8 @@ class Parser : public AsyncWrap, public StreamListener {
442437

443438
argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade);
444439

445-
MaybeLocal<Value> head_response;
446-
{
447-
InternalCallbackScope callback_scope(
448-
this, InternalCallbackScope::kSkipTaskQueues);
449-
head_response = cb.As<Function>()->Call(
450-
env()->context(), object(), arraysize(argv), argv);
451-
if (head_response.IsEmpty()) callback_scope.MarkAsFailed();
452-
}
440+
MaybeLocal<Value> head_response = cb.As<Function>()->Call(
441+
env()->context(), object(), arraysize(argv), argv);
453442

454443
int64_t val;
455444

@@ -478,7 +467,8 @@ class Parser : public AsyncWrap, public StreamListener {
478467

479468
Local<Value> buffer = Buffer::Copy(env, at, length).ToLocalChecked();
480469

481-
MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 1, &buffer);
470+
MaybeLocal<Value> r =
471+
cb.As<Function>()->Call(env->context(), object(), 1, &buffer);
482472

483473
if (r.IsEmpty()) {
484474
got_exception_ = true;
@@ -516,14 +506,8 @@ class Parser : public AsyncWrap, public StreamListener {
516506
if (!cb->IsFunction())
517507
return 0;
518508

519-
MaybeLocal<Value> r;
520-
{
521-
InternalCallbackScope callback_scope(
522-
this, InternalCallbackScope::kSkipTaskQueues);
523-
r = cb.As<Function>()->Call(env()->context(), object(), 0, nullptr);
524-
if (r.IsEmpty()) callback_scope.MarkAsFailed();
525-
}
526-
509+
MaybeLocal<Value> r =
510+
cb.As<Function>()->Call(env()->context(), object(), 0, nullptr);
527511
if (r.IsEmpty()) {
528512
got_exception_ = true;
529513
return -1;
@@ -575,11 +559,6 @@ class Parser : public AsyncWrap, public StreamListener {
575559
static void Free(const FunctionCallbackInfo<Value>& args) {
576560
Parser* parser;
577561
ASSIGN_OR_RETURN_UNWRAP(&parser, args.This());
578-
579-
// Since the Parser destructor isn't going to run the destroy() callbacks
580-
// it needs to be triggered manually.
581-
parser->EmitTraceEventDestroy();
582-
parser->EmitDestroy();
583562
}
584563

585564
static void Remove(const FunctionCallbackInfo<Value>& args) {
@@ -638,25 +617,24 @@ class Parser : public AsyncWrap, public StreamListener {
638617
ConnectionsList* connectionsList = nullptr;
639618

640619
CHECK(args[0]->IsInt32());
641-
CHECK(args[1]->IsObject());
642620

643-
if (args.Length() > 2) {
644-
CHECK(args[2]->IsNumber());
621+
if (args.Length() > 1) {
622+
CHECK(args[1]->IsNumber());
645623
max_http_header_size =
646-
static_cast<uint64_t>(args[2].As<Number>()->Value());
624+
static_cast<uint64_t>(args[1].As<Number>()->Value());
647625
}
648626
if (max_http_header_size == 0) {
649627
max_http_header_size = env->options()->max_http_header_size;
650628
}
651629

652-
if (args.Length() > 3) {
653-
CHECK(args[3]->IsInt32());
654-
lenient_flags = args[3].As<Int32>()->Value();
630+
if (args.Length() > 2) {
631+
CHECK(args[2]->IsInt32());
632+
lenient_flags = args[2].As<Int32>()->Value();
655633
}
656634

657-
if (args.Length() > 4 && !args[4]->IsNullOrUndefined()) {
658-
CHECK(args[4]->IsObject());
659-
ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[4]);
635+
if (args.Length() > 3 && !args[3]->IsNullOrUndefined()) {
636+
CHECK(args[3]->IsObject());
637+
ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[3]);
660638
}
661639

662640
llhttp_type_t type =
@@ -668,13 +646,6 @@ class Parser : public AsyncWrap, public StreamListener {
668646
// Should always be called from the same context.
669647
CHECK_EQ(env, parser->env());
670648

671-
AsyncWrap::ProviderType provider =
672-
(type == HTTP_REQUEST ?
673-
AsyncWrap::PROVIDER_HTTPINCOMINGMESSAGE
674-
: AsyncWrap::PROVIDER_HTTPCLIENTREQUEST);
675-
676-
parser->set_provider_type(provider);
677-
parser->AsyncReset(args[1].As<Object>());
678649
parser->Init(type, max_http_header_size, lenient_flags);
679650

680651
if (connectionsList != nullptr) {
@@ -820,7 +791,7 @@ class Parser : public AsyncWrap, public StreamListener {
820791
current_buffer_len_ = nread;
821792
current_buffer_data_ = buf.base;
822793

823-
MakeCallback(cb.As<Function>(), 1, &ret);
794+
USE(cb.As<Function>()->Call(env()->context(), object(), 1, &ret));
824795

825796
current_buffer_len_ = 0;
826797
current_buffer_data_ = nullptr;
@@ -935,11 +906,9 @@ class Parser : public AsyncWrap, public StreamListener {
935906
url_.ToString(env())
936907
};
937908

938-
MaybeLocal<Value> r = MakeCallback(cb.As<Function>(),
939-
arraysize(argv),
940-
argv);
941-
942-
if (r.IsEmpty())
909+
if (cb.As<Function>()
910+
->Call(env()->context(), object(), arraysize(argv), argv)
911+
.IsEmpty())
943912
got_exception_ = true;
944913

945914
url_.Reset();
@@ -1299,7 +1268,6 @@ void CreatePerIsolateProperties(IsolateData* isolate_data,
12991268
t->Set(FIXED_ONE_BYTE_STRING(isolate, "kLenientAll"),
13001269
Integer::NewFromUnsigned(isolate, kLenientAll));
13011270

1302-
t->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data));
13031271
SetProtoMethod(isolate, t, "close", Parser::Close);
13041272
SetProtoMethod(isolate, t, "free", Parser::Free);
13051273
SetProtoMethod(isolate, t, "remove", Parser::Remove);

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,7 @@ process.on('exit', () => {
3636
{ type: 'TCPCONNECTWRAP',
3737
id: 'tcpconnect:1',
3838
triggerAsyncId: 'tcp:1' },
39-
{ type: 'HTTPCLIENTREQUEST',
40-
id: 'httpclientrequest:1',
41-
triggerAsyncId: 'tcpserver:1' },
4239
{ type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' },
43-
{ type: 'HTTPINCOMINGMESSAGE',
44-
id: 'httpincomingmessage:1',
45-
triggerAsyncId: 'tcp:2' },
4640
{ type: 'Timeout',
4741
id: 'timeout:1',
4842
triggerAsyncId: null },

‎test/async-hooks/test-httpparser-reuse.js

Lines changed: 0 additions & 75 deletions
This file was deleted.

‎test/async-hooks/test-httpparser.request.js

Lines changed: 0 additions & 53 deletions
This file was deleted.

0 commit comments

Comments
 (0)