From bb27bfe1e1697df5f0bc35289ecc39463a02a63d Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 24 Jan 2018 23:25:20 -0500 Subject: [PATCH] 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 Reviewed-By: Ben Noordhuis Reviewed-By: Joyee Cheung Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel --- lib/internal/bootstrap_node.js | 53 ++++++++++++++++++++++++--- src/env-inl.h | 12 ------- src/env.h | 3 -- src/node.cc | 65 ++++------------------------------ 4 files changed, 55 insertions(+), 78 deletions(-) diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index b11f5901a1769c..f3f3264f353c24 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -21,9 +21,6 @@ setupProcessObject(); - internalBinding = process._internalBinding; - delete process._internalBinding; - // do this good and early, since it handles errors. setupProcessFatal(); @@ -246,6 +243,54 @@ perf.markMilestone(NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE); } + const moduleLoadList = []; + Object.defineProperty(process, 'moduleLoadList', { + value: moduleLoadList, + configurable: true, + enumerable: true, + writable: false + }); + + { + const bindingObj = Object.create(null); + + const getBinding = process.binding; + process.binding = function binding(module) { + module = String(module); + let mod = bindingObj[module]; + if (typeof mod !== 'object') { + mod = bindingObj[module] = getBinding(module); + moduleLoadList.push(`Binding ${module}`); + } + return mod; + }; + + const getLinkedBinding = process._linkedBinding; + process._linkedBinding = function _linkedBinding(module) { + module = String(module); + let mod = bindingObj[module]; + if (typeof mod !== 'object') + mod = bindingObj[module] = getLinkedBinding(module); + return mod; + }; + } + + { + const bindingObj = Object.create(null); + + const getInternalBinding = process._internalBinding; + delete process._internalBinding; + + internalBinding = function internalBinding(module) { + let mod = bindingObj[module]; + if (typeof mod !== 'object') { + mod = bindingObj[module] = getInternalBinding(module); + moduleLoadList.push(`Internal Binding ${module}`); + } + return mod; + }; + } + function setupProcessObject() { process._setupProcessObject(pushValueToArray); @@ -550,7 +595,7 @@ throw err; } - process.moduleLoadList.push(`NativeModule ${id}`); + moduleLoadList.push(`NativeModule ${id}`); const nativeModule = new NativeModule(id); diff --git a/src/env-inl.h b/src/env-inl.h index 202393f679e219..545a92ba17f47c 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 null = v8::Null(isolate()); - v8::Local binding_cache_object = v8::Object::New(isolate()); - CHECK(binding_cache_object->SetPrototype(context, null).FromJust()); - set_binding_cache_object(binding_cache_object); - - v8::Local 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 b24ada5740dd7b..d73be8156ecebf 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 c9af7c7dcd1efc..2e1d08f5b5d76d 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2460,22 +2460,6 @@ Maybe ProcessEmitDeprecationWarning(Environment* env, } -static bool PullFromCache(Environment* env, - const FunctionCallbackInfo& args, - Local module, - Local cache) { - Local context = env->context(); - Local exports_v; - Local 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 InitModule(Environment* env, node_module* mod, Local module) { @@ -2503,22 +2487,10 @@ static void ThrowIfNoSuchModule(Environment* env, const char* module_v) { static void Binding(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Local module; - if (!args[0]->ToString(env->context()).ToLocal(&module)) return; - - Local 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 module = args[0].As(); node::Utf8Value module_v(env->isolate(), module); - snprintf(buf, sizeof(buf), "Binding %s", *module_v); - - Local 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 exports; @@ -2535,7 +2507,6 @@ static void Binding(const FunctionCallbackInfo& args) { } else { return ThrowIfNoSuchModule(env, *module_v); } - cache->Set(module, exports); args.GetReturnValue().Set(exports); } @@ -2543,27 +2514,14 @@ static void Binding(const FunctionCallbackInfo& args) { static void InternalBinding(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Local module; - if (!args[0]->ToString(env->context()).ToLocal(&module)) return; - - Local 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 module = args[0].As(); node::Utf8Value module_v(env->isolate(), module); - snprintf(buf, sizeof(buf), "Internal Binding %s", *module_v); - - Local 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 exports = InitModule(env, mod, module); - cache->Set(module, exports); args.GetReturnValue().Set(exports); } @@ -2571,14 +2529,9 @@ static void InternalBinding(const FunctionCallbackInfo& args) { static void LinkedBinding(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args.GetIsolate()); - Local module_name; - if (!args[0]->ToString(env->context()).ToLocal(&module_name)) return; - - Local cache = env->binding_cache_object(); - Local exports_v = cache->Get(module_name); + CHECK(args[0]->IsString()); - if (exports_v->IsObject()) - return args.GetReturnValue().Set(exports_v.As()); + Local module_name = args[0].As(); 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& 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 versions = Object::New(env->isolate()); READONLY_PROPERTY(process, "versions", versions);