Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: lazily initialize global BuiltinLoader #46256

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/node_builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ using v8::String;
using v8::Undefined;
using v8::Value;

BuiltinLoader BuiltinLoader::instance_;

BuiltinLoader::BuiltinLoader() : config_(GetConfig()), has_code_cache_(false) {
LoadJavaScriptSource();
#ifdef NODE_SHARED_BUILTIN_CJS_MODULE_LEXER_LEXER_PATH
Expand All @@ -55,6 +53,7 @@ BuiltinLoader::BuiltinLoader() : config_(GetConfig()), has_code_cache_(false) {
}

BuiltinLoader* BuiltinLoader::GetInstance() {
static BuiltinLoader instance_;
return &instance_;
}

Expand All @@ -63,8 +62,11 @@ bool BuiltinLoader::Exists(const char* id) {
return source.find(id) != source.end();
}

bool BuiltinLoader::Add(const char* id, const UnionBytes& source) {
auto result = GetInstance()->source_.emplace(id, source);
bool BuiltinLoader::Add(const char* id,
const UnionBytes& source,
BuiltinLoader* instance) {
if (instance == nullptr) instance = GetInstance();
auto result = instance->source_.emplace(id, source);
return result.second;
}

Expand Down Expand Up @@ -249,10 +251,12 @@ void BuiltinLoader::AddExternalizedBuiltin(const char* id,
ABORT();
}

Add(id, source);
Add(id, source, this);
}

bool BuiltinLoader::Add(const char* id, std::string_view utf8source) {
bool BuiltinLoader::Add(const char* id,
std::string_view utf8source,
BuiltinLoader* instance) {
size_t expected_u16_length =
simdutf::utf16_length_from_utf8(utf8source.data(), utf8source.length());
auto out = std::make_shared<std::vector<uint16_t>>(expected_u16_length);
Expand All @@ -261,7 +265,7 @@ bool BuiltinLoader::Add(const char* id, std::string_view utf8source) {
utf8source.length(),
reinterpret_cast<char16_t*>(out->data()));
out->resize(u16_length);
return Add(id, UnionBytes(out));
return Add(id, UnionBytes(out), instance);
}

// Returns Local<Function> of the compiled module if return_code_cache
Expand Down
13 changes: 9 additions & 4 deletions src/node_builtins.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,14 @@ class NODE_EXTERN_PRIVATE BuiltinLoader {
// Returns config.gypi as a JSON string
static v8::Local<v8::String> GetConfigString(v8::Isolate* isolate);
static bool Exists(const char* id);
static bool Add(const char* id, const UnionBytes& source);
static bool Add(const char* id, std::string_view utf8source);
// TODO(addaleax): Make BuiltinLoader non-global so that these
// methods do not need to be static.
static bool Add(const char* id,
const UnionBytes& source,
BuiltinLoader* instance = nullptr);
static bool Add(const char* id,
std::string_view utf8source,
BuiltinLoader* instance = nullptr);

static bool CompileAllBuiltins(v8::Local<v8::Context> context);
static void RefreshCodeCache(const std::vector<CodeCacheInfo>& in);
Expand Down Expand Up @@ -134,9 +140,8 @@ class NODE_EXTERN_PRIVATE BuiltinLoader {
static void HasCachedBuiltins(
const v8::FunctionCallbackInfo<v8::Value>& args);

static void AddExternalizedBuiltin(const char* id, const char* filename);
void AddExternalizedBuiltin(const char* id, const char* filename);

static BuiltinLoader instance_;
BuiltinCategories builtin_categories_;
BuiltinSourceMap source_;
BuiltinCodeCacheMap code_cache_;
Expand Down
2 changes: 1 addition & 1 deletion test/cctest/test_per_process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ using node::builtins::BuiltinSourceMap;
class PerProcessTest : public ::testing::Test {
protected:
static const BuiltinSourceMap get_sources_for_test() {
return BuiltinLoader::instance_.source_;
return BuiltinLoader::GetInstance()->source_;
}
};

Expand Down