From 11982aecd4676352bc8ea47a745c96f47358c4d0 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 f14eed2ace2a63..a6ac7b66afbc0b 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(); @@ -245,6 +242,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); @@ -542,7 +587,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 c83915295b981c..7cdd9cecd378b9 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -298,18 +298,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 dc664692e2620a..748c15f8af78f1 100644 --- a/src/env.h +++ b/src/env.h @@ -281,15 +281,12 @@ 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(http2ping_constructor_template, v8::ObjectTemplate) \ V(http2stream_constructor_template, v8::ObjectTemplate) \ V(http2settings_constructor_template, v8::ObjectTemplate) \ 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 980272842a3439..cc2bdefbb41f66 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2835,22 +2835,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) { @@ -2878,22 +2862,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; @@ -2910,7 +2882,6 @@ static void Binding(const FunctionCallbackInfo& args) { } else { return ThrowIfNoSuchModule(env, *module_v); } - cache->Set(module, exports); args.GetReturnValue().Set(exports); } @@ -2918,27 +2889,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); } @@ -2946,14 +2904,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); @@ -2984,7 +2937,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); } @@ -3319,11 +3271,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);