src: use type_int as binding data store key

Locally the hashing of the binding names sometimes has significant presence in the profile of bindings, which makes the cost of adding a new binding data non-trivial, but it's wasteful to spend time on hashing them at all, since we can just use the numeric type as the key, as the names are not actually used beyond debugging purposes.
nodejs · Feb 12, 2023 · e25375f · e25375f
1 parent a37c083
commit e25375f
Show file tree

Hide file tree

Showing 12 changed files with 32 additions and 29 deletions.
diff --git 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<Object> obj) : BaseObject(env, obj) {}
 
-  static constexpr FastStringKey type_name { "http_parser" };
+  static constexpr EmbedderObjectType type_int =
+      EmbedderObjectType::k_http_parser_binding_data;
 
   std::vector<char> parser_buffer;
   bool parser_buffer_in_use = false;
@@ -527,12 +537,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/env-inl.h b/src/env-inl.h
@@ -218,7 +218,7 @@ inline T* Environment::GetBindingData(v8::Local<v8::Context> context) {
       context->GetAlignedPointerFromEmbedderData(
           ContextEmbedderIndex::kBindingListIndex));
   DCHECK_NOT_NULL(map);
-  auto it = map->find(T::type_name);
+  auto it = map->find(static_cast<uint8_t>(T::type_int));
   if (UNLIKELY(it == map->end())) return nullptr;
   T* result = static_cast<T*>(it->second.get());
   DCHECK_NOT_NULL(result);
@@ -237,7 +237,7 @@ inline T* Environment::AddBindingData(
       context->GetAlignedPointerFromEmbedderData(
           ContextEmbedderIndex::kBindingListIndex));
   DCHECK_NOT_NULL(map);
-  auto result = map->emplace(T::type_name, item);
+  auto result = map->emplace(static_cast<uint8_t>(T::type_int), item);
   CHECK(result.second);
   DCHECK_EQ(GetBindingData<T>(context), item.get());
   return item.get();

diff --git a/src/env.h b/src/env.h
@@ -601,10 +601,7 @@ class Environment : public MemoryRetainer {
   template <typename T>
   static inline T* GetBindingData(v8::Local<v8::Context> context);
 
-  typedef std::unordered_map<
-      FastStringKey,
-      BaseObjectPtr<BaseObject>,
-      FastStringKey::Hash> BindingDataStore;
+  typedef std::unordered_map<uint8_t, BaseObjectPtr<BaseObject>> 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
@@ -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
@@ -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
@@ -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
@@ -96,7 +96,8 @@ class BindingData : public BaseObject {
   BindingData(Environment* env, Local<Object> obj)
       : BaseObject(env, obj) {}
 
-  static constexpr FastStringKey type_name { "http_parser" };
+  static constexpr EmbedderObjectType type_int =
+      EmbedderObjectType::k_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,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
@@ -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<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,                                                                \
@@ -1422,7 +1422,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,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)                                    \
@@ -32,6 +36,7 @@ struct PropInfo {
 enum class EmbedderObjectType : uint8_t {
 #define V(PropertyName, NativeType) k_##PropertyName,
   SERIALIZABLE_OBJECT_TYPES(V)
+  UNSERIALIZABLE_OBJECT_TYPES(V)
 #undef V
 };
 
@@ -101,7 +106,7 @@ class SnapshotableObject : public BaseObject {
   SnapshotableObject(Environment* env,
                      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,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
@@ -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;