From cc86b48a622aacaeda76c08775d103a23e3db149 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 22 Mar 2023 15:13:13 +0100 Subject: [PATCH 1/2] bootstrap: store internal loaders in C++ via a binding 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. --- lib/internal/bootstrap/loaders.js | 5 +++-- src/node_builtins.cc | 12 ++++++++++++ src/node_realm.cc | 32 ++----------------------------- src/node_realm.h | 1 - 4 files changed, 17 insertions(+), 33 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 50e8b8702de6f1..a0a48e6c451dd7 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -182,6 +182,7 @@ const loaderId = 'internal/bootstrap/loaders'; const { builtinIds, compileFunction, + setInternalLoaders, } = internalBinding('builtins'); const getOwn = (target, property, receiver) => { @@ -373,5 +374,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); diff --git a/src/node_builtins.cc b/src/node_builtins.cc index 8ec3e828650e44..c3f6d61f20efc9 100644 --- a/src/node_builtins.cc +++ b/src/node_builtins.cc @@ -647,6 +647,16 @@ void BuiltinLoader::HasCachedBuiltins(const FunctionCallbackInfo& args) { args.GetIsolate(), instance->code_cache_->has_code_cache)); } +void SetInternalLoaders(const FunctionCallbackInfo& 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()); + realm->set_builtin_module_require(args[1].As()); +} + void BuiltinLoader::CopySourceAndCodeCacheReferenceFrom( const BuiltinLoader* other) { code_cache_ = other->code_cache_; @@ -685,6 +695,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 target, @@ -703,6 +714,7 @@ void BuiltinLoader::RegisterExternalReferences( registry->Register(GetCacheUsage); registry->Register(CompileFunction); registry->Register(HasCachedBuiltins); + registry->Register(SetInternalLoaders); } } // namespace builtins diff --git a/src/node_realm.cc b/src/node_realm.cc index a8cd9b9a55da2f..efecbae65482e0 100644 --- a/src/node_realm.cc +++ b/src/node_realm.cc @@ -174,42 +174,14 @@ MaybeLocal Realm::ExecuteBootstrapper(const char* id) { return scope.EscapeMaybe(result); } -MaybeLocal Realm::BootstrapInternalLoaders() { - EscapableHandleScope scope(isolate_); - - // Bootstrap internal loaders - Local loader_exports; - if (!ExecuteBootstrapper("internal/bootstrap/loaders") - .ToLocal(&loader_exports)) { - return MaybeLocal(); - } - CHECK(loader_exports->IsObject()); - Local loader_exports_obj = loader_exports.As(); - Local 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()); - Local require = - loader_exports_obj->Get(context(), env_->require_string()) - .ToLocalChecked(); - CHECK(require->IsFunction()); - set_builtin_module_require(require.As()); - - return scope.Escape(loader_exports); -} - MaybeLocal Realm::RunBootstrapping() { EscapableHandleScope scope(isolate_); CHECK(!has_run_bootstrapping_code()); - if (BootstrapInternalLoaders().IsEmpty()) { - return MaybeLocal(); - } - Local result; - if (!BootstrapRealm().ToLocal(&result)) { + if (!ExecuteBootstrapper("internal/bootstrap/loaders").ToLocal(&result) || + !BootstrapRealm().ToLocal(&result)) { return MaybeLocal(); } diff --git a/src/node_realm.h b/src/node_realm.h index 73ab1718e002de..4bff386baff227 100644 --- a/src/node_realm.h +++ b/src/node_realm.h @@ -129,7 +129,6 @@ class Realm : public MemoryRetainer { protected: ~Realm(); - v8::MaybeLocal BootstrapInternalLoaders(); virtual v8::MaybeLocal BootstrapRealm() = 0; Environment* env_; From 668c1e54004b49b80c06d4ec1b8542f9e031b15f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 24 Mar 2023 21:04:55 +0100 Subject: [PATCH 2/2] fixup! bootstrap: store internal loaders in C++ via a binding --- src/node_realm.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node_realm.cc b/src/node_realm.cc index efecbae65482e0..6a0876fa6dc87c 100644 --- a/src/node_realm.cc +++ b/src/node_realm.cc @@ -10,7 +10,6 @@ namespace node { using v8::Context; using v8::EscapableHandleScope; -using v8::Function; using v8::HandleScope; using v8::Local; using v8::MaybeLocal;