diff --git a/src/env-inl.h b/src/env-inl.h index acf48b07bfd7a4e..40cf3c901d368f4 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -436,14 +436,9 @@ inline std::vector* Environment::destroy_async_id_list() { return &destroy_async_id_list_; } -inline std::shared_ptr Environment::builtin_loader() { +inline builtins::BuiltinLoader* Environment::builtin_loader() { DCHECK(builtin_loader_); - return builtin_loader_; -} - -inline void Environment::set_builtin_loader( - std::shared_ptr loader) { - builtin_loader_ = loader; + return builtin_loader_.get(); } inline double Environment::new_async_id() { diff --git a/src/env.cc b/src/env.cc index 51672712ed75ec4..a449bcc7ae564fb 100644 --- a/src/env.cc +++ b/src/env.cc @@ -674,7 +674,8 @@ Environment::Environment(IsolateData* isolate_data, flags_(flags), thread_id_(thread_id.id == static_cast(-1) ? AllocateEnvironmentThreadId().id - : thread_id.id) { + : thread_id.id), + builtin_loader_(builtins::BuiltinLoader::Create()) { // We'll be creating new objects so make sure we've entered the context. HandleScope handle_scope(isolate); @@ -752,8 +753,6 @@ Environment::Environment(IsolateData* isolate_data, env_info, flags, thread_id) { - // TODO(addaleax): Make this part of CreateEnvironment(). - set_builtin_loader(builtins::BuiltinLoader::Create()); InitializeMainContext(context, env_info); } diff --git a/src/env.h b/src/env.h index 868d099ab528309..6c6c57a59eb91d5 100644 --- a/src/env.h +++ b/src/env.h @@ -721,8 +721,7 @@ class Environment : public MemoryRetainer { // List of id's that have been destroyed and need the destroy() cb called. inline std::vector* destroy_async_id_list(); - std::shared_ptr builtin_loader(); - void set_builtin_loader(std::shared_ptr loader); + builtins::BuiltinLoader* builtin_loader(); std::unordered_multimap hash_to_module_map; std::unordered_map id_to_module_map; @@ -1146,7 +1145,7 @@ class Environment : public MemoryRetainer { std::unique_ptr principal_realm_ = nullptr; - std::shared_ptr builtin_loader_; + std::unique_ptr builtin_loader_; // Used by allocate_managed_buffer() and release_managed_buffer() to keep // track of the BackingStore for a given pointer. diff --git a/src/node_builtins.cc b/src/node_builtins.cc index fe93902c1ab4943..ca395c2bdb38e72 100644 --- a/src/node_builtins.cc +++ b/src/node_builtins.cc @@ -3,6 +3,7 @@ #include "env-inl.h" #include "node_external_reference.h" #include "node_internals.h" +#include "node_threadsafe_cow-inl.h" #include "simdutf.h" #include "util-inl.h" @@ -53,24 +54,22 @@ BuiltinLoader::BuiltinLoader() : config_(GetConfig()), has_code_cache_(false) { } bool BuiltinLoader::Exists(const char* id) { - RwLock::ScopedReadLock lock(source_mutex_); - return source_.find(id) != source_.end(); + auto source = source_.read(); + return source->find(id) != source->end(); } bool BuiltinLoader::Add(const char* id, const UnionBytes& source) { - RwLock::ScopedLock source_lock(source_mutex_); - Mutex::ScopedLock categories_lock(builtin_categories_mutex_); builtin_categories_ .reset(); // The category cache is reset by adding new sources - auto result = source_.emplace(id, source); + auto result = source_.write()->emplace(id, source); return result.second; } Local BuiltinLoader::GetSourceObject(Local context) { - RwLock::ScopedReadLock lock(source_mutex_); Isolate* isolate = context->GetIsolate(); Local out = Object::New(isolate); - for (auto const& x : source_) { + auto source = source_.read(); + for (auto const& x : *source) { Local key = OneByteString(isolate, x.first.c_str(), x.first.size()); out->Set(context, key, x.second.ToStringChecked(isolate)).FromJust(); } @@ -82,10 +81,10 @@ Local BuiltinLoader::GetConfigString(Isolate* isolate) { } std::vector BuiltinLoader::GetBuiltinIds() const { - RwLock::ScopedReadLock lock(source_mutex_); std::vector ids; - ids.reserve(source_.size()); - for (auto const& x : source_) { + auto source = source_.read(); + ids.reserve(source->size()); + for (auto const& x : *source) { ids.emplace_back(x.first); } return ids; @@ -93,7 +92,6 @@ std::vector BuiltinLoader::GetBuiltinIds() const { const BuiltinLoader::BuiltinCategories& BuiltinLoader::InitializeBuiltinCategories() const { - Mutex::ScopedLock lock(builtin_categories_mutex_); if (LIKELY(builtin_categories_.has_value())) { DCHECK(!builtin_categories_.value().can_be_required.empty()); return builtin_categories_.value(); @@ -138,7 +136,8 @@ BuiltinLoader::InitializeBuiltinCategories() const { "internal/v8_prof_processor", }; - for (auto const& x : source_) { + auto source = source_.read(); + for (auto const& x : *source) { const std::string& id = x.first; for (auto const& prefix : prefixes) { if (prefix.length() > id.length()) { @@ -151,7 +150,7 @@ BuiltinLoader::InitializeBuiltinCategories() const { } } - for (auto const& x : source_) { + for (auto const& x : *source) { const std::string& id = x.first; if (0 == builtin_categories.cannot_be_required.count(id)) { builtin_categories.can_be_required.emplace(id); @@ -177,7 +176,8 @@ bool BuiltinLoader::CannotBeRequired(const char* id) const { return GetCannotBeRequired().count(id) == 1; } -ScriptCompiler::CachedData* BuiltinLoader::GetCodeCache(const char* id) const { +const ScriptCompiler::CachedData* BuiltinLoader::GetCodeCache( + const char* id) const { RwLock::ScopedReadLock lock(code_cache_mutex_); const auto it = code_cache_.find(id); if (it == code_cache_.end()) { @@ -206,12 +206,12 @@ static std::string OnDiskFileName(const char* id) { MaybeLocal BuiltinLoader::LoadBuiltinSource(Isolate* isolate, const char* id) const { + auto source = source_.read(); #ifdef NODE_BUILTIN_MODULES_PATH if (strncmp(id, "embedder_main_", strlen("embedder_main_")) == 0) { #endif // NODE_BUILTIN_MODULES_PATH - RwLock::ScopedReadLock lock(source_mutex_); - const auto source_it = source_.find(id); - if (UNLIKELY(source_it == source_.end())) { + const auto source_it = source->find(id); + if (UNLIKELY(source_it == source->end())) { fprintf(stderr, "Cannot find native builtin: \"%s\".\n", id); ABORT(); } @@ -438,7 +438,7 @@ MaybeLocal BuiltinLoader::LookupAndCompile(Local context, MaybeLocal maybe = LookupAndCompileInternal(context, id, ¶meters, &result); if (optional_realm != nullptr) { - DCHECK_EQ(this, optional_realm->env()->builtin_loader().get()); + DCHECK_EQ(this, optional_realm->env()->builtin_loader()); RecordResult(id, result, optional_realm); } return maybe; @@ -534,7 +534,7 @@ bool BuiltinLoader::CompileAllBuiltins(Local context) { return all_succeeded; } -void BuiltinLoader::CopyCodeCache(std::vector* out) { +void BuiltinLoader::CopyCodeCache(std::vector* out) const { RwLock::ScopedReadLock lock(code_cache_mutex_); for (auto const& item : code_cache_) { out->push_back( @@ -691,8 +691,19 @@ void BuiltinLoader::HasCachedBuiltins(const FunctionCallbackInfo& args) { v8::Boolean::New(args.GetIsolate(), instance->has_code_cache_)); } -std::shared_ptr BuiltinLoader::Create() { - return std::shared_ptr{new BuiltinLoader()}; +std::unique_ptr BuiltinLoader::Create() { + return std::unique_ptr{new BuiltinLoader()}; +} + +void BuiltinLoader::CopySourceAndCodeCacheFrom(const BuiltinLoader* other) { + { + std::vector cache; + other->CopyCodeCache(&cache); + RefreshCodeCache(cache); + has_code_cache_ = other->has_code_cache_; + } + + source_ = other->source_; } void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data, diff --git a/src/node_builtins.h b/src/node_builtins.h index 07aae5d697d78db..dad3c0c4d3ae064 100644 --- a/src/node_builtins.h +++ b/src/node_builtins.h @@ -11,6 +11,7 @@ #include #include #include "node_mutex.h" +#include "node_threadsafe_cow.h" #include "node_union_bytes.h" #include "v8.h" @@ -74,9 +75,10 @@ class NODE_EXTERN_PRIVATE BuiltinLoader { bool CompileAllBuiltins(v8::Local context); void RefreshCodeCache(const std::vector& in); - void CopyCodeCache(std::vector* out); + void CopyCodeCache(std::vector* out) const; - static std::shared_ptr Create(); + static std::unique_ptr Create(); + void CopySourceAndCodeCacheFrom(const BuiltinLoader* other); private: // Only allow access from friends. @@ -101,7 +103,7 @@ class NODE_EXTERN_PRIVATE BuiltinLoader { bool CanBeRequired(const char* id) const; bool CannotBeRequired(const char* id) const; - v8::ScriptCompiler::CachedData* GetCodeCache(const char* id) const; + const v8::ScriptCompiler::CachedData* GetCodeCache(const char* id) const; enum class Result { kWithCache, kWithoutCache }; v8::MaybeLocal LoadBuiltinSource(v8::Isolate* isolate, const char* id) const; @@ -135,19 +137,14 @@ class NODE_EXTERN_PRIVATE BuiltinLoader { void AddExternalizedBuiltin(const char* id, const char* filename); - // Used to synchronize access to builtin_categories. // builtin_categories is marked mutable because it is only initialized // as a cache, so that mutating it does not change any state. - Mutex builtin_categories_mutex_; mutable std::optional builtin_categories_; - // Used to synchronize access to the code cache map - RwLock source_mutex_; - BuiltinSourceMap source_; + ThreadsafeCopyOnWrite source_; const UnionBytes config_; - // Used to synchronize access to the code cache map RwLock code_cache_mutex_; BuiltinCodeCacheMap code_cache_; bool has_code_cache_; diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 5a0497ccf41c3b8..a40866b482c326c 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -159,11 +159,11 @@ NodeMainInstance::CreateMainEnvironment(ExitCode* exit_code) { &(snapshot_data_->env_info), EnvironmentFlags::kDefaultFlags, {})); +#ifdef NODE_V8_SHARED_RO_HEAP // TODO(addaleax): Do this as part of creating the Environment // once we store the SnapshotData* itself on IsolateData. - auto builtin_loader = builtins::BuiltinLoader::Create(); - builtin_loader->RefreshCodeCache(snapshot_data_->code_cache); - env->set_builtin_loader(builtin_loader); + env->builtin_loader()->RefreshCodeCache(snapshot_data_->code_cache); +#endif context = Context::FromSnapshot(isolate_, SnapshotData::kNodeMainContextIndex, {DeserializeNodeInternalFields, env.get()}) diff --git a/src/node_threadsafe_cow-inl.h b/src/node_threadsafe_cow-inl.h new file mode 100644 index 000000000000000..875b38766063f67 --- /dev/null +++ b/src/node_threadsafe_cow-inl.h @@ -0,0 +1,54 @@ +#ifndef SRC_NODE_THREADSAFE_COW_INL_H_ +#define SRC_NODE_THREADSAFE_COW_INL_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +namespace node { + +template +T* CopyOnWrite::write() { + if (!data_.unique()) { + data_ = std::make_shared(*data_); + } + return data_.get(); +} + +template +ThreadsafeCopyOnWrite::Read::Read(const ThreadsafeCopyOnWrite* cow) + : cow_(cow), lock_(cow->impl_->mutex) {} + +template +const T& ThreadsafeCopyOnWrite::Read::operator*() const { + return cow_->impl_->data; +} + +template +const T* ThreadsafeCopyOnWrite::Read::operator->() const { + return &cow_->impl_->data; +} + +template +ThreadsafeCopyOnWrite::Write::Write(ThreadsafeCopyOnWrite* cow) + : cow_(cow), impl_(cow->impl_.write()), lock_(impl_->mutex) {} + +template +T& ThreadsafeCopyOnWrite::Write::operator*() { + return impl_->data; +} + +template +T* ThreadsafeCopyOnWrite::Write::operator->() { + return &impl_->data; +} + +template +ThreadsafeCopyOnWrite::Impl::Impl(const Impl& other) { + RwLock::ScopedReadLock lock(other.mutex); + data = other.data; +} + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_NODE_THREADSAFE_COW_INL_H_ diff --git a/src/node_threadsafe_cow.h b/src/node_threadsafe_cow.h new file mode 100644 index 000000000000000..326e967adcd0590 --- /dev/null +++ b/src/node_threadsafe_cow.h @@ -0,0 +1,102 @@ +#ifndef SRC_NODE_THREADSAFE_COW_H_ +#define SRC_NODE_THREADSAFE_COW_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "util.h" +#include "uv.h" + +#include // std::shared_ptr +#include // std::forward + +namespace node { + +// Copy-on-write utility. Not threadsafe, i.e. there is no synchronization +// of the copy operation with other operations. +template +class CopyOnWrite final { + public: + template + explicit CopyOnWrite(Args&&... args) + : data_(std::make_shared(std::forward(args)...)) {} + + CopyOnWrite(const CopyOnWrite& other) = default; + CopyOnWrite& operator=(const CopyOnWrite& other) = default; + CopyOnWrite(CopyOnWrite&& other) = default; + CopyOnWrite& operator=(CopyOnWrite&& other) = default; + + const T* read() const { return data_.get(); } + T* write(); + + const T& operator*() const { return *read(); } + const T* operator->() const { return read(); } + + private: + std::shared_ptr data_; +}; + +// Threadsafe copy-on-write utility. Consumers need to use the Read and +// Write helpers to access the target data structure. +template +class ThreadsafeCopyOnWrite final { + public: + template + ThreadsafeCopyOnWrite(Args&&... args) + : impl_(T(std::forward(args)...)) {} + + ThreadsafeCopyOnWrite(const ThreadsafeCopyOnWrite& other) = default; + ThreadsafeCopyOnWrite& operator=(const ThreadsafeCopyOnWrite& other) = + default; + ThreadsafeCopyOnWrite(ThreadsafeCopyOnWrite&& other) = default; + ThreadsafeCopyOnWrite& operator=(ThreadsafeCopyOnWrite&& other) = default; + + class Read { + public: + explicit Read(const ThreadsafeCopyOnWrite* cow); + + const T& operator*() const; + const T* operator->() const; + + private: + const ThreadsafeCopyOnWrite* cow_; + RwLock::ScopedReadLock lock_; + }; + + class Write { + public: + explicit Write(ThreadsafeCopyOnWrite* cow); + + T& operator*(); + T* operator->(); + + private: + ThreadsafeCopyOnWrite* cow_; + typename ThreadsafeCopyOnWrite::Impl* impl_; + RwLock::ScopedLock lock_; + }; + + Read read() const { return Read(this); } + Write write() { return Write(this); } + + private: + struct Impl { + Impl(const T& data) : data(data) {} + Impl(T&& data) : data(std::move(data)) {} + + Impl(const Impl& other); + Impl& operator=(const Impl& other) = delete; + Impl(Impl&& other) = delete; + Impl& operator=(Impl&& other) = delete; + + RwLock mutex; + T data; + }; + + CopyOnWrite impl_; +}; + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_NODE_THREADSAFE_COW_H_ diff --git a/src/node_worker.cc b/src/node_worker.cc index 4c3053c32fbeb89..fb30046337cd648 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -344,8 +344,11 @@ void Worker::Run() { static_cast(environment_flags_), thread_id_, std::move(inspector_parent_handle_))); +#ifdef NODE_V8_SHARED_RO_HEAP // TODO(addaleax): Adjust for the embedder API snapshot support changes - env_->set_builtin_loader(env()->builtin_loader()); + env_->builtin_loader()->CopySourceAndCodeCacheFrom( + env()->builtin_loader()); +#endif if (is_stopped()) return; CHECK_NOT_NULL(env_); env_->set_env_vars(std::move(env_vars_)); diff --git a/test/cctest/test_per_process.cc b/test/cctest/test_per_process.cc index 1cd7adba7bb90fc..93904f5c5b21207 100644 --- a/test/cctest/test_per_process.cc +++ b/test/cctest/test_per_process.cc @@ -11,7 +11,7 @@ using node::builtins::BuiltinSourceMap; class PerProcessTest : public ::testing::Test { protected: static const BuiltinSourceMap get_sources_for_test() { - return BuiltinLoader::Create()->source_; + return *BuiltinLoader::Create()->source_.read(); } }; diff --git a/tools/js2c.py b/tools/js2c.py index e295949a18508d7..b8b26efc882a6d6 100755 --- a/tools/js2c.py +++ b/tools/js2c.py @@ -57,8 +57,14 @@ def ReadFile(filename): {0} +namespace {{ +const ThreadsafeCopyOnWrite global_source_map {{ + BuiltinSourceMap{{ {1} }} +}}; +}} + void BuiltinLoader::LoadJavaScriptSource() {{ - {1} + source_ = global_source_map; }} UnionBytes BuiltinLoader::GetConfig() {{ @@ -82,7 +88,7 @@ def ReadFile(filename): }}; """ -INITIALIZER = 'source_.emplace("{0}", UnionBytes{{{1}, {2}}});' +INITIALIZER = '{{"{0}", UnionBytes{{{1}, {2}}} }},' CONFIG_GYPI_ID = 'config_raw'