From e262e31951253067e18029366bb6326977dc8243 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 5 Feb 2024 18:24:03 +0100 Subject: [PATCH] src: compile code eagerly in snapshot builder By default V8 only compiles the top-level function and skips code generation for inner functions - that would only be done when those inner functions are invoked. Since builtins are compiled as wrapped functions, most functions that look visually top-level are not actually included in the built-in code cache. For most of the builtins this is not too bad because usually only a subset of all builtin functions are needed by a particular application and including all their code in the binary would incur an unnecessary size overhead. But there is also a subset of more commonly used builtins and it would be better to include the inner functions in the built-in code cache because they are more universally used by most applications. This patch changes the compilation strategy to eager compilation (including inner functions) for the following scripts: 1. Primordials (internal/per_context/*), in all situations. 2. Bootstrap scripts (internal/bootstrap/*) and main scripts (internal/main/*), when being compiled for built-in code cache. 3. Any scripts loaded during built-in snapshot generation. With this patch the binary size increases by about 1.2MB (~1% increase) in return the startup of --version command of bundled CLIs and the core startup of processes are 2-4% faster and worker startup can be 10-12% faster. --- src/api/environment.cc | 3 +++ src/env.cc | 9 +++++++++ src/node_builtins.cc | 42 ++++++++++++++++++++++++++++++++-------- src/node_builtins.h | 26 ++++++++++++++++++++----- src/node_snapshotable.cc | 19 +++++++++++++----- 5 files changed, 81 insertions(+), 18 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 8d7ecad56175ff..29826aa2d79586 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -802,6 +802,9 @@ Maybe InitializePrimordials(Local context) { // relatively cheap and all the scripts that we may want to run at // startup are always present in it. thread_local builtins::BuiltinLoader builtin_loader; + // Primordials can always be just eagerly compiled. + builtin_loader.SetEagerCompile(); + for (const char** module = context_files; *module != nullptr; module++) { Local arguments[] = {exports, primordials}; if (builtin_loader diff --git a/src/env.cc b/src/env.cc index 424f400b8f0f8f..17a1a1130c8eac 100644 --- a/src/env.cc +++ b/src/env.cc @@ -827,6 +827,15 @@ Environment::Environment(IsolateData* isolate_data, } } + // We are supposed to call builtin_loader_.SetEagerCompile() in + // snapshot mode here because it's beneficial to compile built-ins + // loaded in the snapshot eagerly and include the code of inner functions + // that are likely to be used by user since they are part of the core + // startup. But this requires us to start the coverage collections + // before Environment/Context creation which is not currently possible. + // TODO(joyeecheung): refactor V8ProfilerConnection classes to parse + // JSON without v8 and lift this restriction. + // We'll be creating new objects so make sure we've entered the context. HandleScope handle_scope(isolate); diff --git a/src/node_builtins.cc b/src/node_builtins.cc index bafd8d4b8581f0..bbb63df7899d4b 100644 --- a/src/node_builtins.cc +++ b/src/node_builtins.cc @@ -286,15 +286,24 @@ MaybeLocal BuiltinLoader::LookupAndCompileInternal( ScriptCompiler::CompileOptions options = has_cache ? ScriptCompiler::kConsumeCodeCache : ScriptCompiler::kNoCompileOptions; + if (should_eager_compile_) { + options = ScriptCompiler::kEagerCompile; + } else if (!to_eager_compile_.empty()) { + if (to_eager_compile_.find(id) != to_eager_compile_.end()) { + options = ScriptCompiler::kEagerCompile; + } + } ScriptCompiler::Source script_source( source, origin, has_cache ? cached_data.AsCachedData().release() : nullptr); - per_process::Debug(DebugCategory::CODE_CACHE, - "Compiling %s %s code cache\n", - id, - has_cache ? "with" : "without"); + per_process::Debug( + DebugCategory::CODE_CACHE, + "Compiling %s %s code cache %s\n", + id, + has_cache ? "with" : "without", + options == ScriptCompiler::kEagerCompile ? "eagerly" : "lazily"); MaybeLocal maybe_fun = ScriptCompiler::CompileFunction(context, @@ -481,14 +490,33 @@ MaybeLocal BuiltinLoader::CompileAndCall(Local context, return fn->Call(context, undefined, argc, argv); } -bool BuiltinLoader::CompileAllBuiltins(Local context) { +bool BuiltinLoader::CompileAllBuiltinsAndCopyCodeCache( + Local context, + const std::vector& eager_builtins, + std::vector* out) { std::vector ids = GetBuiltinIds(); bool all_succeeded = true; std::string v8_tools_prefix = "internal/deps/v8/tools/"; + std::string primordial_prefix = "internal/per_context/"; + std::string bootstrap_prefix = "internal/bootstrap/"; + std::string main_prefix = "internal/main/"; + to_eager_compile_ = std::unordered_set(eager_builtins.begin(), + eager_builtins.end()); + for (const auto& id : ids) { if (id.compare(0, v8_tools_prefix.size(), v8_tools_prefix) == 0) { + // No need to generate code cache for v8 scripts. continue; } + + // Eagerly compile primordials/boostrap/main scripts during code cache + // generation. + if (id.compare(0, primordial_prefix.size(), primordial_prefix) == 0 || + id.compare(0, bootstrap_prefix.size(), bootstrap_prefix) == 0 || + id.compare(0, main_prefix.size(), main_prefix) == 0) { + to_eager_compile_.emplace(id); + } + v8::TryCatch bootstrapCatch(context->GetIsolate()); auto fn = LookupAndCompile(context, id.data(), nullptr); if (bootstrapCatch.HasCaught()) { @@ -503,14 +531,12 @@ bool BuiltinLoader::CompileAllBuiltins(Local context) { SaveCodeCache(id.data(), fn.ToLocalChecked()); } } - return all_succeeded; -} -void BuiltinLoader::CopyCodeCache(std::vector* out) const { RwLock::ScopedReadLock lock(code_cache_->mutex); for (auto const& item : code_cache_->map) { out->push_back({item.first, item.second}); } + return all_succeeded; } void BuiltinLoader::RefreshCodeCache(const std::vector& in) { diff --git a/src/node_builtins.h b/src/node_builtins.h index 9f2fbc1e539374..495803eebf73d0 100644 --- a/src/node_builtins.h +++ b/src/node_builtins.h @@ -7,8 +7,8 @@ #include #include #include -#include #include +#include #include #include "node_external_reference.h" #include "node_mutex.h" @@ -112,12 +112,18 @@ class NODE_EXTERN_PRIVATE BuiltinLoader { bool Exists(const char* id); bool Add(const char* id, const UnionBytes& source); - bool CompileAllBuiltins(v8::Local context); + bool CompileAllBuiltinsAndCopyCodeCache( + v8::Local context, + const std::vector& lazy_builtins, + std::vector* out); void RefreshCodeCache(const std::vector& in); - void CopyCodeCache(std::vector* out) const; void CopySourceAndCodeCacheReferenceFrom(const BuiltinLoader* other); + std::vector GetBuiltinIds() const; + + void SetEagerCompile() { should_eager_compile_ = true; } + private: // Only allow access from friends. friend class CodeCacheBuilder; @@ -126,8 +132,6 @@ class NODE_EXTERN_PRIVATE BuiltinLoader { void LoadJavaScriptSource(); // Loads data into source_ UnionBytes GetConfig(); // Return data for config.gypi - std::vector GetBuiltinIds() const; - struct BuiltinCategories { std::set can_be_required; std::set cannot_be_required; @@ -179,6 +183,17 @@ class NODE_EXTERN_PRIVATE BuiltinLoader { const UnionBytes config_; + // If any bulitins should be eagerly compiled i.e. with inner functions + // compiled too, either use should_eager_compile_ to compile all builtins + // eagerly, or use to_eager_compile_ to compile specific builtins eagerly. + // Currently we set should_eager_compile_ to true when building the snapshot, + // and use to_eager_compile_ to compile code cache that complements the + // snapshot, where builtins already loaded in the snapshot and a few extras + // are compiled eagerly. At runtime any additional compilation is done + // lazily (except primordials in no-snapshot mode). + bool should_eager_compile_ = false; + std::unordered_set to_eager_compile_; + struct BuiltinCodeCache { RwLock mutex; BuiltinCodeCacheMap map; @@ -188,6 +203,7 @@ class NODE_EXTERN_PRIVATE BuiltinLoader { friend class ::PerProcessTest; }; + } // namespace builtins } // namespace node diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index c59a4cdccb9c8a..dbc191a88e1f4a 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -1069,10 +1069,12 @@ ExitCode BuildCodeCacheFromSnapshot(SnapshotData* out, Context::Scope context_scope(context); builtins::BuiltinLoader builtin_loader; // Regenerate all the code cache. - if (!builtin_loader.CompileAllBuiltins(context)) { + if (!builtin_loader.CompileAllBuiltinsAndCopyCodeCache( + context, + out->env_info.principal_realm.builtins, + &(out->code_cache))) { return ExitCode::kGenericUserError; } - builtin_loader.CopyCodeCache(&(out->code_cache)); if (per_process::enabled_debug_list.enabled(DebugCategory::MKSNAPSHOT)) { for (const auto& item : out->code_cache) { std::string size_str = FormatSize(item.data.length); @@ -1098,6 +1100,9 @@ ExitCode SnapshotBuilder::Generate( } if (!WithoutCodeCache(snapshot_config)) { + per_process::Debug( + DebugCategory::CODE_CACHE, + "---\nGenerate code cache to complement snapshot\n---\n"); // Deserialize the snapshot to recompile code cache. We need to do this in // the second pass because V8 requires the code cache to be compiled with a // finalized read-only space. @@ -1176,10 +1181,14 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out, CHECK_EQ(index, SnapshotData::kNodeMainContextIndex); } - // Must be out of HandleScope + // Must be out of HandleScope. + // Clear the code in the snapshot because we can't compile them eagerly + // during snapshot generation yet. We'll rely on the accompanying eagerly + // compiled code cache instead. + // TODO(joyeecheung): use FunctionCodeHandling::kKeep when we can start + // profilers before Environment/Context creation. SnapshotCreator::FunctionCodeHandling handling = - WithoutCodeCache(*config) ? SnapshotCreator::FunctionCodeHandling::kClear - : SnapshotCreator::FunctionCodeHandling::kKeep; + SnapshotCreator::FunctionCodeHandling::kClear; out->v8_snapshot_blob_data = creator->CreateBlob(handling); // We must be able to rehash the blob when we restore it or otherwise