From c841b77b613d2974df4103c14b9372d260f27d91 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 12 Feb 2023 00:44:48 +0100 Subject: [PATCH] src: use an array for faster binding data lookup Locally the hashing of the binding names sometimes has significant presence in the profile of bindings, because there can be collisions, which makes the cost of adding a new binding data non-trivial, but it's wasteful to spend time on hashing them or dealing with collisions at all, since we can just use the EmbedderObjectType enum as the key, as the string names are not actually used beyond debugging purposes and can be easily matched with a macro. And since we can just use the enum as the key, we do not even need the map and can just use an array with the enum as indices for the lookup. --- src/README.md | 26 +++++++++++++++----------- src/env-inl.h | 14 +++++++++----- src/env.cc | 4 +++- src/env.h | 8 ++++---- src/node_blob.h | 1 - src/node_file.h | 1 - src/node_http2_state.h | 3 ++- src/node_http_parser.cc | 3 ++- src/node_process.h | 1 - src/node_snapshotable.cc | 8 ++++---- src/node_snapshotable.h | 9 +++++++-- src/node_util.h | 1 - src/node_v8.h | 1 - 13 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/README.md b/src/README.md index 1b2b8a951d8852f..f8b2dfd871cd781 100644 --- a/src/README.md +++ b/src/README.md @@ -486,18 +486,28 @@ that state is through the use of `Environment::AddBindingData`, which gives binding functions access to an object for storing such state. That object is always a [`BaseObject`][]. -Its class needs to have a static `type_name` field based on a -constant string, in order to disambiguate it from other classes of this type, -and which could e.g. match the binding's name (in the example above, that would -be `cares_wrap`). +If the binding should be supported in a snapshot, it needs to be in the +`SERIALIZABLE_OBJECT_TYPES` list in `node_snapshotable.h` and implement the +serialization and deserialization methods. See the comments of +`SnapshotableObject` on how to implement them. Otherwise, the `type_int` field +needs to be added to the `UNSERIALIZABLE_OBJECT_TYPES` list. ```cpp +// In node_snapshotable.h, add the binding to either UNSERIALIZABLE_OBJECT_TYPES +// or SERIALIZABLE_OBJECT_TYPES. The second parameter is a descriptive name +// of the class, which is usually the class name with the (actual or conceptual) +// namespace. + +#define UNSERIALIZABLE_OBJECT_TYPES(V) \ + V(http_parser_binding_data, http_parser::BindingData) + // In the HTTP parser source code file: class BindingData : public BaseObject { public: BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} - static constexpr FastStringKey type_name { "http_parser" }; + static constexpr EmbedderObjectType type_int = + EmbedderObjectType::k_http_parser_binding_data; std::vector parser_buffer; bool parser_buffer_in_use = false; @@ -527,12 +537,6 @@ void InitializeHttpParser(Local target, } ``` -If the binding is loaded during bootstrap, add it to the -`SERIALIZABLE_OBJECT_TYPES` list in `src/node_snapshotable.h` and -inherit from the `SnapshotableObject` class instead. See the comments -of `SnapshotableObject` on how to implement its serialization and -deserialization. - ### Exception handling diff --git a/src/env-inl.h b/src/env-inl.h index 0ca495bd58df6f5..ab0f1ed9b8c2447 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -218,9 +218,11 @@ inline T* Environment::GetBindingData(v8::Local context) { context->GetAlignedPointerFromEmbedderData( ContextEmbedderIndex::kBindingListIndex)); DCHECK_NOT_NULL(map); - auto it = map->find(T::type_name); - if (UNLIKELY(it == map->end())) return nullptr; - T* result = static_cast(it->second.get()); + constexpr size_t binding_index = static_cast(T::type_int); + static_assert(binding_index < std::tuple_size_v); + auto ptr = (*map)[binding_index]; + if (UNLIKELY(!ptr)) return nullptr; + T* result = static_cast(ptr.get()); DCHECK_NOT_NULL(result); DCHECK_EQ(result->env(), GetCurrent(context)); return result; @@ -237,8 +239,10 @@ inline T* Environment::AddBindingData( context->GetAlignedPointerFromEmbedderData( ContextEmbedderIndex::kBindingListIndex)); DCHECK_NOT_NULL(map); - auto result = map->emplace(T::type_name, item); - CHECK(result.second); + constexpr size_t binding_index = static_cast(T::type_int); + static_assert(binding_index < std::tuple_size_v); + CHECK(!(*map)[binding_index]); // Should not insert the binding twice. + (*map)[binding_index] = item; DCHECK_EQ(GetBindingData(context), item.get()); return item.get(); } diff --git a/src/env.cc b/src/env.cc index 692a344703a196d..f85b0177aace33d 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1018,7 +1018,9 @@ MaybeLocal Environment::RunSnapshotDeserializeMain() const { void Environment::RunCleanup() { started_cleanup_ = true; TRACE_EVENT0(TRACING_CATEGORY_NODE1(environment), "RunCleanup"); - bindings_.clear(); + for (size_t i = 0; i < bindings_.size(); ++i) { + bindings_[i].reset(); + } // Only BaseObject's cleanups are registered as per-realm cleanup hooks now. // Defer the BaseObject cleanup after handles are cleaned up. CleanupHandles(); diff --git a/src/env.h b/src/env.h index c2eb764e740400c..c4d4419ed821133 100644 --- a/src/env.h +++ b/src/env.h @@ -601,10 +601,10 @@ class Environment : public MemoryRetainer { template static inline T* GetBindingData(v8::Local context); - typedef std::unordered_map< - FastStringKey, - BaseObjectPtr, - FastStringKey::Hash> BindingDataStore; + typedef std::array, + static_cast( + EmbedderObjectType::kEmbedderObjectTypeCount)> + BindingDataStore; // Create an Environment without initializing a main Context. Use // InitializeMainContext() to initialize a main context for it. diff --git a/src/node_blob.h b/src/node_blob.h index f0340f313bde6f2..82a86eebac69f2e 100644 --- a/src/node_blob.h +++ b/src/node_blob.h @@ -147,7 +147,6 @@ class BlobBindingData : public SnapshotableObject { SERIALIZABLE_OBJECT_METHODS() - static constexpr FastStringKey type_name{"node::BlobBindingData"}; static constexpr EmbedderObjectType type_int = EmbedderObjectType::k_blob_binding_data; diff --git a/src/node_file.h b/src/node_file.h index 5554ba7de51a2ab..07224cb55122612 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -69,7 +69,6 @@ class BindingData : public SnapshotableObject { using InternalFieldInfo = InternalFieldInfoBase; SERIALIZABLE_OBJECT_METHODS() - static constexpr FastStringKey type_name{"node::fs::BindingData"}; static constexpr EmbedderObjectType type_int = EmbedderObjectType::k_fs_binding_data; diff --git a/src/node_http2_state.h b/src/node_http2_state.h index 7cf40ff1017ca33..1fd5d34d05fae4d 100644 --- a/src/node_http2_state.h +++ b/src/node_http2_state.h @@ -127,7 +127,8 @@ class Http2State : public BaseObject { SET_SELF_SIZE(Http2State) SET_MEMORY_INFO_NAME(Http2State) - static constexpr FastStringKey type_name { "http2" }; + static constexpr EmbedderObjectType type_int = + EmbedderObjectType::k_http2_binding_data; private: struct http2_state_internal { diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index bb2019c74457856..dcd627c66db48fc 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -96,7 +96,8 @@ class BindingData : public BaseObject { BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} - static constexpr FastStringKey type_name { "http_parser" }; + static constexpr EmbedderObjectType type_int = + EmbedderObjectType::k_http_parser_binding_data; std::vector parser_buffer; bool parser_buffer_in_use = false; diff --git a/src/node_process.h b/src/node_process.h index 8065378960887d2..00af553ed80e293 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -54,7 +54,6 @@ class BindingData : public SnapshotableObject { using InternalFieldInfo = InternalFieldInfoBase; SERIALIZABLE_OBJECT_METHODS() - static constexpr FastStringKey type_name{"node::process::BindingData"}; static constexpr EmbedderObjectType type_int = EmbedderObjectType::k_process_binding_data; diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index d4f45bf31019ea7..c1e4ae1f6d88991 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -1290,11 +1290,11 @@ SnapshotableObject::SnapshotableObject(Environment* env, : BaseObject(env, wrap), type_(type) { } -std::string_view SnapshotableObject::GetTypeName() const { +std::string SnapshotableObject::GetTypeName() const { switch (type_) { #define V(PropertyName, NativeTypeName) \ case EmbedderObjectType::k_##PropertyName: { \ - return NativeTypeName::type_name.as_string_view(); \ + return #NativeTypeName; \ } SERIALIZABLE_OBJECT_TYPES(V) #undef V @@ -1335,7 +1335,7 @@ void DeserializeNodeInternalFields(Local holder, per_process::Debug(DebugCategory::MKSNAPSHOT, \ "Object %p is %s\n", \ (*holder), \ - NativeTypeName::type_name.as_string_view()); \ + #NativeTypeName); \ env_ptr->EnqueueDeserializeRequest( \ NativeTypeName::Deserialize, \ holder, \ @@ -1422,7 +1422,7 @@ void SerializeSnapshotableObjects(Realm* realm, } SnapshotableObject* ptr = static_cast(obj); - std::string type_name{ptr->GetTypeName()}; + std::string type_name = ptr->GetTypeName(); per_process::Debug(DebugCategory::MKSNAPSHOT, "Serialize snapshotable object %i (%p), " "object=%p, type=%s\n", diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h index a825350806bfb06..86bc9d96bdfad8b 100644 --- a/src/node_snapshotable.h +++ b/src/node_snapshotable.h @@ -22,6 +22,10 @@ struct PropInfo { SnapshotIndex index; // In the snapshot }; +#define UNSERIALIZABLE_OBJECT_TYPES(V) \ + V(http2_binding_data, http2::BindingData) \ + V(http_parser_binding_data, http_parser::BindingData) + #define SERIALIZABLE_OBJECT_TYPES(V) \ V(fs_binding_data, fs::BindingData) \ V(v8_binding_data, v8_utils::BindingData) \ @@ -31,8 +35,9 @@ struct PropInfo { enum class EmbedderObjectType : uint8_t { #define V(PropertyName, NativeType) k_##PropertyName, - SERIALIZABLE_OBJECT_TYPES(V) + SERIALIZABLE_OBJECT_TYPES(V) UNSERIALIZABLE_OBJECT_TYPES(V) #undef V + kEmbedderObjectTypeCount }; typedef size_t SnapshotIndex; @@ -101,7 +106,7 @@ class SnapshotableObject : public BaseObject { SnapshotableObject(Environment* env, v8::Local wrap, EmbedderObjectType type); - std::string_view GetTypeName() const; + std::string GetTypeName() const; // If returns false, the object will not be serialized. virtual bool PrepareForSerialization(v8::Local context, diff --git a/src/node_util.h b/src/node_util.h index 9590842ae4764dd..c36e4ea11a114f3 100644 --- a/src/node_util.h +++ b/src/node_util.h @@ -14,7 +14,6 @@ class WeakReference : public SnapshotableObject { public: SERIALIZABLE_OBJECT_METHODS() - static constexpr FastStringKey type_name{"node::util::WeakReference"}; static constexpr EmbedderObjectType type_int = EmbedderObjectType::k_util_weak_reference; diff --git a/src/node_v8.h b/src/node_v8.h index ecab454603b36bf..38e68e0b6a0106f 100644 --- a/src/node_v8.h +++ b/src/node_v8.h @@ -23,7 +23,6 @@ class BindingData : public SnapshotableObject { using InternalFieldInfo = InternalFieldInfoBase; SERIALIZABLE_OBJECT_METHODS() - static constexpr FastStringKey type_name{"node::v8::BindingData"}; static constexpr EmbedderObjectType type_int = EmbedderObjectType::k_v8_binding_data;