Skip to content

Commit

Permalink
vm: avoid unnecessary property getter interceptor calls
Browse files Browse the repository at this point in the history
Access to the global object from within a vm context is intercepted
so it's slow, therefore we should try to avoid unnecessary access
to it during the initialization of vm contexts.

- Remove the Atomics.wake deletion as V8 now does not install it
  anymore.
- Move the Intl.v8BreakIterator deletion into the snapshot.
- Do not query the Object prototype if --disable-proto is not set.

This should speed up the creation of vm contexts by about ~12%.

PR-URL: #44252
Refs: #44014
Refs: #37476
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
  • Loading branch information
joyeecheung authored and RafaelGSS committed Sep 5, 2022
1 parent 736a04a commit c4a45a9
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 53 deletions.
79 changes: 33 additions & 46 deletions src/api/environment.cc
Expand Up @@ -549,50 +549,8 @@ Maybe<bool> InitializeContextRuntime(Local<Context> context) {
Isolate* isolate = context->GetIsolate();
HandleScope handle_scope(isolate);

// Delete `Intl.v8BreakIterator`
// https://github.com/nodejs/node/issues/14909
{
Local<String> intl_string =
FIXED_ONE_BYTE_STRING(isolate, "Intl");
Local<String> break_iter_string =
FIXED_ONE_BYTE_STRING(isolate, "v8BreakIterator");

Local<Value> intl_v;
if (!context->Global()
->Get(context, intl_string)
.ToLocal(&intl_v)) {
return Nothing<bool>();
}

if (intl_v->IsObject() &&
intl_v.As<Object>()
->Delete(context, break_iter_string)
.IsNothing()) {
return Nothing<bool>();
}
}

// Delete `Atomics.wake`
// https://github.com/nodejs/node/issues/21219
{
Local<String> atomics_string =
FIXED_ONE_BYTE_STRING(isolate, "Atomics");
Local<String> wake_string =
FIXED_ONE_BYTE_STRING(isolate, "wake");

Local<Value> atomics_v;
if (!context->Global()
->Get(context, atomics_string)
.ToLocal(&atomics_v)) {
return Nothing<bool>();
}

if (atomics_v->IsObject() &&
atomics_v.As<Object>()
->Delete(context, wake_string)
.IsNothing()) {
return Nothing<bool>();
}
if (per_process::cli_options->disable_proto == "") {
return Just(true);
}

// Remove __proto__
Expand Down Expand Up @@ -654,7 +612,32 @@ Maybe<bool> InitializeContextRuntime(Local<Context> context) {
return Just(true);
}

Maybe<bool> InitializeContextForSnapshot(Local<Context> context) {
Maybe<bool> InitializeBaseContextForSnapshot(Local<Context> context) {
Isolate* isolate = context->GetIsolate();
HandleScope handle_scope(isolate);

// Delete `Intl.v8BreakIterator`
// https://github.com/nodejs/node/issues/14909
{
Context::Scope context_scope(context);
Local<String> intl_string = FIXED_ONE_BYTE_STRING(isolate, "Intl");
Local<String> break_iter_string =
FIXED_ONE_BYTE_STRING(isolate, "v8BreakIterator");

Local<Value> intl_v;
if (!context->Global()->Get(context, intl_string).ToLocal(&intl_v)) {
return Nothing<bool>();
}

if (intl_v->IsObject() &&
intl_v.As<Object>()->Delete(context, break_iter_string).IsNothing()) {
return Nothing<bool>();
}
}
return Just(true);
}

Maybe<bool> InitializeMainContextForSnapshot(Local<Context> context) {
Isolate* isolate = context->GetIsolate();
HandleScope handle_scope(isolate);

Expand All @@ -664,6 +647,9 @@ Maybe<bool> InitializeContextForSnapshot(Local<Context> context) {
context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
True(isolate));

if (InitializeBaseContextForSnapshot(context).IsNothing()) {
return Nothing<bool>();
}
return InitializePrimordials(context);
}

Expand Down Expand Up @@ -710,8 +696,9 @@ Maybe<bool> InitializePrimordials(Local<Context> context) {
return Just(true);
}

// This initializes the main context (i.e. vm contexts are not included).
Maybe<bool> InitializeContext(Local<Context> context) {
if (InitializeContextForSnapshot(context).IsNothing()) {
if (InitializeMainContextForSnapshot(context).IsNothing()) {
return Nothing<bool>();
}

Expand Down
16 changes: 9 additions & 7 deletions src/node_contextify.cc
Expand Up @@ -206,14 +206,16 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
{}, // global object
{}, // deserialization callback
queue);
if (ctx.IsEmpty()) return MaybeLocal<Context>();
if (ctx.IsEmpty() || InitializeBaseContextForSnapshot(ctx).IsNothing()) {
return MaybeLocal<Context>();
}
} else if (!Context::FromSnapshot(isolate,
SnapshotData::kNodeVMContextIndex,
{}, // deserialization callback
nullptr, // extensions
{}, // global object
queue)
.ToLocal(&ctx)) {
SnapshotData::kNodeVMContextIndex,
{}, // deserialization callback
nullptr, // extensions
{}, // global object
queue)
.ToLocal(&ctx)) {
return MaybeLocal<Context>();
}
return scope.Escape(ctx);
Expand Down
2 changes: 2 additions & 0 deletions src/node_internals.h
Expand Up @@ -92,6 +92,8 @@ void SignalExit(int signal, siginfo_t* info, void* ucontext);
std::string GetProcessTitle(const char* default_title);
std::string GetHumanReadableProcessName();

v8::Maybe<bool> InitializeBaseContextForSnapshot(
v8::Local<v8::Context> context);
v8::Maybe<bool> InitializeContextRuntime(v8::Local<v8::Context> context);
v8::Maybe<bool> InitializePrimordials(v8::Local<v8::Context> context);

Expand Down

0 comments on commit c4a45a9

Please sign in to comment.