Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c0fcad3

Browse filesBrowse files
joyeecheungtargos
joyeecheung
authored and
targos
committed
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 <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 68dde38 commit c0fcad3
Copy full SHA for c0fcad3
Expand file treeCollapse file tree

16 files changed

+117
-56
lines changed
Open diff view settings
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@
573573
'src/async_wrap-inl.h',
574574
'src/base_object.h',
575575
'src/base_object-inl.h',
576+
'src/base_object_types.h',
576577
'src/base64.h',
577578
'src/base64-inl.h',
578579
'src/callback_queue.h',
Collapse file

‎src/README.md‎

Copy file name to clipboardExpand all lines: src/README.md
+19-11Lines changed: 19 additions & 11 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -486,18 +486,32 @@ that state is through the use of `Realm::AddBindingData`, which gives
486486
binding functions access to an object for storing such state.
487487
That object is always a [`BaseObject`][].
488488

489-
Its class needs to have a static `type_name` field based on a
490-
constant string, in order to disambiguate it from other classes of this type,
491-
and which could e.g. match the binding's name (in the example above, that would
492-
be `cares_wrap`).
489+
In the binding, call `SET_BINDING_ID()` with an identifier for the binding
490+
type. For example, for `http_parser::BindingData`, the identifier can be
491+
`http_parser_binding_data`.
492+
493+
If the binding should be supported in a snapshot, the id and the
494+
fully-specified class name should be added to the `SERIALIZABLE_BINDING_TYPES`
495+
list in `base_object_types.h`, and the class should implement the serialization
496+
and deserialization methods. See the comments of `SnapshotableObject` on how to
497+
implement them. Otherwise, add the id and the class name to the
498+
`UNSERIALIZABLE_BINDING_TYPES` list instead.
493499

494500
```cpp
501+
// In base_object_types.h, add the binding to either
502+
// UNSERIALIZABLE_BINDING_TYPES or SERIALIZABLE_BINDING_TYPES.
503+
// The second parameter is a descriptive name of the class, which is
504+
// usually the fully-specified class name.
505+
506+
#define UNSERIALIZABLE_BINDING_TYPES(V) \
507+
V(http_parser_binding_data, http_parser::BindingData)
508+
495509
// In the HTTP parser source code file:
496510
class BindingData : public BaseObject {
497511
public:
498512
BindingData(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}
499513

500-
static constexpr FastStringKey type_name { "http_parser" };
514+
SET_BINDING_ID(http_parser_binding_data)
501515

502516
std::vector<char> parser_buffer;
503517
bool parser_buffer_in_use = false;
@@ -527,12 +541,6 @@ void InitializeHttpParser(Local<Object> target,
527541
}
528542
```
529543
530-
If the binding is loaded during bootstrap, add it to the
531-
`SERIALIZABLE_OBJECT_TYPES` list in `src/node_snapshotable.h` and
532-
inherit from the `SnapshotableObject` class instead. See the comments
533-
of `SnapshotableObject` on how to implement its serialization and
534-
deserialization.
535-
536544
<a id="exception-handling"></a>
537545
538546
### Exception handling
Collapse file

‎src/base_object.h‎

Copy file name to clipboardExpand all lines: src/base_object.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
2626

2727
#include <type_traits> // std::remove_reference
28+
#include "base_object_types.h"
2829
#include "memory_tracker.h"
2930
#include "v8.h"
3031

Collapse file

‎src/base_object_types.h‎

Copy file name to clipboard
+69Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
#ifndef SRC_BASE_OBJECT_TYPES_H_
2+
#define SRC_BASE_OBJECT_TYPES_H_
3+
4+
#include <cinttypes>
5+
6+
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
7+
8+
namespace node {
9+
// List of internalBinding() data wrappers. The first argument should match
10+
// what the class passes to SET_BINDING_ID(), the second argument should match
11+
// the C++ class name.
12+
#define SERIALIZABLE_BINDING_TYPES(V) \
13+
V(fs_binding_data, fs::BindingData) \
14+
V(v8_binding_data, v8_utils::BindingData) \
15+
V(blob_binding_data, BlobBindingData) \
16+
V(process_binding_data, process::BindingData)
17+
18+
#define UNSERIALIZABLE_BINDING_TYPES(V) \
19+
V(http2_binding_data, http2::BindingData) \
20+
V(http_parser_binding_data, http_parser::BindingData)
21+
22+
// List of (non-binding) BaseObjects that are serializable in the snapshot.
23+
// The first argument should match what the type passes to
24+
// SET_OBJECT_ID(), the second argument should match the C++ class
25+
// name.
26+
#define SERIALIZABLE_NON_BINDING_TYPES(V) \
27+
V(util_weak_reference, util::WeakReference)
28+
29+
// Helper list of all binding data wrapper types.
30+
#define BINDING_TYPES(V) \
31+
SERIALIZABLE_BINDING_TYPES(V) \
32+
UNSERIALIZABLE_BINDING_TYPES(V)
33+
34+
// Helper list of all BaseObjects that implement snapshot support.
35+
#define SERIALIZABLE_OBJECT_TYPES(V) \
36+
SERIALIZABLE_BINDING_TYPES(V) \
37+
SERIALIZABLE_NON_BINDING_TYPES(V)
38+
39+
#define V(TypeId, NativeType) k_##TypeId,
40+
enum class BindingDataType : uint8_t { BINDING_TYPES(V) kBindingDataTypeCount };
41+
// Make sure that we put the bindings first so that we can also use the enums
42+
// for the bindings as index to the binding data store.
43+
enum class EmbedderObjectType : uint8_t {
44+
BINDING_TYPES(V) SERIALIZABLE_NON_BINDING_TYPES(V)
45+
// We do not need to know about all the unserializable non-binding types for
46+
// now so we do not list them.
47+
kEmbedderObjectTypeCount
48+
};
49+
#undef V
50+
51+
// For now, BaseObjects only need to call this when they implement snapshot
52+
// support.
53+
#define SET_OBJECT_ID(TypeId) \
54+
static constexpr EmbedderObjectType type_int = EmbedderObjectType::k_##TypeId;
55+
56+
// Binding data should call this so that they can be looked up from the binding
57+
// data store.
58+
#define SET_BINDING_ID(TypeId) \
59+
static constexpr BindingDataType binding_type_int = \
60+
BindingDataType::k_##TypeId; \
61+
SET_OBJECT_ID(TypeId) \
62+
static_assert(static_cast<uint8_t>(type_int) == \
63+
static_cast<uint8_t>(binding_type_int));
64+
65+
} // namespace node
66+
67+
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
68+
69+
#endif // SRC_BASE_OBJECT_TYPES_H_
Collapse file

‎src/node_blob.h‎

Copy file name to clipboardExpand all lines: src/node_blob.h
+1-3Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,7 @@ class BlobBindingData : public SnapshotableObject {
117117

118118
SERIALIZABLE_OBJECT_METHODS()
119119

120-
static constexpr FastStringKey type_name{"node::BlobBindingData"};
121-
static constexpr EmbedderObjectType type_int =
122-
EmbedderObjectType::k_blob_binding_data;
120+
SET_BINDING_ID(blob_binding_data)
123121

124122
void MemoryInfo(MemoryTracker* tracker) const override;
125123
SET_SELF_SIZE(BlobBindingData)
Collapse file

‎src/node_file.h‎

Copy file name to clipboardExpand all lines: src/node_file.h
+1-3Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,7 @@ class BindingData : public SnapshotableObject {
7070

7171
using InternalFieldInfo = InternalFieldInfoBase;
7272
SERIALIZABLE_OBJECT_METHODS()
73-
static constexpr FastStringKey type_name{"node::fs::BindingData"};
74-
static constexpr EmbedderObjectType type_int =
75-
EmbedderObjectType::k_fs_binding_data;
73+
SET_BINDING_ID(fs_binding_data)
7674

7775
void MemoryInfo(MemoryTracker* tracker) const override;
7876
SET_SELF_SIZE(BindingData)
Collapse file

‎src/node_http2_state.h‎

Copy file name to clipboardExpand all lines: src/node_http2_state.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ class Http2State : public BaseObject {
125125
SET_SELF_SIZE(Http2State)
126126
SET_MEMORY_INFO_NAME(Http2State)
127127

128-
static constexpr FastStringKey type_name { "http2" };
128+
SET_BINDING_ID(http2_binding_data)
129129

130130
private:
131131
struct http2_state_internal {
Collapse file

‎src/node_http_parser.cc‎

Copy file name to clipboardExpand all lines: src/node_http_parser.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class BindingData : public BaseObject {
9595
public:
9696
BindingData(Realm* realm, Local<Object> obj) : BaseObject(realm, obj) {}
9797

98-
static constexpr FastStringKey type_name { "http_parser" };
98+
SET_BINDING_ID(http_parser_binding_data)
9999

100100
std::vector<char> parser_buffer;
101101
bool parser_buffer_in_use = false;
Collapse file

‎src/node_process.h‎

Copy file name to clipboardExpand all lines: src/node_process.h
+1-3Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,7 @@ class BindingData : public SnapshotableObject {
5454
using InternalFieldInfo = InternalFieldInfoBase;
5555

5656
SERIALIZABLE_OBJECT_METHODS()
57-
static constexpr FastStringKey type_name{"node::process::BindingData"};
58-
static constexpr EmbedderObjectType type_int =
59-
EmbedderObjectType::k_process_binding_data;
57+
SET_BINDING_ID(process_binding_data)
6058

6159
BindingData(Realm* realm, v8::Local<v8::Object> object);
6260

Collapse file

‎src/node_realm-inl.h‎

Copy file name to clipboardExpand all lines: src/node_realm-inl.h
+9-5Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,11 @@ inline T* Realm::GetBindingData(v8::Local<v8::Context> context) {
6666
static_cast<BindingDataStore*>(context->GetAlignedPointerFromEmbedderData(
6767
ContextEmbedderIndex::kBindingDataStoreIndex));
6868
DCHECK_NOT_NULL(map);
69-
auto it = map->find(T::type_name);
70-
if (UNLIKELY(it == map->end())) return nullptr;
71-
T* result = static_cast<T*>(it->second.get());
69+
constexpr size_t binding_index = static_cast<size_t>(T::binding_type_int);
70+
static_assert(binding_index < std::tuple_size_v<BindingDataStore>);
71+
auto ptr = (*map)[binding_index];
72+
if (UNLIKELY(!ptr)) return nullptr;
73+
T* result = static_cast<T*>(ptr.get());
7274
DCHECK_NOT_NULL(result);
7375
DCHECK_EQ(result->realm(), GetCurrent(context));
7476
return result;
@@ -84,8 +86,10 @@ inline T* Realm::AddBindingData(v8::Local<v8::Context> context,
8486
static_cast<BindingDataStore*>(context->GetAlignedPointerFromEmbedderData(
8587
ContextEmbedderIndex::kBindingDataStoreIndex));
8688
DCHECK_NOT_NULL(map);
87-
auto result = map->emplace(T::type_name, item);
88-
CHECK(result.second);
89+
constexpr size_t binding_index = static_cast<size_t>(T::binding_type_int);
90+
static_assert(binding_index < std::tuple_size_v<BindingDataStore>);
91+
CHECK(!(*map)[binding_index]); // Should not insert the binding twice.
92+
(*map)[binding_index] = item;
8993
DCHECK_EQ(GetBindingData<T>(context), item.get());
9094
return item.get();
9195
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.