diff --git a/src/node_builtins.cc b/src/node_builtins.cc index 9e9cf6948b2c17..fb4d53e0ce3ddb 100644 --- a/src/node_builtins.cc +++ b/src/node_builtins.cc @@ -154,17 +154,6 @@ BuiltinLoader::BuiltinCategories BuiltinLoader::GetBuiltinCategories() const { return builtin_categories; } -const ScriptCompiler::CachedData* BuiltinLoader::GetCodeCache( - const char* id) const { - RwLock::ScopedReadLock lock(code_cache_->mutex); - const auto it = code_cache_->map.find(id); - if (it == code_cache_->map.end()) { - // The module has not been compiled before. - return nullptr; - } - return it->second.get(); -} - #ifdef NODE_BUILTIN_MODULES_PATH static std::string OnDiskFileName(const char* id) { std::string filename = NODE_BUILTIN_MODULES_PATH; @@ -276,7 +265,7 @@ MaybeLocal BuiltinLoader::LookupAndCompileInternal( OneByteString(isolate, filename_s.c_str(), filename_s.size()); ScriptOrigin origin(isolate, filename, 0, 0, true); - ScriptCompiler::CachedData* cached_data = nullptr; + BuiltinCodeCacheData cached_data{}; { // Note: The lock here should not extend into the // `CompileFunction()` call below, because this function may recurse if @@ -286,16 +275,18 @@ MaybeLocal BuiltinLoader::LookupAndCompileInternal( auto cache_it = code_cache_->map.find(id); if (cache_it != code_cache_->map.end()) { // Transfer ownership to ScriptCompiler::Source later. - cached_data = cache_it->second.release(); - code_cache_->map.erase(cache_it); + cached_data = cache_it->second; } } - const bool has_cache = cached_data != nullptr; + const bool has_cache = cached_data.data != nullptr; ScriptCompiler::CompileOptions options = has_cache ? ScriptCompiler::kConsumeCodeCache : ScriptCompiler::kEagerCompile; - ScriptCompiler::Source script_source(source, origin, cached_data); + 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", @@ -342,14 +333,19 @@ MaybeLocal BuiltinLoader::LookupAndCompileInternal( : "is accepted"); } - // Generate new cache for next compilation - std::unique_ptr new_cached_data( - ScriptCompiler::CreateCodeCacheForFunction(fun)); - CHECK_NOT_NULL(new_cached_data); - - { - RwLock::ScopedLock lock(code_cache_->mutex); - code_cache_->map[id] = std::move(new_cached_data); + if (*result == Result::kWithoutCache) { + // We failed to accept this cache, maybe because it was rejected, maybe + // because it wasn't present. Either way, we'll attempt to replace this + // code cache info with a new one. + std::shared_ptr new_cached_data( + ScriptCompiler::CreateCodeCacheForFunction(fun)); + CHECK_NOT_NULL(new_cached_data); + + { + RwLock::ScopedLock lock(code_cache_->mutex); + code_cache_->map.insert_or_assign( + id, BuiltinCodeCacheData(std::move(new_cached_data))); + } } return scope.Escape(fun); @@ -499,9 +495,7 @@ bool BuiltinLoader::CompileAllBuiltins(Local context) { 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->data, item.second->data + item.second->length}}); + out->push_back({item.first, item.second}); } } @@ -510,12 +504,7 @@ void BuiltinLoader::RefreshCodeCache(const std::vector& in) { code_cache_->map.reserve(in.size()); DCHECK(code_cache_->map.empty()); for (auto const& item : in) { - auto result = code_cache_->map.emplace( - item.id, - std::make_unique( - item.data.data(), - item.data.size(), - v8::ScriptCompiler::CachedData::BufferNotOwned)); + auto result = code_cache_->map.emplace(item.id, item.data); USE(result.second); DCHECK(result.second); } diff --git a/src/node_builtins.h b/src/node_builtins.h index 2dd3ee8b8c9c4e..11af941780be90 100644 --- a/src/node_builtins.h +++ b/src/node_builtins.h @@ -26,20 +26,55 @@ class Realm; namespace builtins { +class BuiltinCodeCacheData { + public: + BuiltinCodeCacheData() : data(nullptr), length(0), owning_ptr(nullptr) {} + + explicit BuiltinCodeCacheData( + std::shared_ptr cached_data) + : data(cached_data->data), + length(cached_data->length), + owning_ptr(cached_data) {} + + explicit BuiltinCodeCacheData( + std::shared_ptr> cached_data) + : data(cached_data->data()), + length(cached_data->size()), + owning_ptr(cached_data) {} + + BuiltinCodeCacheData(const uint8_t* data, size_t length) + : data(data), length(length), owning_ptr(nullptr) {} + + const uint8_t* data; + size_t length; + + // Returns a v8::ScriptCompiler::CachedData corresponding to this + // BuiltinCodeCacheData. The lifetime of the returned + // v8::ScriptCompiler::CachedData must not outlive that of the data. + std::unique_ptr AsCachedData() { + return std::make_unique( + data, length, v8::ScriptCompiler::CachedData::BufferNotOwned); + } + + private: + // If not null, represents a pointer which owns data. Otherwise indicates + // that data has static lifetime. + std::shared_ptr owning_ptr; +}; + +struct CodeCacheInfo { + std::string id; + BuiltinCodeCacheData data; +}; + using BuiltinSourceMap = std::map; using BuiltinCodeCacheMap = - std::unordered_map>; + std::unordered_map; // Generated by tools/js2c.py as node_javascript.cc void RegisterExternalReferencesForInternalizedBuiltinCode( ExternalReferenceRegistry* registry); -struct CodeCacheInfo { - std::string id; - std::vector data; -}; - // Handles compilation and caching of built-in JavaScript modules and // bootstrap scripts, whose source are bundled into the binary as static data. class NODE_EXTERN_PRIVATE BuiltinLoader { diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index b656332f8c7e5b..0db70f5ce08640 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -53,7 +53,7 @@ const uint32_t SnapshotData::kMagic; std::ostream& operator<<(std::ostream& output, const builtins::CodeCacheInfo& info) { output << "\n"; + << ", length=" << info.data.length << ">\n"; return output; } @@ -207,7 +207,11 @@ template <> builtins::CodeCacheInfo SnapshotDeserializer::Read() { Debug("Read()\n"); - builtins::CodeCacheInfo result{ReadString(), ReadVector()}; + std::string id = ReadString(); + auto owning_ptr = + std::make_shared>(ReadVector()); + builtins::BuiltinCodeCacheData code_cache_data{std::move(owning_ptr)}; + builtins::CodeCacheInfo result{id, code_cache_data}; if (is_debug) { std::string str = ToStr(result); @@ -217,14 +221,16 @@ builtins::CodeCacheInfo SnapshotDeserializer::Read() { } template <> -size_t SnapshotSerializer::Write(const builtins::CodeCacheInfo& data) { +size_t SnapshotSerializer::Write(const builtins::CodeCacheInfo& info) { Debug("\nWrite() id = %s" - ", size=%d\n", - data.id.c_str(), - data.data.size()); + ", length=%d\n", + info.id.c_str(), + info.data.length); - size_t written_total = WriteString(data.id); - written_total += WriteVector(data.data); + size_t written_total = WriteString(info.id); + + written_total += WriteArithmetic(info.data.length); + written_total += WriteArithmetic(info.data.data, info.data.length); Debug("Write() wrote %d bytes\n", written_total); return written_total; @@ -734,7 +740,7 @@ static std::string FormatSize(size_t size) { static void WriteStaticCodeCacheData(std::ostream* ss, const builtins::CodeCacheInfo& info) { *ss << "static const uint8_t " << GetCodeCacheDefName(info.id) << "[] = {\n"; - WriteVector(ss, info.data.data(), info.data.size()); + WriteVector(ss, info.data.data, info.data.length); *ss << "};"; } @@ -742,7 +748,7 @@ static void WriteCodeCacheInitializer(std::ostream* ss, const std::string& id) { std::string def_name = GetCodeCacheDefName(id); *ss << " { \"" << id << "\",\n"; *ss << " {" << def_name << ",\n"; - *ss << " " << def_name << " + arraysize(" << def_name << "),\n"; + *ss << " arraysize(" << def_name << "),\n"; *ss << " }\n"; *ss << " },\n"; } @@ -954,7 +960,7 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out, } env->builtin_loader()->CopyCodeCache(&(out->code_cache)); for (const auto& item : out->code_cache) { - std::string size_str = FormatSize(item.data.size()); + std::string size_str = FormatSize(item.data.length); per_process::Debug(DebugCategory::MKSNAPSHOT, "Generated code cache for %d: %s\n", item.id.c_str(),