From f476c6dc0c1f6ed16fab22b91ce341ead669a952 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 28 Jan 2021 23:37:21 +0800 Subject: [PATCH] src: refactor v8 binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/37112 Refs: https://github.com/nodejs/node/pull/36943 Reviewed-By: Juan José Arboleda --- node.gyp | 1 + src/node_v8.cc | 100 ++++++++++++++++++++----------------------------- src/node_v8.h | 36 ++++++++++++++++++ 3 files changed, 77 insertions(+), 60 deletions(-) create mode 100644 src/node_v8.h diff --git a/node.gyp b/node.gyp index 69b4adf96cb7e9..61b789ec74ab55 100644 --- a/node.gyp +++ b/node.gyp @@ -721,6 +721,7 @@ 'src/node_union_bytes.h', 'src/node_url.h', 'src/node_version.h', + 'src/node_v8.h', 'src/node_v8_platform-inl.h', 'src/node_wasi.h', 'src/node_watchdog.h', diff --git a/src/node_v8.cc b/src/node_v8.cc index d66b5e03b8620c..4354e1e1772d82 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -19,15 +19,16 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -#include "node.h" +#include "node_v8.h" #include "base_object-inl.h" #include "env-inl.h" #include "memory_tracker-inl.h" +#include "node.h" #include "util-inl.h" #include "v8.h" namespace node { - +namespace v8_utils { using v8::Array; using v8::Context; using v8::FunctionCallbackInfo; @@ -44,7 +45,6 @@ using v8::Uint32; using v8::V8; using v8::Value; - #define HEAP_STATISTICS_PROPERTIES(V) \ V(0, total_heap_size, kTotalHeapSizeIndex) \ V(1, total_heap_size_executable, kTotalHeapSizeExecutableIndex) \ @@ -63,7 +63,6 @@ static constexpr size_t kHeapStatisticsPropertiesCount = HEAP_STATISTICS_PROPERTIES(V); #undef V - #define HEAP_SPACE_STATISTICS_PROPERTIES(V) \ V(0, space_size, kSpaceSizeIndex) \ V(1, space_used_size, kSpaceUsedSizeIndex) \ @@ -85,32 +84,34 @@ static const size_t kHeapCodeStatisticsPropertiesCount = HEAP_CODE_STATISTICS_PROPERTIES(V); #undef V -class BindingData : public BaseObject { - public: - BindingData(Environment* env, Local obj) - : BaseObject(env, obj), - heap_statistics_buffer(env->isolate(), kHeapStatisticsPropertiesCount), - heap_space_statistics_buffer(env->isolate(), - kHeapSpaceStatisticsPropertiesCount), - heap_code_statistics_buffer(env->isolate(), - kHeapCodeStatisticsPropertiesCount) {} - - static constexpr FastStringKey type_name { "v8" }; - - AliasedFloat64Array heap_statistics_buffer; - AliasedFloat64Array heap_space_statistics_buffer; - AliasedFloat64Array heap_code_statistics_buffer; - - void MemoryInfo(MemoryTracker* tracker) const override { - tracker->TrackField("heap_statistics_buffer", heap_statistics_buffer); - tracker->TrackField("heap_space_statistics_buffer", - heap_space_statistics_buffer); - tracker->TrackField("heap_code_statistics_buffer", - heap_code_statistics_buffer); - } - SET_SELF_SIZE(BindingData) - SET_MEMORY_INFO_NAME(BindingData) -}; +BindingData::BindingData(Environment* env, Local obj) + : BaseObject(env, obj), + heap_statistics_buffer(env->isolate(), kHeapStatisticsPropertiesCount), + heap_space_statistics_buffer(env->isolate(), + kHeapSpaceStatisticsPropertiesCount), + heap_code_statistics_buffer(env->isolate(), + kHeapCodeStatisticsPropertiesCount) { + obj->Set(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "heapStatisticsBuffer"), + heap_statistics_buffer.GetJSArray()) + .Check(); + obj->Set(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "heapCodeStatisticsBuffer"), + heap_code_statistics_buffer.GetJSArray()) + .Check(); + obj->Set(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "heapSpaceStatisticsBuffer"), + heap_space_statistics_buffer.GetJSArray()) + .Check(); +} + +void BindingData::MemoryInfo(MemoryTracker* tracker) const { + tracker->TrackField("heap_statistics_buffer", heap_statistics_buffer); + tracker->TrackField("heap_space_statistics_buffer", + heap_space_statistics_buffer); + tracker->TrackField("heap_code_statistics_buffer", + heap_code_statistics_buffer); +} // TODO(addaleax): Remove once we're on C++17. constexpr FastStringKey BindingData::type_name; @@ -179,36 +180,12 @@ void Initialize(Local target, env->SetMethodNoSideEffect(target, "cachedDataVersionTag", CachedDataVersionTag); - - // Export symbols used by v8.getHeapStatistics() env->SetMethod( target, "updateHeapStatisticsBuffer", UpdateHeapStatisticsBuffer); - target - ->Set(env->context(), - FIXED_ONE_BYTE_STRING(env->isolate(), "heapStatisticsBuffer"), - binding_data->heap_statistics_buffer.GetJSArray()) - .Check(); - -#define V(i, _, name) \ - target->Set(env->context(), \ - FIXED_ONE_BYTE_STRING(env->isolate(), #name), \ - Uint32::NewFromUnsigned(env->isolate(), i)).Check(); - - HEAP_STATISTICS_PROPERTIES(V) - - // Export symbols used by v8.getHeapCodeStatistics() env->SetMethod( target, "updateHeapCodeStatisticsBuffer", UpdateHeapCodeStatisticsBuffer); - target - ->Set(env->context(), - FIXED_ONE_BYTE_STRING(env->isolate(), "heapCodeStatisticsBuffer"), - binding_data->heap_code_statistics_buffer.GetJSArray()) - .Check(); - - HEAP_CODE_STATISTICS_PROPERTIES(V) - size_t number_of_heap_spaces = env->isolate()->NumberOfHeapSpaces(); // Heap space names are extracted once and exposed to JavaScript to @@ -230,13 +207,15 @@ void Initialize(Local target, "updateHeapSpaceStatisticsBuffer", UpdateHeapSpaceStatisticsBuffer); - target - ->Set(env->context(), - FIXED_ONE_BYTE_STRING(env->isolate(), - "heapSpaceStatisticsBuffer"), - binding_data->heap_space_statistics_buffer.GetJSArray()) +#define V(i, _, name) \ + target \ + ->Set(env->context(), \ + FIXED_ONE_BYTE_STRING(env->isolate(), #name), \ + Uint32::NewFromUnsigned(env->isolate(), i)) \ .Check(); + HEAP_STATISTICS_PROPERTIES(V) + HEAP_CODE_STATISTICS_PROPERTIES(V) HEAP_SPACE_STATISTICS_PROPERTIES(V) #undef V @@ -244,6 +223,7 @@ void Initialize(Local target, env->SetMethod(target, "setFlagsFromString", SetFlagsFromString); } +} // namespace v8_utils } // namespace node -NODE_MODULE_CONTEXT_AWARE_INTERNAL(v8, node::Initialize) +NODE_MODULE_CONTEXT_AWARE_INTERNAL(v8, node::v8_utils::Initialize) diff --git a/src/node_v8.h b/src/node_v8.h new file mode 100644 index 00000000000000..745c6580b844f4 --- /dev/null +++ b/src/node_v8.h @@ -0,0 +1,36 @@ +#ifndef SRC_NODE_V8_H_ +#define SRC_NODE_V8_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "aliased_buffer.h" +#include "base_object.h" +#include "util.h" +#include "v8.h" + +namespace node { +class Environment; + +namespace v8_utils { +class BindingData : public BaseObject { + public: + BindingData(Environment* env, v8::Local obj); + + static constexpr FastStringKey type_name{"node::v8::BindingData"}; + + AliasedFloat64Array heap_statistics_buffer; + AliasedFloat64Array heap_space_statistics_buffer; + AliasedFloat64Array heap_code_statistics_buffer; + + void MemoryInfo(MemoryTracker* tracker) const override; + SET_SELF_SIZE(BindingData) + SET_MEMORY_INFO_NAME(BindingData) +}; + +} // namespace v8_utils + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_NODE_V8_H_