From 5d6ffea0490ca479201fd6521dfb58f72a2bfb6f Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Nov 2024 12:04:06 -0500 Subject: [PATCH] src: avoid copy by using std::views::keys --- src/node_builtins.cc | 41 +++++++++++++++-------------------------- src/node_builtins.h | 5 ++++- src/util-inl.h | 20 ++++++++++++++++++++ src/util.h | 7 +++++++ 4 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/node_builtins.cc b/src/node_builtins.cc index 9aaf5626fcfe4a..2d83df9c36eb5f 100644 --- a/src/node_builtins.cc +++ b/src/node_builtins.cc @@ -88,16 +88,6 @@ Local BuiltinLoader::GetConfigString(Isolate* isolate) { return config_.ToStringChecked(isolate); } -std::vector BuiltinLoader::GetBuiltinIds() const { - std::vector ids; - auto source = source_.read(); - ids.reserve(source->size()); - for (auto const& x : *source) { - ids.emplace_back(x.first); - } - return ids; -} - BuiltinLoader::BuiltinCategories BuiltinLoader::GetBuiltinCategories() const { BuiltinCategories builtin_categories; @@ -512,26 +502,25 @@ bool BuiltinLoader::CompileAllBuiltinsAndCopyCodeCache( Local context, const std::vector& eager_builtins, std::vector* out) { - std::vector ids = GetBuiltinIds(); + auto 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()); + constexpr std::string_view v8_tools_prefix = "internal/deps/v8/tools/"; + constexpr std::string_view primordial_prefix = "internal/per_context/"; + constexpr std::string_view bootstrap_prefix = "internal/bootstrap/"; + constexpr std::string_view 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) { + if (id.starts_with(v8_tools_prefix)) { // 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) { + if (id.starts_with(primordial_prefix) || id.starts_with(bootstrap_prefix) || + id.starts_with(main_prefix)) { to_eager_compile_.emplace(id); } @@ -551,8 +540,8 @@ bool BuiltinLoader::CompileAllBuiltinsAndCopyCodeCache( } RwLock::ScopedReadLock lock(code_cache_->mutex); - for (auto const& item : code_cache_->map) { - out->push_back({item.first, item.second}); + for (const auto& [id, data] : code_cache_->map) { + out->push_back({id, data}); } return all_succeeded; } @@ -561,8 +550,8 @@ void BuiltinLoader::RefreshCodeCache(const std::vector& in) { RwLock::ScopedLock lock(code_cache_->mutex); code_cache_->map.reserve(in.size()); DCHECK(code_cache_->map.empty()); - for (auto const& item : in) { - auto result = code_cache_->map.emplace(item.id, item.data); + for (auto const& [id, data] : in) { + auto result = code_cache_->map.emplace(id, data); USE(result.second); DCHECK(result.second); } @@ -662,7 +651,7 @@ void BuiltinLoader::BuiltinIdsGetter(Local property, Environment* env = Environment::GetCurrent(info); Isolate* isolate = env->isolate(); - std::vector ids = env->builtin_loader()->GetBuiltinIds(); + auto ids = env->builtin_loader()->GetBuiltinIds(); info.GetReturnValue().Set( ToV8Value(isolate->GetCurrentContext(), ids).ToLocalChecked()); } diff --git a/src/node_builtins.h b/src/node_builtins.h index 1cb85b9058d065..2fa2182fe77b40 100644 --- a/src/node_builtins.h +++ b/src/node_builtins.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -126,7 +127,9 @@ class NODE_EXTERN_PRIVATE BuiltinLoader { void CopySourceAndCodeCacheReferenceFrom(const BuiltinLoader* other); - std::vector GetBuiltinIds() const; + [[nodiscard]] auto GetBuiltinIds() const { + return std::views::keys(*source_.read()); + } void SetEagerCompile() { should_eager_compile_ = true; } diff --git a/src/util-inl.h b/src/util-inl.h index e078c9a11b2fac..cc69d235a625e9 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -27,6 +27,7 @@ #include #include #include +#include #include // NOLINT(build/c++11) #include "node_revert.h" #include "util.h" @@ -374,6 +375,25 @@ v8::MaybeLocal ToV8Value(v8::Local context, return set_js; } +template +v8::MaybeLocal ToV8Value(v8::Local context, + const std::ranges::elements_view& vec, + v8::Isolate* isolate) { + if (isolate == nullptr) isolate = context->GetIsolate(); + v8::EscapableHandleScope handle_scope(isolate); + + MaybeStackBuffer, 128> arr(vec.size()); + arr.SetLength(vec.size()); + auto it = vec.begin(); + for (size_t i = 0; i < vec.size(); ++i) { + if (!ToV8Value(context, *it, isolate).ToLocal(&arr[i])) + return v8::MaybeLocal(); + std::advance(it, 1); + } + + return handle_scope.Escape(v8::Array::New(isolate, arr.out(), arr.length())); +} + template v8::MaybeLocal ToV8Value(v8::Local context, const std::unordered_map& map, diff --git a/src/util.h b/src/util.h index b1f316eebc7199..dcf1952021353f 100644 --- a/src/util.h +++ b/src/util.h @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -730,6 +731,12 @@ inline v8::MaybeLocal ToV8Value(v8::Local context, const std::unordered_map& map, v8::Isolate* isolate = nullptr); +template +inline v8::MaybeLocal ToV8Value( + v8::Local context, + const std::ranges::elements_view& vec, + v8::Isolate* isolate = nullptr); + // These macros expects a `Isolate* isolate` and a `Local context` // to be in the scope. #define READONLY_PROPERTY(obj, name, value) \