Skip to content

Commit

Permalink
src: use an array for faster binding data lookup
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
joyeecheung committed Feb 12, 2023
1 parent a37c083 commit c841b77
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 34 deletions.
26 changes: 15 additions & 11 deletions src/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,11 @@ inline T* Environment::GetBindingData(v8::Local<v8::Context> 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<T*>(it->second.get());
constexpr size_t binding_index = static_cast<size_t>(T::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->env(), GetCurrent(context));
return result;
Expand All @@ -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<size_t>(T::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();
}
Expand Down
4 changes: 3 additions & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,9 @@ MaybeLocal<Value> 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();
Expand Down
8 changes: 4 additions & 4 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -601,10 +601,10 @@ 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::array<BaseObjectPtr<BaseObject>,
static_cast<size_t>(
EmbedderObjectType::kEmbedderObjectTypeCount)>
BindingDataStore;

// Create an Environment without initializing a main Context. Use
// InitializeMainContext() to initialize a main context for it.
Expand Down
1 change: 0 additions & 1 deletion src/node_blob.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 0 additions & 1 deletion src/node_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion src/node_http2_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/node_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
8 changes: 4 additions & 4 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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, \
Expand Down Expand Up @@ -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",
Expand Down
9 changes: 7 additions & 2 deletions src/node_snapshotable.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/node_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 0 additions & 1 deletion src/node_v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit c841b77

Please sign in to comment.