vm: avoid unnecessary property getter interceptor calls

joyeecheung · RafaelGSS · commit c4a45a93f335 · 2022-09-05T11:32:53.000-03:00
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>
diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -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__
@@ -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);
 
@@ -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);
 }
 
@@ -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>();
   }
 
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -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);
diff --git a/src/node_internals.h b/src/node_internals.h
@@ -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);
 
