Skip to content

Commit bb27bfe

Browse files
apapirovskiapapirovski
committed
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: #18365 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
1 parent eeede3b commit bb27bfe

File tree

4 files changed

+55
-78
lines changed

4 files changed

+55
-78
lines changed

‎lib/internal/bootstrap_node.js

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@
2121

2222
setupProcessObject();
2323

24-
internalBinding = process._internalBinding;
25-
delete process._internalBinding;
26-
2724
// do this good and early, since it handles errors.
2825
setupProcessFatal();
2926

@@ -246,6 +243,54 @@
246243
perf.markMilestone(NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
247244
}
248245

246+
const moduleLoadList = [];
247+
Object.defineProperty(process, 'moduleLoadList', {
248+
value: moduleLoadList,
249+
configurable: true,
250+
enumerable: true,
251+
writable: false
252+
});
253+
254+
{
255+
const bindingObj = Object.create(null);
256+
257+
const getBinding = process.binding;
258+
process.binding = function binding(module) {
259+
module = String(module);
260+
let mod = bindingObj[module];
261+
if (typeof mod !== 'object') {
262+
mod = bindingObj[module] = getBinding(module);
263+
moduleLoadList.push(`Binding ${module}`);
264+
}
265+
return mod;
266+
};
267+
268+
const getLinkedBinding = process._linkedBinding;
269+
process._linkedBinding = function _linkedBinding(module) {
270+
module = String(module);
271+
let mod = bindingObj[module];
272+
if (typeof mod !== 'object')
273+
mod = bindingObj[module] = getLinkedBinding(module);
274+
return mod;
275+
};
276+
}
277+
278+
{
279+
const bindingObj = Object.create(null);
280+
281+
const getInternalBinding = process._internalBinding;
282+
delete process._internalBinding;
283+
284+
internalBinding = function internalBinding(module) {
285+
let mod = bindingObj[module];
286+
if (typeof mod !== 'object') {
287+
mod = bindingObj[module] = getInternalBinding(module);
288+
moduleLoadList.push(`Internal Binding ${module}`);
289+
}
290+
return mod;
291+
};
292+
}
293+
249294
function setupProcessObject() {
250295
process._setupProcessObject(pushValueToArray);
251296

@@ -550,7 +595,7 @@
550595
throw err;
551596
}
552597

553-
process.moduleLoadList.push(`NativeModule ${id}`);
598+
moduleLoadList.push(`NativeModule ${id}`);
554599

555600
const nativeModule = new NativeModule(id);
556601

‎src/env-inl.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -328,18 +328,6 @@ inline Environment::Environment(IsolateData* isolate_data,
328328
v8::Context::Scope context_scope(context);
329329
set_as_external(v8::External::New(isolate(), this));
330330

331-
v8::Local<v8::Primitive> null = v8::Null(isolate());
332-
v8::Local<v8::Object> binding_cache_object = v8::Object::New(isolate());
333-
CHECK(binding_cache_object->SetPrototype(context, null).FromJust());
334-
set_binding_cache_object(binding_cache_object);
335-
336-
v8::Local<v8::Object> internal_binding_cache_object =
337-
v8::Object::New(isolate());
338-
CHECK(internal_binding_cache_object->SetPrototype(context, null).FromJust());
339-
set_internal_binding_cache_object(internal_binding_cache_object);
340-
341-
set_module_load_list_array(v8::Array::New(isolate()));
342-
343331
AssignToContext(context, ContextInfo(""));
344332

345333
destroy_async_id_list_.reserve(512);

‎src/env.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,6 @@ class ModuleWrap;
274274
V(async_hooks_after_function, v8::Function) \
275275
V(async_hooks_promise_resolve_function, v8::Function) \
276276
V(async_hooks_binding, v8::Object) \
277-
V(binding_cache_object, v8::Object) \
278-
V(internal_binding_cache_object, v8::Object) \
279277
V(buffer_prototype_object, v8::Object) \
280278
V(context, v8::Context) \
281279
V(domain_callback, v8::Function) \
@@ -285,7 +283,6 @@ class ModuleWrap;
285283
V(http2settings_constructor_template, v8::ObjectTemplate) \
286284
V(immediate_callback_function, v8::Function) \
287285
V(inspector_console_api_object, v8::Object) \
288-
V(module_load_list_array, v8::Array) \
289286
V(pbkdf2_constructor_template, v8::ObjectTemplate) \
290287
V(pipe_constructor_template, v8::FunctionTemplate) \
291288
V(performance_entry_callback, v8::Function) \

‎src/node.cc

Lines changed: 6 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2460,22 +2460,6 @@ Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
24602460
}
24612461

24622462

2463-
static bool PullFromCache(Environment* env,
2464-
const FunctionCallbackInfo<Value>& args,
2465-
Local<String> module,
2466-
Local<Object> cache) {
2467-
Local<Context> context = env->context();
2468-
Local<Value> exports_v;
2469-
Local<Object> exports;
2470-
if (cache->Get(context, module).ToLocal(&exports_v) &&
2471-
exports_v->IsObject() &&
2472-
exports_v->ToObject(context).ToLocal(&exports)) {
2473-
args.GetReturnValue().Set(exports);
2474-
return true;
2475-
}
2476-
return false;
2477-
}
2478-
24792463
static Local<Object> InitModule(Environment* env,
24802464
node_module* mod,
24812465
Local<String> module) {
@@ -2503,22 +2487,10 @@ static void ThrowIfNoSuchModule(Environment* env, const char* module_v) {
25032487
static void Binding(const FunctionCallbackInfo<Value>& args) {
25042488
Environment* env = Environment::GetCurrent(args);
25052489

2506-
Local<String> module;
2507-
if (!args[0]->ToString(env->context()).ToLocal(&module)) return;
2508-
2509-
Local<Object> cache = env->binding_cache_object();
2510-
2511-
if (PullFromCache(env, args, module, cache))
2512-
return;
2490+
CHECK(args[0]->IsString());
25132491

2514-
// Append a string to process.moduleLoadList
2515-
char buf[1024];
2492+
Local<String> module = args[0].As<String>();
25162493
node::Utf8Value module_v(env->isolate(), module);
2517-
snprintf(buf, sizeof(buf), "Binding %s", *module_v);
2518-
2519-
Local<Array> modules = env->module_load_list_array();
2520-
uint32_t l = modules->Length();
2521-
modules->Set(l, OneByteString(env->isolate(), buf));
25222494

25232495
node_module* mod = get_builtin_module(*module_v);
25242496
Local<Object> exports;
@@ -2535,50 +2507,31 @@ static void Binding(const FunctionCallbackInfo<Value>& args) {
25352507
} else {
25362508
return ThrowIfNoSuchModule(env, *module_v);
25372509
}
2538-
cache->Set(module, exports);
25392510

25402511
args.GetReturnValue().Set(exports);
25412512
}
25422513

25432514
static void InternalBinding(const FunctionCallbackInfo<Value>& args) {
25442515
Environment* env = Environment::GetCurrent(args);
25452516

2546-
Local<String> module;
2547-
if (!args[0]->ToString(env->context()).ToLocal(&module)) return;
2548-
2549-
Local<Object> cache = env->internal_binding_cache_object();
2550-
2551-
if (PullFromCache(env, args, module, cache))
2552-
return;
2517+
CHECK(args[0]->IsString());
25532518

2554-
// Append a string to process.moduleLoadList
2555-
char buf[1024];
2519+
Local<String> module = args[0].As<String>();
25562520
node::Utf8Value module_v(env->isolate(), module);
2557-
snprintf(buf, sizeof(buf), "Internal Binding %s", *module_v);
2558-
2559-
Local<Array> modules = env->module_load_list_array();
2560-
uint32_t l = modules->Length();
2561-
modules->Set(l, OneByteString(env->isolate(), buf));
25622521

25632522
node_module* mod = get_internal_module(*module_v);
25642523
if (mod == nullptr) return ThrowIfNoSuchModule(env, *module_v);
25652524
Local<Object> exports = InitModule(env, mod, module);
2566-
cache->Set(module, exports);
25672525

25682526
args.GetReturnValue().Set(exports);
25692527
}
25702528

25712529
static void LinkedBinding(const FunctionCallbackInfo<Value>& args) {
25722530
Environment* env = Environment::GetCurrent(args.GetIsolate());
25732531

2574-
Local<String> module_name;
2575-
if (!args[0]->ToString(env->context()).ToLocal(&module_name)) return;
2576-
2577-
Local<Object> cache = env->binding_cache_object();
2578-
Local<Value> exports_v = cache->Get(module_name);
2532+
CHECK(args[0]->IsString());
25792533

2580-
if (exports_v->IsObject())
2581-
return args.GetReturnValue().Set(exports_v.As<Object>());
2534+
Local<String> module_name = args[0].As<String>();
25822535

25832536
node::Utf8Value module_name_v(env->isolate(), module_name);
25842537
node_module* mod = get_linked_module(*module_name_v);
@@ -2609,7 +2562,6 @@ static void LinkedBinding(const FunctionCallbackInfo<Value>& args) {
26092562
}
26102563

26112564
auto effective_exports = module->Get(exports_prop);
2612-
cache->Set(module_name, effective_exports);
26132565

26142566
args.GetReturnValue().Set(effective_exports);
26152567
}
@@ -2937,11 +2889,6 @@ void SetupProcessObject(Environment* env,
29372889
"version",
29382890
FIXED_ONE_BYTE_STRING(env->isolate(), NODE_VERSION));
29392891

2940-
// process.moduleLoadList
2941-
READONLY_PROPERTY(process,
2942-
"moduleLoadList",
2943-
env->module_load_list_array());
2944-
29452892
// process.versions
29462893
Local<Object> versions = Object::New(env->isolate());
29472894
READONLY_PROPERTY(process, "versions", versions);

0 commit comments

Comments
 (0)