Skip to content

Commit

Permalink
process: JS fast path for bindings
Browse files Browse the repository at this point in the history 
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: nodejs#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>
  • Loading branch information
@apapirovski @MayaLekova
apapirovski authored and MayaLekova committed May 8, 2018
1 parent e48f648 commit d791351
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 78 deletions.
53 changes: 49 additions & 4 deletions lib/internal/bootstrap_node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@

setupProcessObject();

internalBinding = process._internalBinding;
delete process._internalBinding;

// do this good and early, since it handles errors.
setupProcessFatal();

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -550,7 +595,7 @@
throw err;
}

process.moduleLoadList.push(`NativeModule ${id}`);
moduleLoadList.push(`NativeModule ${id}`);

const nativeModule = new NativeModule(id);

Expand Down
12 changes: 0 additions & 12 deletions src/env-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand All @@ -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) \
Expand Down
65 changes: 6 additions & 59 deletions src/node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -2535,50 +2507,31 @@ static void Binding(const FunctionCallbackInfo<Value>& args) {
} else {
return ThrowIfNoSuchModule(env, *module_v);
}
cache->Set(module, exports);

args.GetReturnValue().Set(exports);
}

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);
}

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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit d791351

Please sign in to comment.