Skip to content

Commit

Permalink
bootstrap: store internal loaders in C++ via a binding
Browse files Browse the repository at this point in the history
Instead of returning the internal loaders from the bootstrap script,
we can simply call a binding to store them in C++. This eliminates
the need for specializing the handling of this script.

PR-URL: nodejs/node#47215
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
  • Loading branch information
sercher committed Apr 24, 2024
1 parent 1f18eab commit 4b01597
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 31 deletions.
5 changes: 3 additions & 2 deletions graal-nodejs/lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ const loaderId = 'internal/bootstrap/loaders';
const {
builtinIds,
compileFunction,
setInternalLoaders,
} = internalBinding('builtins');

const getOwn = (target, property, receiver) => {
Expand Down Expand Up @@ -378,5 +379,5 @@ function requireWithFallbackInDeps(request) {
return requireBuiltin(request);
}

// Pass the exports back to C++ land for C++ internals to use.
return loaderExports;
// Store the internal loaders in C++.
setInternalLoaders(internalBinding, requireBuiltin);
12 changes: 12 additions & 0 deletions graal-nodejs/src/node_builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,16 @@ void BuiltinLoader::HasCachedBuiltins(const FunctionCallbackInfo<Value>& args) {
args.GetIsolate(), instance->code_cache_->has_code_cache));
}

void SetInternalLoaders(const FunctionCallbackInfo<Value>& args) {
Realm* realm = Realm::GetCurrent(args);
CHECK(args[0]->IsFunction());
CHECK(args[1]->IsFunction());
DCHECK(realm->internal_binding_loader().IsEmpty());
DCHECK(realm->builtin_module_require().IsEmpty());
realm->set_internal_binding_loader(args[0].As<Function>());
realm->set_builtin_module_require(args[1].As<Function>());
}

void BuiltinLoader::CopySourceAndCodeCacheReferenceFrom(
const BuiltinLoader* other) {
code_cache_ = other->code_cache_;
Expand Down Expand Up @@ -704,6 +714,7 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
SetMethod(isolate, proto, "getCacheUsage", BuiltinLoader::GetCacheUsage);
SetMethod(isolate, proto, "compileFunction", BuiltinLoader::CompileFunction);
SetMethod(isolate, proto, "hasCachedBuiltins", HasCachedBuiltins);
SetMethod(isolate, proto, "setInternalLoaders", SetInternalLoaders);
}

void BuiltinLoader::CreatePerContextProperties(Local<Object> target,
Expand All @@ -722,6 +733,7 @@ void BuiltinLoader::RegisterExternalReferences(
registry->Register(GetCacheUsage);
registry->Register(CompileFunction);
registry->Register(HasCachedBuiltins);
registry->Register(SetInternalLoaders);
}

} // namespace builtins
Expand Down
30 changes: 2 additions & 28 deletions graal-nodejs/src/node_realm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace node {

using v8::Context;
using v8::EscapableHandleScope;
using v8::Function;
using v8::HandleScope;
using v8::Local;
using v8::MaybeLocal;
Expand Down Expand Up @@ -183,31 +182,6 @@ MaybeLocal<Value> Realm::ExecuteBootstrapper(const char* id) {
return scope.EscapeMaybe(result);
}

MaybeLocal<Value> Realm::BootstrapInternalLoaders() {
EscapableHandleScope scope(isolate_);

// Bootstrap internal loaders
Local<Value> loader_exports;
if (!ExecuteBootstrapper("internal/bootstrap/loaders")
.ToLocal(&loader_exports)) {
return MaybeLocal<Value>();
}
CHECK(loader_exports->IsObject());
Local<Object> loader_exports_obj = loader_exports.As<Object>();
Local<Value> internal_binding_loader =
loader_exports_obj->Get(context(), env_->internal_binding_string())
.ToLocalChecked();
CHECK(internal_binding_loader->IsFunction());
set_internal_binding_loader(internal_binding_loader.As<Function>());
Local<Value> require =
loader_exports_obj->Get(context(), env_->require_string())
.ToLocalChecked();
CHECK(require->IsFunction());
set_builtin_module_require(require.As<Function>());

return scope.Escape(loader_exports);
}

MaybeLocal<Value> Realm::BootstrapNode() {
EscapableHandleScope scope(isolate_);

Expand Down Expand Up @@ -261,11 +235,11 @@ MaybeLocal<Value> Realm::RunBootstrapping() {

CHECK(!has_run_bootstrapping_code());

if (BootstrapInternalLoaders().IsEmpty()) {
Local<Value> result;
if (!ExecuteBootstrapper("internal/bootstrap/loaders").ToLocal(&result)) {
return MaybeLocal<Value>();
}

Local<Value> result;
if (!BootstrapNode().ToLocal(&result)) {
return MaybeLocal<Value>();
}
Expand Down
1 change: 0 additions & 1 deletion graal-nodejs/src/node_realm.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class Realm : public MemoryRetainer {
void DeserializeProperties(const RealmSerializeInfo* info);

v8::MaybeLocal<v8::Value> ExecuteBootstrapper(const char* id);
v8::MaybeLocal<v8::Value> BootstrapInternalLoaders();
v8::MaybeLocal<v8::Value> BootstrapNode();
v8::MaybeLocal<v8::Value> RunBootstrapping();

Expand Down

0 comments on commit 4b01597

Please sign in to comment.