src: use object to pass Environment to functions

addaleax · addaleax · commit 955be8623d58 · 2019-03-05T00:56:37.000+01:00
Use a `v8::Object` with an internal field, rather than a `v8::External`. On a `GetReturnValue().Set(Environment::GetCurrent(args) == nullptr)` noop function, this benchmarks as a ~60 % speedup, as calls to `obj->GetAlignedPointerFromInternalField()` can be inlined and the field is stored with one level of indirection less. This also makes breaking up some pieces of the `Environment` class into per-native-binding data easier, if we want to pursue that path in the future. PR-URL: #26382 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -316,16 +316,23 @@ inline Environment* Environment::GetCurrent(v8::Local<v8::Context> context) {
 
 inline Environment* Environment::GetCurrent(
     const v8::FunctionCallbackInfo<v8::Value>& info) {
-  CHECK(info.Data()->IsExternal());
-  return static_cast<Environment*>(info.Data().As<v8::External>()->Value());
+  return GetFromCallbackData(info.Data());
 }
 
 template <typename T>
 inline Environment* Environment::GetCurrent(
     const v8::PropertyCallbackInfo<T>& info) {
-  CHECK(info.Data()->IsExternal());
-  return static_cast<Environment*>(
-      info.Data().template As<v8::External>()->Value());
+  return GetFromCallbackData(info.Data());
+}
+
+inline Environment* Environment::GetFromCallbackData(v8::Local<v8::Value> val) {
+  DCHECK(val->IsObject());
+  v8::Local<v8::Object> obj = val.As<v8::Object>();
+  DCHECK_GE(obj->InternalFieldCount(), 1);
+  Environment* env =
+      static_cast<Environment*>(obj->GetAlignedPointerFromInternalField(0));
+  DCHECK(env->as_callback_data_template()->HasInstance(obj));
+  return env;
 }
 
 inline Environment* Environment::GetThreadLocalEnv() {
@@ -857,7 +864,7 @@ inline v8::Local<v8::FunctionTemplate>
                                      v8::Local<v8::Signature> signature,
                                      v8::ConstructorBehavior behavior,
                                      v8::SideEffectType side_effect_type) {
-  v8::Local<v8::External> external = as_external();
+  v8::Local<v8::Object> external = as_callback_data();
   return v8::FunctionTemplate::New(isolate(), callback, external,
                                    signature, 0, behavior, side_effect_type);
 }
diff --git a/src/env.cc b/src/env.cc
@@ -25,8 +25,8 @@ using v8::ArrayBuffer;
 using v8::Boolean;
 using v8::Context;
 using v8::EmbedderGraph;
-using v8::External;
 using v8::Function;
+using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Integer;
 using v8::Isolate;
@@ -195,7 +195,16 @@ Environment::Environment(IsolateData* isolate_data,
   // We'll be creating new objects so make sure we've entered the context.
   HandleScope handle_scope(isolate());
   Context::Scope context_scope(context);
-  set_as_external(External::New(isolate(), this));
+  {
+    Local<FunctionTemplate> templ = FunctionTemplate::New(isolate());
+    templ->InstanceTemplate()->SetInternalFieldCount(1);
+    Local<Object> obj =
+        templ->GetFunction(context).ToLocalChecked()->NewInstance(
+            context).ToLocalChecked();
+    obj->SetAlignedPointerInInternalField(0, this);
+    set_as_callback_data(obj);
+    set_as_callback_data_template(templ);
+  }
 
   // We create new copies of the per-Environment option sets, so that it is
   // easier to modify them after Environment creation. The defaults are
diff --git a/src/env.h b/src/env.h
@@ -321,7 +321,8 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
   V(zero_return_string, "ZERO_RETURN")
 
 #define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)                            \
-  V(as_external, v8::External)                                                 \
+  V(as_callback_data, v8::Object)                                              \
+  V(as_callback_data_template, v8::FunctionTemplate)                           \
   V(async_hooks_after_function, v8::Function)                                  \
   V(async_hooks_before_function, v8::Function)                                 \
   V(async_hooks_binding, v8::Object)                                           \
@@ -663,6 +664,8 @@ class Environment {
   static inline Environment* GetCurrent(
       const v8::PropertyCallbackInfo<T>& info);
 
+  static inline Environment* GetFromCallbackData(v8::Local<v8::Value> val);
+
   static uv_key_t thread_local_env;
   static inline Environment* GetThreadLocalEnv();
 
diff --git a/src/fs_event_wrap.cc b/src/fs_event_wrap.cc
@@ -111,7 +111,7 @@ void FSEventWrap::Initialize(Local<Object> target,
   Local<FunctionTemplate> get_initialized_templ =
       FunctionTemplate::New(env->isolate(),
                             GetInitialized,
-                            env->as_external(),
+                            env->as_callback_data(),
                             Signature::New(env->isolate(), t));
 
   t->PrototypeTemplate()->SetAccessorProperty(
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -371,7 +371,7 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> ctx_getter_templ =
       FunctionTemplate::New(env->isolate(),
                             CtxGetter,
-                            env->as_external(),
+                            env->as_callback_data(),
                             Signature::New(env->isolate(), t));
 
 
@@ -4789,7 +4789,7 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
     Local<FunctionTemplate> verify_error_getter_templ =
         FunctionTemplate::New(env->isolate(),
                               DiffieHellman::VerifyErrorGetter,
-                              env->as_external(),
+                              env->as_callback_data(),
                               Signature::New(env->isolate(), t),
                               /* length */ 0,
                               ConstructorBehavior::kThrow,
diff --git a/src/node_env_var.cc b/src/node_env_var.cc
@@ -212,7 +212,7 @@ static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
 
 MaybeLocal<Object> CreateEnvVarProxy(Local<Context> context,
                                      Isolate* isolate,
-                                     Local<Value> data) {
+                                     Local<Object> data) {
   EscapableHandleScope scope(isolate);
   Local<ObjectTemplate> env_proxy_template = ObjectTemplate::New(isolate);
   env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration(
diff --git a/src/node_process.h b/src/node_process.h
@@ -9,7 +9,7 @@ namespace node {
 
 v8::MaybeLocal<v8::Object> CreateEnvVarProxy(v8::Local<v8::Context> context,
                                              v8::Isolate* isolate,
-                                             v8::Local<v8::Value> data);
+                                             v8::Local<v8::Object> data);
 
 // Most of the time, it's best to use `console.error` to write
 // to the process.stderr stream.  However, in some cases, such as
diff --git a/src/node_process_object.cc b/src/node_process_object.cc
@@ -92,7 +92,7 @@ MaybeLocal<Object> CreateProcessObject(
                 title_string,
                 ProcessTitleGetter,
                 env->owns_process_state() ? ProcessTitleSetter : nullptr,
-                env->as_external(),
+                env->as_callback_data(),
                 DEFAULT,
                 None,
                 SideEffectType::kHasNoSideEffect)
@@ -152,7 +152,7 @@ MaybeLocal<Object> CreateProcessObject(
                    .ToLocalChecked()).FromJust();
 
   Local<Object> env_var_proxy;
-  if (!CreateEnvVarProxy(context, isolate, env->as_external())
+  if (!CreateEnvVarProxy(context, isolate, env->as_callback_data())
            .ToLocal(&env_var_proxy))
     return MaybeLocal<Object>();
 
@@ -310,7 +310,7 @@ MaybeLocal<Object> CreateProcessObject(
                           debug_port_string,
                           DebugPortGetter,
                           env->owns_process_state() ? DebugPortSetter : nullptr,
-                          env->as_external())
+                          env->as_callback_data())
             .FromJust());
 
   // process._rawDebug: may be overwritten later in JS land, but should be
diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
@@ -137,7 +137,7 @@ Local<FunctionTemplate> LibuvStreamWrap::GetConstructorTemplate(
     Local<FunctionTemplate> get_write_queue_size =
         FunctionTemplate::New(env->isolate(),
                               GetWriteQueueSize,
-                              env->as_external(),
+                              env->as_callback_data(),
                               Signature::New(env->isolate(), tmpl));
     tmpl->PrototypeTemplate()->SetAccessorProperty(
         env->write_queue_size_string(),
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -962,7 +962,7 @@ void TLSWrap::Initialize(Local<Object> target,
   Local<FunctionTemplate> get_write_queue_size =
       FunctionTemplate::New(env->isolate(),
                             GetWriteQueueSize,
-                            env->as_external(),
+                            env->as_callback_data(),
                             Signature::New(env->isolate(), t));
   t->PrototypeTemplate()->SetAccessorProperty(
       env->write_queue_size_string(),
diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc
@@ -105,7 +105,7 @@ void UDPWrap::Initialize(Local<Object> target,
   Local<FunctionTemplate> get_fd_templ =
       FunctionTemplate::New(env->isolate(),
                             UDPWrap::GetFD,
-                            env->as_external(),
+                            env->as_callback_data(),
                             signature);
 
   t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(),
