process: JS fast path for bindings

Currently, both process.binding and internalBinding have to call into
C++ regardless of whether the module has been cached or not. This
creates significant overhead to all binding calls and unfortunately
the rest of the codebase doesn't really optimize the amount of times
that bindings are required (as an example: 12 files require the
async_wrap binding).

Changing all the usage of this function throughout the codebase would
introduce a lot of churn (and is kind of a hassle) so instead this PR
introduces a JS fast path for both functions for cases where the
binding has already been cached. While micro benchmarks are not super
meaningful here (it's not like we call binding that often...), this
does speed up the cached call by 400%.

In addition, move moduleLoadList creation and management entirely into
JS-land as it requires less code and is more efficient.

PR-URL: https://github.com/nodejs/node/pull/18365
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

3 files changed, 6 insertions, 74 deletions

diff --git a/src/env-inl.h b/src/env-inl.h
index 202393f679..545a92ba17 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -328,18 +328,6 @@ inline Environment::Environment(IsolateData* isolate_data,
   v8::Context::Scope context_scope(context);
   set_as_external(v8::External::New(isolate(), this));
 
-  v8::Local<v8::Primitive> null = v8::Null(isolate());
-  v8::Local<v8::Object> binding_cache_object = v8::Object::New(isolate());
-  CHECK(binding_cache_object->SetPrototype(context, null).FromJust());
-  set_binding_cache_object(binding_cache_object);
-
-  v8::Local<v8::Object> internal_binding_cache_object =
-      v8::Object::New(isolate());
-  CHECK(internal_binding_cache_object->SetPrototype(context, null).FromJust());
-  set_internal_binding_cache_object(internal_binding_cache_object);
-
-  set_module_load_list_array(v8::Array::New(isolate()));
-
   AssignToContext(context, ContextInfo(""));
 
   destroy_async_id_list_.reserve(512);
diff --git a/src/env.h b/src/env.h
index b24ada5740..d73be8156e 100644
--- a/src/env.h
+++ b/src/env.h
@@ -274,8 +274,6 @@ class ModuleWrap;
   V(async_hooks_after_function, v8::Function)                                 \
   V(async_hooks_promise_resolve_function, v8::Function)                       \
   V(async_hooks_binding, v8::Object)                                          \
-  V(binding_cache_object, v8::Object)                                         \
-  V(internal_binding_cache_object, v8::Object)                                \
   V(buffer_prototype_object, v8::Object)                                      \
   V(context, v8::Context)                                                     \
   V(domain_callback, v8::Function)                                            \
@@ -285,7 +283,6 @@ class ModuleWrap;
   V(http2settings_constructor_template, v8::ObjectTemplate)                   \
   V(immediate_callback_function, v8::Function)                                \
   V(inspector_console_api_object, v8::Object)                                 \
-  V(module_load_list_array, v8::Array)                                        \
   V(pbkdf2_constructor_template, v8::ObjectTemplate)                          \
   V(pipe_constructor_template, v8::FunctionTemplate)                          \
   V(performance_entry_callback, v8::Function)                                 \
diff --git a/src/node.cc b/src/node.cc
index c9af7c7dcd..2e1d08f5b5 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -2460,22 +2460,6 @@ Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
 }
 
 
-static bool PullFromCache(Environment* env,
-                          const FunctionCallbackInfo<Value>& args,
-                          Local<String> module,
-                          Local<Object> cache) {
-  Local<Context> context = env->context();
-  Local<Value> exports_v;
-  Local<Object> exports;
-  if (cache->Get(context, module).ToLocal(&exports_v) &&
-      exports_v->IsObject() &&
-      exports_v->ToObject(context).ToLocal(&exports)) {
-    args.GetReturnValue().Set(exports);
-    return true;
-  }
-  return false;
-}
-
 static Local<Object> InitModule(Environment* env,
                                  node_module* mod,
                                  Local<String> module) {
@@ -2503,22 +2487,10 @@ static void ThrowIfNoSuchModule(Environment* env, const char* module_v) {
 static void Binding(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  Local<String> module;
-  if (!args[0]->ToString(env->context()).ToLocal(&module)) return;
-
-  Local<Object> cache = env->binding_cache_object();
-
-  if (PullFromCache(env, args, module, cache))
-    return;
+  CHECK(args[0]->IsString());
 
-  // Append a string to process.moduleLoadList
-  char buf[1024];
+  Local<String> module = args[0].As<String>();
   node::Utf8Value module_v(env->isolate(), module);
-  snprintf(buf, sizeof(buf), "Binding %s", *module_v);
-
-  Local<Array> modules = env->module_load_list_array();
-  uint32_t l = modules->Length();
-  modules->Set(l, OneByteString(env->isolate(), buf));
 
   node_module* mod = get_builtin_module(*module_v);
   Local<Object> exports;
@@ -2535,7 +2507,6 @@ static void Binding(const FunctionCallbackInfo<Value>& args) {
   } else {
     return ThrowIfNoSuchModule(env, *module_v);
   }
-  cache->Set(module, exports);
 
   args.GetReturnValue().Set(exports);
 }
@@ -2543,27 +2514,14 @@ static void Binding(const FunctionCallbackInfo<Value>& args) {
 static void InternalBinding(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  Local<String> module;
-  if (!args[0]->ToString(env->context()).ToLocal(&module)) return;
-
-  Local<Object> cache = env->internal_binding_cache_object();
-
-  if (PullFromCache(env, args, module, cache))
-    return;
+  CHECK(args[0]->IsString());
 
-  // Append a string to process.moduleLoadList
-  char buf[1024];
+  Local<String> module = args[0].As<String>();
   node::Utf8Value module_v(env->isolate(), module);
-  snprintf(buf, sizeof(buf), "Internal Binding %s", *module_v);
-
-  Local<Array> modules = env->module_load_list_array();
-  uint32_t l = modules->Length();
-  modules->Set(l, OneByteString(env->isolate(), buf));
 
   node_module* mod = get_internal_module(*module_v);
   if (mod == nullptr) return ThrowIfNoSuchModule(env, *module_v);
   Local<Object> exports = InitModule(env, mod, module);
-  cache->Set(module, exports);
 
   args.GetReturnValue().Set(exports);
 }
@@ -2571,14 +2529,9 @@ static void InternalBinding(const FunctionCallbackInfo<Value>& args) {
 static void LinkedBinding(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args.GetIsolate());
 
-  Local<String> module_name;
-  if (!args[0]->ToString(env->context()).ToLocal(&module_name)) return;
-
-  Local<Object> cache = env->binding_cache_object();
-  Local<Value> exports_v = cache->Get(module_name);
+  CHECK(args[0]->IsString());
 
-  if (exports_v->IsObject())
-    return args.GetReturnValue().Set(exports_v.As<Object>());
+  Local<String> module_name = args[0].As<String>();
 
   node::Utf8Value module_name_v(env->isolate(), module_name);
   node_module* mod = get_linked_module(*module_name_v);
@@ -2609,7 +2562,6 @@ static void LinkedBinding(const FunctionCallbackInfo<Value>& args) {
   }
 
   auto effective_exports = module->Get(exports_prop);
-  cache->Set(module_name, effective_exports);
 
   args.GetReturnValue().Set(effective_exports);
 }
@@ -2937,11 +2889,6 @@ void SetupProcessObject(Environment* env,
                     "version",
                     FIXED_ONE_BYTE_STRING(env->isolate(), NODE_VERSION));
 
-  // process.moduleLoadList
-  READONLY_PROPERTY(process,
-                    "moduleLoadList",
-                    env->module_load_list_array());
-
   // process.versions
   Local<Object> versions = Object::New(env->isolate());
   READONLY_PROPERTY(process, "versions", versions);