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. PR-URL: #46620 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
nodejs · Feb 22, 2023 · f32aa19 · f32aa19
1 parent 1aed454
commit f32aa19
Show file tree

Hide file tree

Showing 16 changed files with 117 additions and 56 deletions.
diff --git a/node.gyp b/node.gyp
@@ -573,6 +573,7 @@
         'src/async_wrap-inl.h',
         'src/base_object.h',
         'src/base_object-inl.h',
+        'src/base_object_types.h',
         'src/base64.h',
         'src/base64-inl.h',
         'src/callback_queue.h',

diff --git a/src/README.md b/src/README.md
@@ -486,18 +486,32 @@ that state is through the use of `Realm::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`).
+In the binding, call `SET_BINDING_ID()` with an identifier for the binding
+type. For example, for `http_parser::BindingData`, the identifier can be
+`http_parser_binding_data`.
+
+If the binding should be supported in a snapshot, the id and the
+fully-specified class name should be added to the `SERIALIZABLE_BINDING_TYPES`
+list in `base_object_types.h`, and the class should implement the serialization
+and deserialization methods. See the comments of `SnapshotableObject` on how to
+implement them. Otherwise, add the id and the class name to the
+`UNSERIALIZABLE_BINDING_TYPES` list instead.
 
 ```cpp
+// In base_object_types.h, add the binding to either
+// UNSERIALIZABLE_BINDING_TYPES or SERIALIZABLE_BINDING_TYPES.
+// The second parameter is a descriptive name of the class, which is
+// usually the fully-specified class name.
+
+#define UNSERIALIZABLE_BINDING_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<Object> obj) : BaseObject(env, obj) {}
 
-  static constexpr FastStringKey type_name { "http_parser" };
+  SET_BINDING_ID(http_parser_binding_data)
 
   std::vector<char> parser_buffer;
   bool parser_buffer_in_use = false;
@@ -527,12 +541,6 @@ void InitializeHttpParser(Local<Object> 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.
-
 <a id="exception-handling"></a>
 
 ### Exception handling

diff --git a/src/base_object.h b/src/base_object.h
@@ -25,6 +25,7 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include <type_traits>  // std::remove_reference
+#include "base_object_types.h"
 #include "memory_tracker.h"
 #include "v8.h"
 

diff --git a/src/base_object_types.h b/src/base_object_types.h
@@ -0,0 +1,69 @@
+#ifndef SRC_BASE_OBJECT_TYPES_H_
+#define SRC_BASE_OBJECT_TYPES_H_
+
+#include <cinttypes>
+
+#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+namespace node {
+// List of internalBinding() data wrappers. The first argument should match
+// what the class passes to SET_BINDING_ID(), the second argument should match
+// the C++ class name.
+#define SERIALIZABLE_BINDING_TYPES(V)                                          \
+  V(fs_binding_data, fs::BindingData)                                          \
+  V(v8_binding_data, v8_utils::BindingData)                                    \
+  V(blob_binding_data, BlobBindingData)                                        \
+  V(process_binding_data, process::BindingData)
+
+#define UNSERIALIZABLE_BINDING_TYPES(V)                                        \
+  V(http2_binding_data, http2::BindingData)                                    \
+  V(http_parser_binding_data, http_parser::BindingData)
+
+// List of (non-binding) BaseObjects that are serializable in the snapshot.
+// The first argument should match what the type passes to
+// SET_OBJECT_ID(), the second argument should match the C++ class
+// name.
+#define SERIALIZABLE_NON_BINDING_TYPES(V)                                      \
+  V(util_weak_reference, util::WeakReference)
+
+// Helper list of all binding data wrapper types.
+#define BINDING_TYPES(V)                                                       \
+  SERIALIZABLE_BINDING_TYPES(V)                                                \
+  UNSERIALIZABLE_BINDING_TYPES(V)
+
+// Helper list of all BaseObjects that implement snapshot support.
+#define SERIALIZABLE_OBJECT_TYPES(V)                                           \
+  SERIALIZABLE_BINDING_TYPES(V)                                                \
+  SERIALIZABLE_NON_BINDING_TYPES(V)
+
+#define V(TypeId, NativeType) k_##TypeId,
+enum class BindingDataType : uint8_t { BINDING_TYPES(V) kBindingDataTypeCount };
+// Make sure that we put the bindings first so that we can also use the enums
+// for the bindings as index to the binding data store.
+enum class EmbedderObjectType : uint8_t {
+  BINDING_TYPES(V) SERIALIZABLE_NON_BINDING_TYPES(V)
+  // We do not need to know about all the unserializable non-binding types for
+  // now so we do not list them.
+  kEmbedderObjectTypeCount
+};
+#undef V
+
+// For now, BaseObjects only need to call this when they implement snapshot
+// support.
+#define SET_OBJECT_ID(TypeId)                                                  \
+  static constexpr EmbedderObjectType type_int = EmbedderObjectType::k_##TypeId;
+
+// Binding data should call this so that they can be looked up from the binding
+// data store.
+#define SET_BINDING_ID(TypeId)                                                 \
+  static constexpr BindingDataType binding_type_int =                          \
+      BindingDataType::k_##TypeId;                                             \
+  SET_OBJECT_ID(TypeId)                                                        \
+  static_assert(static_cast<uint8_t>(type_int) ==                              \
+                static_cast<uint8_t>(binding_type_int));
+
+}  // namespace node
+
+#endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#endif  // SRC_BASE_OBJECT_TYPES_H_
diff --git a/src/node_blob.h b/src/node_blob.h
@@ -117,9 +117,7 @@ class BlobBindingData : public SnapshotableObject {
 
   SERIALIZABLE_OBJECT_METHODS()
 
-  static constexpr FastStringKey type_name{"node::BlobBindingData"};
-  static constexpr EmbedderObjectType type_int =
-      EmbedderObjectType::k_blob_binding_data;
+  SET_BINDING_ID(blob_binding_data)
 
   void MemoryInfo(MemoryTracker* tracker) const override;
   SET_SELF_SIZE(BlobBindingData)

diff --git a/src/node_file.h b/src/node_file.h
@@ -70,9 +70,7 @@ 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;
+  SET_BINDING_ID(fs_binding_data)
 
   void MemoryInfo(MemoryTracker* tracker) const override;
   SET_SELF_SIZE(BindingData)

diff --git a/src/node_http2_state.h b/src/node_http2_state.h
@@ -125,7 +125,7 @@ class Http2State : public BaseObject {
   SET_SELF_SIZE(Http2State)
   SET_MEMORY_INFO_NAME(Http2State)
 
-  static constexpr FastStringKey type_name { "http2" };
+  SET_BINDING_ID(http2_binding_data)
 
  private:
   struct http2_state_internal {

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
@@ -95,7 +95,7 @@ class BindingData : public BaseObject {
  public:
   BindingData(Realm* realm, Local<Object> obj) : BaseObject(realm, obj) {}
 
-  static constexpr FastStringKey type_name { "http_parser" };
+  SET_BINDING_ID(http_parser_binding_data)
 
   std::vector<char> parser_buffer;
   bool parser_buffer_in_use = false;

diff --git a/src/node_process.h b/src/node_process.h
@@ -54,9 +54,7 @@ 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;
+  SET_BINDING_ID(process_binding_data)
 
   BindingData(Realm* realm, v8::Local<v8::Object> object);
 

diff --git a/src/node_realm-inl.h b/src/node_realm-inl.h
@@ -66,9 +66,11 @@ inline T* Realm::GetBindingData(v8::Local<v8::Context> context) {
       static_cast<BindingDataStore*>(context->GetAlignedPointerFromEmbedderData(
           ContextEmbedderIndex::kBindingDataStoreIndex));
   DCHECK_NOT_NULL(map);
-  auto it = map->find(T::type_name);
-  if (UNLIKELY(it == map->end())) return nullptr;
-  T* result = static_cast<T*>(it->second.get());
+  constexpr size_t binding_index = static_cast<size_t>(T::binding_type_int);
+  static_assert(binding_index < std::tuple_size_v<BindingDataStore>);
+  auto ptr = (*map)[binding_index];
+  if (UNLIKELY(!ptr)) return nullptr;
+  T* result = static_cast<T*>(ptr.get());
   DCHECK_NOT_NULL(result);
   DCHECK_EQ(result->realm(), GetCurrent(context));
   return result;
@@ -84,8 +86,10 @@ inline T* Realm::AddBindingData(v8::Local<v8::Context> context,
       static_cast<BindingDataStore*>(context->GetAlignedPointerFromEmbedderData(
           ContextEmbedderIndex::kBindingDataStoreIndex));
   DCHECK_NOT_NULL(map);
-  auto result = map->emplace(T::type_name, item);
-  CHECK(result.second);
+  constexpr size_t binding_index = static_cast<size_t>(T::binding_type_int);
+  static_assert(binding_index < std::tuple_size_v<BindingDataStore>);
+  CHECK(!(*map)[binding_index]);  // Should not insert the binding twice.
+  (*map)[binding_index] = item;
   DCHECK_EQ(GetBindingData<T>(context), item.get());
   return item.get();
 }

diff --git a/src/node_realm.cc b/src/node_realm.cc
@@ -302,8 +302,9 @@ void Realm::DoneBootstrapping() {
 
 void Realm::RunCleanup() {
   TRACE_EVENT0(TRACING_CATEGORY_NODE1(realm), "RunCleanup");
-  binding_data_store_.clear();
-
+  for (size_t i = 0; i < binding_data_store_.size(); ++i) {
+    binding_data_store_[i].reset();
+  }
   cleanup_queue_.Drain();
 }
 

diff --git a/src/node_realm.h b/src/node_realm.h
@@ -21,9 +21,9 @@ struct RealmSerializeInfo {
   friend std::ostream& operator<<(std::ostream& o, const RealmSerializeInfo& i);
 };
 
-using BindingDataStore = std::unordered_map<FastStringKey,
-                                            BaseObjectPtr<BaseObject>,
-                                            FastStringKey::Hash>;
+using BindingDataStore = std::array<BaseObjectPtr<BaseObject>,
+                     static_cast<size_t>(
+                         BindingDataType::kBindingDataTypeCount)>;
 
 /**
  * node::Realm is a container for a set of JavaScript objects and functions

diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
@@ -1289,11 +1289,11 @@ SnapshotableObject::SnapshotableObject(Realm* realm,
                                        EmbedderObjectType type)
     : BaseObject(realm, 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
@@ -1334,7 +1334,7 @@ void DeserializeNodeInternalFields(Local<Object> 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,                                                                \
@@ -1421,7 +1421,7 @@ void SerializeSnapshotableObjects(Realm* realm,
     }
     SnapshotableObject* ptr = static_cast<SnapshotableObject*>(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
@@ -22,19 +22,6 @@ struct PropInfo {
   SnapshotIndex index;  // In the snapshot
 };
 
-#define SERIALIZABLE_OBJECT_TYPES(V)                                           \
-  V(fs_binding_data, fs::BindingData)                                          \
-  V(v8_binding_data, v8_utils::BindingData)                                    \
-  V(blob_binding_data, BlobBindingData)                                        \
-  V(process_binding_data, process::BindingData)                                \
-  V(util_weak_reference, util::WeakReference)
-
-enum class EmbedderObjectType : uint8_t {
-#define V(PropertyName, NativeType) k_##PropertyName,
-  SERIALIZABLE_OBJECT_TYPES(V)
-#undef V
-};
-
 typedef size_t SnapshotIndex;
 
 // When serializing an embedder object, we'll serialize the native states
@@ -101,7 +88,7 @@ class SnapshotableObject : public BaseObject {
   SnapshotableObject(Realm* realm,
                      v8::Local<v8::Object> 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<v8::Context> context,

diff --git a/src/node_util.h b/src/node_util.h
@@ -14,9 +14,7 @@ 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;
+  SET_OBJECT_ID(util_weak_reference)
 
   WeakReference(Realm* realm,
                 v8::Local<v8::Object> object,

diff --git a/src/node_v8.h b/src/node_v8.h
@@ -23,9 +23,7 @@ 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;
+  SET_BINDING_ID(v8_binding_data)
 
   AliasedFloat64Array heap_statistics_buffer;
   AliasedFloat64Array heap_space_statistics_buffer;