Skip to content

Commit a555be2

Browse files
addaleaxTrott
authored andcommitted
worker: improve MessagePort performance
Use a JS function as the single entry point for emitting `.onmessage()` calls, avoiding the overhead of manually constructing each message event object in C++. confidence improvement accuracy (*) (**) (***) worker/echo.js n=100000 sendsPerBroadcast=1 payload='object' workers=1 *** 16.34 % ±1.16% ±1.54% ±1.99% worker/echo.js n=100000 sendsPerBroadcast=1 payload='string' workers=1 *** 24.41 % ±1.50% ±1.99% ±2.58% worker/echo.js n=100000 sendsPerBroadcast=10 payload='object' workers=1 *** 26.66 % ±1.54% ±2.05% ±2.65% worker/echo.js n=100000 sendsPerBroadcast=10 payload='string' workers=1 *** 32.72 % ±1.60% ±2.11% ±2.73% worker/messageport.js n=1000000 payload='object' *** 40.28 % ±1.48% ±1.95% ±2.52% worker/messageport.js n=1000000 payload='string' *** 76.95 % ±2.19% ±2.90% ±3.75% Also fix handling exceptions returned from `MessagePort::New`. PR-URL: #31605 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent e2a3a87 commit a555be2

File tree

6 files changed

+61
-24
lines changed

6 files changed

+61
-24
lines changed
+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
class MessageEvent {
3+
constructor(data, target) {
4+
this.data = data;
5+
this.target = target;
6+
}
7+
}
8+
9+
exports.emitMessage = function(data) {
10+
if (typeof this.onmessage === 'function')
11+
this.onmessage(new MessageEvent(data, this));
12+
};

node.gyp

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
'lib/internal/bootstrap/switches/is_not_main_thread.js',
3838
'lib/internal/per_context/primordials.js',
3939
'lib/internal/per_context/domexception.js',
40+
'lib/internal/per_context/messageport.js',
4041
'lib/async_hooks.js',
4142
'lib/assert.js',
4243
'lib/buffer.js',

src/api/environment.cc

+1
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ bool InitializeContextForSnapshot(Local<Context> context) {
482482

483483
static const char* context_files[] = {"internal/per_context/primordials",
484484
"internal/per_context/domexception",
485+
"internal/per_context/messageport",
485486
nullptr};
486487

487488
for (const char** module = context_files; *module != nullptr; module++) {

src/env.h

-1
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,6 @@ constexpr size_t kFsStatsBufferLength =
403403
V(http2stream_constructor_template, v8::ObjectTemplate) \
404404
V(http2ping_constructor_template, v8::ObjectTemplate) \
405405
V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
406-
V(message_event_object_template, v8::ObjectTemplate) \
407406
V(message_port_constructor_template, v8::FunctionTemplate) \
408407
V(pipe_constructor_template, v8::FunctionTemplate) \
409408
V(promise_wrap_template, v8::ObjectTemplate) \

src/node_messaging.cc

+41-22
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ using v8::Maybe;
2929
using v8::MaybeLocal;
3030
using v8::Nothing;
3131
using v8::Object;
32-
using v8::ObjectTemplate;
3332
using v8::SharedArrayBuffer;
3433
using v8::String;
3534
using v8::Symbol;
@@ -170,6 +169,20 @@ uint32_t Message::AddWASMModule(CompiledWasmModule&& mod) {
170169

171170
namespace {
172171

172+
MaybeLocal<Function> GetEmitMessageFunction(Local<Context> context) {
173+
Isolate* isolate = context->GetIsolate();
174+
Local<Object> per_context_bindings;
175+
Local<Value> emit_message_val;
176+
if (!GetPerContextExports(context).ToLocal(&per_context_bindings) ||
177+
!per_context_bindings->Get(context,
178+
FIXED_ONE_BYTE_STRING(isolate, "emitMessage"))
179+
.ToLocal(&emit_message_val)) {
180+
return MaybeLocal<Function>();
181+
}
182+
CHECK(emit_message_val->IsFunction());
183+
return emit_message_val.As<Function>();
184+
}
185+
173186
MaybeLocal<Function> GetDOMException(Local<Context> context) {
174187
Isolate* isolate = context->GetIsolate();
175188
Local<Object> per_context_bindings;
@@ -471,20 +484,31 @@ MessagePort::MessagePort(Environment* env,
471484
MessagePort* channel = ContainerOf(&MessagePort::async_, handle);
472485
channel->OnMessage();
473486
};
487+
474488
CHECK_EQ(uv_async_init(env->event_loop(),
475489
&async_,
476490
onmessage), 0);
477-
async_.data = static_cast<void*>(this);
491+
async_.data = nullptr; // Reset later to indicate success of the constructor.
492+
auto cleanup = OnScopeLeave([&]() {
493+
if (async_.data == nullptr) Close();
494+
});
478495

479496
Local<Value> fn;
480497
if (!wrap->Get(context, env->oninit_symbol()).ToLocal(&fn))
481498
return;
482499

483500
if (fn->IsFunction()) {
484501
Local<Function> init = fn.As<Function>();
485-
USE(init->Call(context, wrap, 0, nullptr));
502+
if (init->Call(context, wrap, 0, nullptr).IsEmpty())
503+
return;
486504
}
487505

506+
Local<Function> emit_message_fn;
507+
if (!GetEmitMessageFunction(context).ToLocal(&emit_message_fn))
508+
return;
509+
emit_message_fn_.Reset(env->isolate(), emit_message_fn);
510+
511+
async_.data = static_cast<void*>(this);
488512
Debug(this, "Created message port");
489513
}
490514

@@ -532,6 +556,11 @@ MessagePort* MessagePort::New(
532556
return nullptr;
533557
MessagePort* port = new MessagePort(env, context, instance);
534558
CHECK_NOT_NULL(port);
559+
if (port->IsHandleClosing()) {
560+
// Construction failed with an exception.
561+
return nullptr;
562+
}
563+
535564
if (data) {
536565
port->Detach();
537566
port->data_ = std::move(data);
@@ -624,16 +653,8 @@ void MessagePort::OnMessage() {
624653
continue;
625654
}
626655

627-
Local<Object> event;
628-
Local<Value> cb_args[1];
629-
if (!env()->message_event_object_template()->NewInstance(context)
630-
.ToLocal(&event) ||
631-
event->Set(context, env()->data_string(), payload).IsNothing() ||
632-
event->Set(context, env()->target_string(), object()).IsNothing() ||
633-
(cb_args[0] = event, false) ||
634-
MakeCallback(env()->onmessage_string(),
635-
arraysize(cb_args),
636-
cb_args).IsEmpty()) {
656+
Local<Function> emit_message = PersistentToLocal::Strong(emit_message_fn_);
657+
if (MakeCallback(emit_message, 1, &payload).IsEmpty()) {
637658
// Re-schedule OnMessage() execution in case of failure.
638659
if (data_)
639660
TriggerAsync();
@@ -902,6 +923,7 @@ void MessagePort::Entangle(MessagePort* a, MessagePortData* b) {
902923

903924
void MessagePort::MemoryInfo(MemoryTracker* tracker) const {
904925
tracker->TrackField("data", data_);
926+
tracker->TrackField("emit_message_fn", emit_message_fn_);
905927
}
906928

907929
Local<FunctionTemplate> GetMessagePortConstructorTemplate(Environment* env) {
@@ -911,8 +933,6 @@ Local<FunctionTemplate> GetMessagePortConstructorTemplate(Environment* env) {
911933
if (!templ.IsEmpty())
912934
return templ;
913935

914-
Isolate* isolate = env->isolate();
915-
916936
{
917937
Local<FunctionTemplate> m = env->NewFunctionTemplate(MessagePort::New);
918938
m->SetClassName(env->message_port_constructor_string());
@@ -923,13 +943,6 @@ Local<FunctionTemplate> GetMessagePortConstructorTemplate(Environment* env) {
923943
env->SetProtoMethod(m, "start", MessagePort::Start);
924944

925945
env->set_message_port_constructor_template(m);
926-
927-
Local<FunctionTemplate> event_ctor = FunctionTemplate::New(isolate);
928-
event_ctor->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "MessageEvent"));
929-
Local<ObjectTemplate> e = event_ctor->InstanceTemplate();
930-
e->Set(env->data_string(), Null(isolate));
931-
e->Set(env->target_string(), Null(isolate));
932-
env->set_message_event_object_template(e);
933946
}
934947

935948
return GetMessagePortConstructorTemplate(env);
@@ -948,7 +961,13 @@ static void MessageChannel(const FunctionCallbackInfo<Value>& args) {
948961
Context::Scope context_scope(context);
949962

950963
MessagePort* port1 = MessagePort::New(env, context);
964+
if (port1 == nullptr) return;
951965
MessagePort* port2 = MessagePort::New(env, context);
966+
if (port2 == nullptr) {
967+
port1->Close();
968+
return;
969+
}
970+
952971
MessagePort::Entangle(port1, port2);
953972

954973
args.This()->Set(context, env->port1_string(), port1->object())

src/node_messaging.h

+6-1
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,16 @@ class MessagePortData : public MemoryRetainer {
131131
// the uv_async_t handle that is used to notify the current event loop of
132132
// new incoming messages.
133133
class MessagePort : public HandleWrap {
134-
public:
134+
private:
135135
// Create a new MessagePort. The `context` argument specifies the Context
136136
// instance that is used for creating the values emitted from this port.
137+
// This is called by MessagePort::New(), which is the public API used for
138+
// creating MessagePort instances.
137139
MessagePort(Environment* env,
138140
v8::Local<v8::Context> context,
139141
v8::Local<v8::Object> wrap);
142+
143+
public:
140144
~MessagePort() override;
141145

142146
// Create a new message port instance, optionally over an existing
@@ -205,6 +209,7 @@ class MessagePort : public HandleWrap {
205209
std::unique_ptr<MessagePortData> data_ = nullptr;
206210
bool receiving_messages_ = false;
207211
uv_async_t async_;
212+
v8::Global<v8::Function> emit_message_fn_;
208213

209214
friend class MessagePortData;
210215
};

0 commit comments

Comments
 (0)