Skip to content

Commit 1a9bcdf

Browse files
joyeecheungdanielleadams
joyeecheung
authored and
danielleadams
committed
src: refactor v8 binding
1. Put the v8 binding data class into a header so we can reuse the class definition during deserialization. 2. Put the v8 binding code into node::v8_utils namespace for clarity. 3. Move the binding data property initialization into its constructor so that we can reuse it during deserialization 4. Reorder the v8 binding initialization so that we don't unnecessarily initialize the properties in a loop PR-URL: #37112 Refs: #36943 Reviewed-By: Juan José Arboleda <[email protected]>
1 parent 54d36b0 commit 1a9bcdf

File tree

3 files changed

+77
-60
lines changed

3 files changed

+77
-60
lines changed

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,7 @@
746746
'src/node_union_bytes.h',
747747
'src/node_url.h',
748748
'src/node_version.h',
749+
'src/node_v8.h',
749750
'src/node_v8_platform-inl.h',
750751
'src/node_wasi.h',
751752
'src/node_watchdog.h',

‎src/node_v8.cc‎

Lines changed: 40 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,16 @@
1919
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
2020
// USE OR OTHER DEALINGS IN THE SOFTWARE.
2121

22-
#include "node.h"
22+
#include "node_v8.h"
2323
#include "base_object-inl.h"
2424
#include "env-inl.h"
2525
#include "memory_tracker-inl.h"
26+
#include "node.h"
2627
#include "util-inl.h"
2728
#include "v8.h"
2829

2930
namespace node {
30-
31+
namespace v8_utils {
3132
using v8::Array;
3233
using v8::Context;
3334
using v8::FunctionCallbackInfo;
@@ -44,7 +45,6 @@ using v8::Uint32;
4445
using v8::V8;
4546
using v8::Value;
4647

47-
4848
#define HEAP_STATISTICS_PROPERTIES(V) \
4949
V(0, total_heap_size, kTotalHeapSizeIndex) \
5050
V(1, total_heap_size_executable, kTotalHeapSizeExecutableIndex) \
@@ -63,7 +63,6 @@ static constexpr size_t kHeapStatisticsPropertiesCount =
6363
HEAP_STATISTICS_PROPERTIES(V);
6464
#undef V
6565

66-
6766
#define HEAP_SPACE_STATISTICS_PROPERTIES(V) \
6867
V(0, space_size, kSpaceSizeIndex) \
6968
V(1, space_used_size, kSpaceUsedSizeIndex) \
@@ -85,32 +84,34 @@ static const size_t kHeapCodeStatisticsPropertiesCount =
8584
HEAP_CODE_STATISTICS_PROPERTIES(V);
8685
#undef V
8786

88-
class BindingData : public BaseObject {
89-
public:
90-
BindingData(Environment* env, Local<Object> obj)
91-
: BaseObject(env, obj),
92-
heap_statistics_buffer(env->isolate(), kHeapStatisticsPropertiesCount),
93-
heap_space_statistics_buffer(env->isolate(),
94-
kHeapSpaceStatisticsPropertiesCount),
95-
heap_code_statistics_buffer(env->isolate(),
96-
kHeapCodeStatisticsPropertiesCount) {}
97-
98-
static constexpr FastStringKey type_name { "v8" };
99-
100-
AliasedFloat64Array heap_statistics_buffer;
101-
AliasedFloat64Array heap_space_statistics_buffer;
102-
AliasedFloat64Array heap_code_statistics_buffer;
103-
104-
void MemoryInfo(MemoryTracker* tracker) const override {
105-
tracker->TrackField("heap_statistics_buffer", heap_statistics_buffer);
106-
tracker->TrackField("heap_space_statistics_buffer",
107-
heap_space_statistics_buffer);
108-
tracker->TrackField("heap_code_statistics_buffer",
109-
heap_code_statistics_buffer);
110-
}
111-
SET_SELF_SIZE(BindingData)
112-
SET_MEMORY_INFO_NAME(BindingData)
113-
};
87+
BindingData::BindingData(Environment* env, Local<Object> obj)
88+
: BaseObject(env, obj),
89+
heap_statistics_buffer(env->isolate(), kHeapStatisticsPropertiesCount),
90+
heap_space_statistics_buffer(env->isolate(),
91+
kHeapSpaceStatisticsPropertiesCount),
92+
heap_code_statistics_buffer(env->isolate(),
93+
kHeapCodeStatisticsPropertiesCount) {
94+
obj->Set(env->context(),
95+
FIXED_ONE_BYTE_STRING(env->isolate(), "heapStatisticsBuffer"),
96+
heap_statistics_buffer.GetJSArray())
97+
.Check();
98+
obj->Set(env->context(),
99+
FIXED_ONE_BYTE_STRING(env->isolate(), "heapCodeStatisticsBuffer"),
100+
heap_code_statistics_buffer.GetJSArray())
101+
.Check();
102+
obj->Set(env->context(),
103+
FIXED_ONE_BYTE_STRING(env->isolate(), "heapSpaceStatisticsBuffer"),
104+
heap_space_statistics_buffer.GetJSArray())
105+
.Check();
106+
}
107+
108+
void BindingData::MemoryInfo(MemoryTracker* tracker) const {
109+
tracker->TrackField("heap_statistics_buffer", heap_statistics_buffer);
110+
tracker->TrackField("heap_space_statistics_buffer",
111+
heap_space_statistics_buffer);
112+
tracker->TrackField("heap_code_statistics_buffer",
113+
heap_code_statistics_buffer);
114+
}
114115

115116
// TODO(addaleax): Remove once we're on C++17.
116117
constexpr FastStringKey BindingData::type_name;
@@ -179,36 +180,12 @@ void Initialize(Local<Object> target,
179180

180181
env->SetMethodNoSideEffect(target, "cachedDataVersionTag",
181182
CachedDataVersionTag);
182-
183-
// Export symbols used by v8.getHeapStatistics()
184183
env->SetMethod(
185184
target, "updateHeapStatisticsBuffer", UpdateHeapStatisticsBuffer);
186185

187-
target
188-
->Set(env->context(),
189-
FIXED_ONE_BYTE_STRING(env->isolate(), "heapStatisticsBuffer"),
190-
binding_data->heap_statistics_buffer.GetJSArray())
191-
.Check();
192-
193-
#define V(i, _, name) \
194-
target->Set(env->context(), \
195-
FIXED_ONE_BYTE_STRING(env->isolate(), #name), \
196-
Uint32::NewFromUnsigned(env->isolate(), i)).Check();
197-
198-
HEAP_STATISTICS_PROPERTIES(V)
199-
200-
// Export symbols used by v8.getHeapCodeStatistics()
201186
env->SetMethod(
202187
target, "updateHeapCodeStatisticsBuffer", UpdateHeapCodeStatisticsBuffer);
203188

204-
target
205-
->Set(env->context(),
206-
FIXED_ONE_BYTE_STRING(env->isolate(), "heapCodeStatisticsBuffer"),
207-
binding_data->heap_code_statistics_buffer.GetJSArray())
208-
.Check();
209-
210-
HEAP_CODE_STATISTICS_PROPERTIES(V)
211-
212189
size_t number_of_heap_spaces = env->isolate()->NumberOfHeapSpaces();
213190

214191
// Heap space names are extracted once and exposed to JavaScript to
@@ -230,20 +207,23 @@ void Initialize(Local<Object> target,
230207
"updateHeapSpaceStatisticsBuffer",
231208
UpdateHeapSpaceStatisticsBuffer);
232209

233-
target
234-
->Set(env->context(),
235-
FIXED_ONE_BYTE_STRING(env->isolate(),
236-
"heapSpaceStatisticsBuffer"),
237-
binding_data->heap_space_statistics_buffer.GetJSArray())
210+
#define V(i, _, name) \
211+
target \
212+
->Set(env->context(), \
213+
FIXED_ONE_BYTE_STRING(env->isolate(), #name), \
214+
Uint32::NewFromUnsigned(env->isolate(), i)) \
238215
.Check();
239216

217+
HEAP_STATISTICS_PROPERTIES(V)
218+
HEAP_CODE_STATISTICS_PROPERTIES(V)
240219
HEAP_SPACE_STATISTICS_PROPERTIES(V)
241220
#undef V
242221

243222
// Export symbols used by v8.setFlagsFromString()
244223
env->SetMethod(target, "setFlagsFromString", SetFlagsFromString);
245224
}
246225

226+
} // namespace v8_utils
247227
} // namespace node
248228

249-
NODE_MODULE_CONTEXT_AWARE_INTERNAL(v8, node::Initialize)
229+
NODE_MODULE_CONTEXT_AWARE_INTERNAL(v8, node::v8_utils::Initialize)

‎src/node_v8.h‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#ifndef SRC_NODE_V8_H_
2+
#define SRC_NODE_V8_H_
3+
4+
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
5+
6+
#include "aliased_buffer.h"
7+
#include "base_object.h"
8+
#include "util.h"
9+
#include "v8.h"
10+
11+
namespace node {
12+
class Environment;
13+
14+
namespace v8_utils {
15+
class BindingData : public BaseObject {
16+
public:
17+
BindingData(Environment* env, v8::Local<v8::Object> obj);
18+
19+
static constexpr FastStringKey type_name{"node::v8::BindingData"};
20+
21+
AliasedFloat64Array heap_statistics_buffer;
22+
AliasedFloat64Array heap_space_statistics_buffer;
23+
AliasedFloat64Array heap_code_statistics_buffer;
24+
25+
void MemoryInfo(MemoryTracker* tracker) const override;
26+
SET_SELF_SIZE(BindingData)
27+
SET_MEMORY_INFO_NAME(BindingData)
28+
};
29+
30+
} // namespace v8_utils
31+
32+
} // namespace node
33+
34+
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
35+
36+
#endif // SRC_NODE_V8_H_

0 commit comments

Comments
 (0)