From de6840c02936ccc03f0b8b9a8ebe531e041b07a6 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 28 Jan 2021 22:00:03 +0800 Subject: [PATCH 1/5] src: refactor bookkeeping of bootstrap status This patch 1. Refactors the bootstrap routine of the main instance so that when --no-node-snapshot is used, Environment::InitializeMainContext() will only be called once (previously it would be called twice, which was harmless for now but not ideal). 2. Mark the number of BaseObjects in RunBootstrapping() when creating the Environment from scratch and in InitializeMainContext() when the Environment is deserialized. Previously the marking was done in the Environment constructor and InitializeMainContext() respectively for the cctest which was incorrect because the cctest never uses an Environment that's not bootstrapped. Also renames the mark to base_object_created_after_bootstrap to reflect what it's intended for. --- src/env-inl.h | 14 ++++++++--- src/env.cc | 9 ------- src/env.h | 5 ++-- src/node.cc | 2 +- src/node_main_instance.cc | 24 +++++++++--------- test/cctest/test_base_object_ptr.cc | 38 ++++++++++++----------------- 6 files changed, 43 insertions(+), 49 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 4cb68e1c5dea8f..eeb74e17e26ed0 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -780,8 +780,12 @@ inline bool Environment::has_run_bootstrapping_code() const { return has_run_bootstrapping_code_; } -inline void Environment::set_has_run_bootstrapping_code(bool value) { - has_run_bootstrapping_code_ = value; +inline void Environment::DoneBootstrapping() { + has_run_bootstrapping_code_ = true; + // This adjusts the return value of base_object_created_after_bootstrap() so + // that tests that check the count do not have to account for internally + // created BaseObjects. + base_object_created_by_bootstrap_ = base_object_count_; } inline bool Environment::has_serialized_options() const { @@ -1085,8 +1089,12 @@ void Environment::modify_base_object_count(int64_t delta) { base_object_count_ += delta; } +int64_t Environment::base_object_created_after_bootstrap() const { + return base_object_count_ - base_object_created_by_bootstrap_; +} + int64_t Environment::base_object_count() const { - return base_object_count_ - initial_base_object_count_; + return base_object_count_; } void Environment::set_main_utf16(std::unique_ptr str) { diff --git a/src/env.cc b/src/env.cc index b618f840af8165..fe774b4bf3183b 100644 --- a/src/env.cc +++ b/src/env.cc @@ -414,10 +414,6 @@ Environment::Environment(IsolateData* isolate_data, "args", std::move(traced_value)); } - - // This adjusts the return value of base_object_count() so that tests that - // check the count do not have to account for internally created BaseObjects. - initial_base_object_count_ = base_object_count(); } Environment::Environment(IsolateData* isolate_data, @@ -460,10 +456,6 @@ void Environment::InitializeMainContext(Local context, per_process::node_start_time); performance_state_->Mark(performance::NODE_PERFORMANCE_MILESTONE_V8_START, performance::performance_v8_start); - - // This adjusts the return value of base_object_count() so that tests that - // check the count do not have to account for internally created BaseObjects. - initial_base_object_count_ = base_object_count(); } Environment::~Environment() { @@ -654,7 +646,6 @@ void Environment::RunCleanup() { TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment), "RunCleanup", this); bindings_.clear(); - initial_base_object_count_ = 0; CleanupHandles(); while (!cleanup_hooks_.empty() || diff --git a/src/env.h b/src/env.h index 514a3871b4ee05..e418983a06663c 100644 --- a/src/env.h +++ b/src/env.h @@ -1168,7 +1168,7 @@ class Environment : public MemoryRetainer { inline void add_refs(int64_t diff); inline bool has_run_bootstrapping_code() const; - inline void set_has_run_bootstrapping_code(bool has_run_bootstrapping_code); + inline void DoneBootstrapping(); inline bool has_serialized_options() const; inline void set_has_serialized_options(bool has_serialized_options); @@ -1354,6 +1354,7 @@ class Environment : public MemoryRetainer { // no memory leaks caused by BaseObjects staying alive longer than expected // (in particular, no circular BaseObjectPtr references). inline void modify_base_object_count(int64_t delta); + inline int64_t base_object_created_after_bootstrap() const; inline int64_t base_object_count() const; inline int32_t stack_trace_limit() const { return 10; } @@ -1547,7 +1548,7 @@ class Environment : public MemoryRetainer { bool started_cleanup_ = false; int64_t base_object_count_ = 0; - int64_t initial_base_object_count_ = 0; + int64_t base_object_created_by_bootstrap_ = 0; std::atomic_bool is_stopping_ { false }; std::unordered_set unmanaged_fds_; diff --git a/src/node.cc b/src/node.cc index c3f423cb579479..343a8d9b79a016 100644 --- a/src/node.cc +++ b/src/node.cc @@ -415,7 +415,7 @@ MaybeLocal Environment::RunBootstrapping() { CHECK(req_wrap_queue()->IsEmpty()); CHECK(handle_wrap_queue()->IsEmpty()); - set_has_run_bootstrapping_code(true); + DoneBootstrapping(); return scope.Escape(result); } diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 07277ccfa5c167..8ffeef0e8ae67f 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -209,10 +209,18 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code, {DeserializeNodeInternalFields, env.get()}) .ToLocalChecked(); + CHECK(!context.IsEmpty()); + Context::Scope context_scope(context); InitializeContextRuntime(context); SetIsolateErrorHandlers(isolate_, {}); + env->InitializeMainContext(context, env_info); +#if HAVE_INSPECTOR + env->InitializeInspector({}); +#endif + env->DoneBootstrapping(); } else { context = NewContext(isolate_); + CHECK(!context.IsEmpty()); Context::Scope context_scope(context); env.reset(new Environment(isolate_data_.get(), context, @@ -221,24 +229,16 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code, nullptr, EnvironmentFlags::kDefaultFlags, {})); - } - - CHECK(!context.IsEmpty()); - Context::Scope context_scope(context); - - env->InitializeMainContext(context, env_info); - #if HAVE_INSPECTOR - env->InitializeInspector({}); + env->InitializeInspector({}); #endif - - if (!deserialize_mode_ && env->RunBootstrapping().IsEmpty()) { - return nullptr; + if (env->RunBootstrapping().IsEmpty()) { + return nullptr; + } } CHECK(env->req_wrap_queue()->IsEmpty()); CHECK(env->handle_wrap_queue()->IsEmpty()); - env->set_has_run_bootstrapping_code(true); return env; } diff --git a/test/cctest/test_base_object_ptr.cc b/test/cctest/test_base_object_ptr.cc index a3a8df98a9efc7..52a1e2aff79dde 100644 --- a/test/cctest/test_base_object_ptr.cc +++ b/test/cctest/test_base_object_ptr.cc @@ -14,10 +14,6 @@ using v8::Isolate; using v8::Local; using v8::Object; -// Environments may come with existing BaseObject instances. -// This variable offsets the expected BaseObject counts. -static const int BASE_OBJECT_COUNT = 0; - class BaseObjectPtrTest : public EnvironmentTestFixture {}; class DummyBaseObject : public BaseObject { @@ -51,12 +47,12 @@ TEST_F(BaseObjectPtrTest, ScopedDetached) { Env env_{handle_scope, argv}; Environment* env = *env_; - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 0); { BaseObjectPtr ptr = DummyBaseObject::NewDetached(env); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 1); } - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 0); } TEST_F(BaseObjectPtrTest, ScopedDetachedWithWeak) { @@ -67,14 +63,14 @@ TEST_F(BaseObjectPtrTest, ScopedDetachedWithWeak) { BaseObjectWeakPtr weak_ptr; - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 0); { BaseObjectPtr ptr = DummyBaseObject::NewDetached(env); weak_ptr = ptr; - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 1); } EXPECT_EQ(weak_ptr.get(), nullptr); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 0); } TEST_F(BaseObjectPtrTest, Undetached) { @@ -86,13 +82,12 @@ TEST_F(BaseObjectPtrTest, Undetached) { node::AddEnvironmentCleanupHook( isolate_, [](void* arg) { - EXPECT_EQ(static_cast(arg)->base_object_count(), - BASE_OBJECT_COUNT); + EXPECT_EQ(static_cast(arg)->base_object_count(), 0); }, env); BaseObjectPtr ptr = DummyBaseObject::New(env); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 1); } TEST_F(BaseObjectPtrTest, GCWeak) { @@ -109,21 +104,21 @@ TEST_F(BaseObjectPtrTest, GCWeak) { weak_ptr = ptr; ptr->MakeWeak(); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 1); EXPECT_EQ(weak_ptr.get(), ptr.get()); EXPECT_EQ(weak_ptr->persistent().IsWeak(), false); ptr.reset(); } - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 1); EXPECT_NE(weak_ptr.get(), nullptr); EXPECT_EQ(weak_ptr->persistent().IsWeak(), true); v8::V8::SetFlagsFromString("--expose-gc"); isolate_->RequestGarbageCollectionForTesting(Isolate::kFullGarbageCollection); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 0); EXPECT_EQ(weak_ptr.get(), nullptr); } @@ -134,7 +129,7 @@ TEST_F(BaseObjectPtrTest, Moveable) { Environment* env = *env_; BaseObjectPtr ptr = DummyBaseObject::NewDetached(env); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 1); BaseObjectWeakPtr weak_ptr { ptr }; EXPECT_EQ(weak_ptr.get(), ptr.get()); @@ -145,12 +140,12 @@ TEST_F(BaseObjectPtrTest, Moveable) { BaseObjectWeakPtr weak_ptr2 = std::move(weak_ptr); EXPECT_EQ(weak_ptr2.get(), ptr2.get()); EXPECT_EQ(weak_ptr.get(), nullptr); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 1); ptr2.reset(); EXPECT_EQ(weak_ptr2.get(), nullptr); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 0); } TEST_F(BaseObjectPtrTest, NestedClasses) { @@ -174,8 +169,7 @@ TEST_F(BaseObjectPtrTest, NestedClasses) { node::AddEnvironmentCleanupHook( isolate_, [](void* arg) { - EXPECT_EQ(static_cast(arg)->base_object_count(), - BASE_OBJECT_COUNT); + EXPECT_EQ(static_cast(arg)->base_object_count(), 0); }, env); @@ -184,5 +178,5 @@ TEST_F(BaseObjectPtrTest, NestedClasses) { obj->ptr1 = DummyBaseObject::NewDetached(env); obj->ptr2 = DummyBaseObject::New(env); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 3); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 3); } From 3e7a3ddeac5b8c8c25a99322909b02bb659949ce Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 28 Jan 2021 23:22:18 +0800 Subject: [PATCH 2/5] src: rename binding_data_name to type_name in BindingData Previously, this was a per-class string constant for BindingData which is used as keys for identifying these objects in the binding data map. These are just type names of the BindingData. This patch renames the variable to type_name so that we can generalize this constant for other BaseObjects and use it for debugging and logging the types of other BaseObjects. --- src/README.md | 4 ++-- src/env-inl.h | 4 ++-- src/node_file.cc | 2 +- src/node_file.h | 2 +- src/node_http2.cc | 2 +- src/node_http2_state.h | 2 +- src/node_http_parser.cc | 4 ++-- src/node_v8.cc | 4 ++-- src/quic/node_quic.cc | 2 +- src/quic/node_quic_state.h | 2 +- 10 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/README.md b/src/README.md index aec56e7a7ba05a..a3b54416a0d2f5 100644 --- a/src/README.md +++ b/src/README.md @@ -422,7 +422,7 @@ 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 `binding_data_name` field based on a +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`). @@ -433,7 +433,7 @@ class BindingData : public BaseObject { public: BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} - static constexpr FastStringKey binding_data_name { "http_parser" }; + static constexpr FastStringKey type_name { "http_parser" }; std::vector parser_buffer; bool parser_buffer_in_use = false; diff --git a/src/env-inl.h b/src/env-inl.h index eeb74e17e26ed0..850363af5aeffb 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -358,7 +358,7 @@ inline T* Environment::GetBindingData(v8::Local context) { context->GetAlignedPointerFromEmbedderData( ContextEmbedderIndex::kBindingListIndex)); DCHECK_NOT_NULL(map); - auto it = map->find(T::binding_data_name); + auto it = map->find(T::type_name); if (UNLIKELY(it == map->end())) return nullptr; T* result = static_cast(it->second.get()); DCHECK_NOT_NULL(result); @@ -377,7 +377,7 @@ inline T* Environment::AddBindingData( context->GetAlignedPointerFromEmbedderData( ContextEmbedderIndex::kBindingListIndex)); DCHECK_NOT_NULL(map); - auto result = map->emplace(T::binding_data_name, item); + auto result = map->emplace(T::type_name, item); CHECK(result.second); DCHECK_EQ(GetBindingData(context), item.get()); return item.get(); diff --git a/src/node_file.cc b/src/node_file.cc index edf55e8766b84b..46b328b50d0b14 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2397,7 +2397,7 @@ void BindingData::MemoryInfo(MemoryTracker* tracker) const { } // TODO(addaleax): Remove once we're on C++17. -constexpr FastStringKey BindingData::binding_data_name; +constexpr FastStringKey BindingData::type_name; void Initialize(Local target, Local unused, diff --git a/src/node_file.h b/src/node_file.h index 95a68b5d89a704..9e652dc7a4afa4 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -27,7 +27,7 @@ class BindingData : public BaseObject { std::vector> file_handle_read_wrap_freelist; - static constexpr FastStringKey binding_data_name { "fs" }; + static constexpr FastStringKey type_name { "fs" }; void MemoryInfo(MemoryTracker* tracker) const override; SET_SELF_SIZE(BindingData) diff --git a/src/node_http2.cc b/src/node_http2.cc index 930167418e18db..a4b8e4d2ac7d50 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -3001,7 +3001,7 @@ void Http2State::MemoryInfo(MemoryTracker* tracker) const { } // TODO(addaleax): Remove once we're on C++17. -constexpr FastStringKey Http2State::binding_data_name; +constexpr FastStringKey Http2State::type_name; // Set up the process.binding('http2') binding. void Initialize(Local target, diff --git a/src/node_http2_state.h b/src/node_http2_state.h index cd29b207498dc0..7cf40ff1017ca3 100644 --- a/src/node_http2_state.h +++ b/src/node_http2_state.h @@ -127,7 +127,7 @@ class Http2State : public BaseObject { SET_SELF_SIZE(Http2State) SET_MEMORY_INFO_NAME(Http2State) - static constexpr FastStringKey binding_data_name { "http2" }; + static constexpr FastStringKey type_name { "http2" }; private: struct http2_state_internal { diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index affc66585ed89a..d3184bb1a14c92 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -88,7 +88,7 @@ class BindingData : public BaseObject { BindingData(Environment* env, Local obj) : BaseObject(env, obj) {} - static constexpr FastStringKey binding_data_name { "http_parser" }; + static constexpr FastStringKey type_name { "http_parser" }; std::vector parser_buffer; bool parser_buffer_in_use = false; @@ -101,7 +101,7 @@ class BindingData : public BaseObject { }; // TODO(addaleax): Remove once we're on C++17. -constexpr FastStringKey BindingData::binding_data_name; +constexpr FastStringKey BindingData::type_name; // helper class for the Parser struct StringPtr { diff --git a/src/node_v8.cc b/src/node_v8.cc index 4ab87dce326bb1..d66b5e03b8620c 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -95,7 +95,7 @@ class BindingData : public BaseObject { heap_code_statistics_buffer(env->isolate(), kHeapCodeStatisticsPropertiesCount) {} - static constexpr FastStringKey binding_data_name { "v8" }; + static constexpr FastStringKey type_name { "v8" }; AliasedFloat64Array heap_statistics_buffer; AliasedFloat64Array heap_space_statistics_buffer; @@ -113,7 +113,7 @@ class BindingData : public BaseObject { }; // TODO(addaleax): Remove once we're on C++17. -constexpr FastStringKey BindingData::binding_data_name; +constexpr FastStringKey BindingData::type_name; void CachedDataVersionTag(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); diff --git a/src/quic/node_quic.cc b/src/quic/node_quic.cc index f34c8669f94809..fb5ef9be9894c9 100644 --- a/src/quic/node_quic.cc +++ b/src/quic/node_quic.cc @@ -31,7 +31,7 @@ using v8::Value; namespace quic { -constexpr FastStringKey QuicState::binding_data_name; +constexpr FastStringKey QuicState::type_name; void QuicState::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("root_buffer", root_buffer); diff --git a/src/quic/node_quic_state.h b/src/quic/node_quic_state.h index 6eb76529f895fe..6348f917f697a1 100644 --- a/src/quic/node_quic_state.h +++ b/src/quic/node_quic_state.h @@ -60,7 +60,7 @@ class QuicState : public BaseObject { bool warn_trace_tls = true; - static constexpr FastStringKey binding_data_name { "quic" }; + static constexpr FastStringKey type_name { "quic" }; void MemoryInfo(MemoryTracker* tracker) const override; SET_SELF_SIZE(QuicState) From b31b9a333dc8c0f562d48e91138808666527d01a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 28 Jan 2021 23:37:21 +0800 Subject: [PATCH 3/5] 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 --- 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 a6ddb4e6dc4f56..f8ec5bfa29a016 100644 --- a/node.gyp +++ b/node.gyp @@ -749,6 +749,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_ From 9b7b6d7f461062e9f81ab87728e8002ce0e03bbc Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 29 Jan 2021 00:04:22 +0800 Subject: [PATCH 4/5] src: put (de)serialization code into node_snapshotable.h/cc So that it's easier to find the corresponding code. --- node.gyp | 2 ++ src/node_main_instance.cc | 14 +---------- src/node_snapshotable.cc | 39 ++++++++++++++++++++++++++++++ src/node_snapshotable.h | 21 ++++++++++++++++ tools/snapshot/snapshot_builder.cc | 18 +------------- 5 files changed, 64 insertions(+), 30 deletions(-) create mode 100644 src/node_snapshotable.cc create mode 100644 src/node_snapshotable.h diff --git a/node.gyp b/node.gyp index f8ec5bfa29a016..0db7e8a144cfd7 100644 --- a/node.gyp +++ b/node.gyp @@ -641,6 +641,7 @@ 'src/node_report_module.cc', 'src/node_report_utils.cc', 'src/node_serdes.cc', + 'src/node_snapshotable.cc', 'src/node_sockaddr.cc', 'src/node_stat_watcher.cc', 'src/node_symbols.cc', @@ -743,6 +744,7 @@ 'src/node_report.h', 'src/node_revert.h', 'src/node_root_certs.h', + 'src/node_snapshotable.h', 'src/node_sockaddr.h', 'src/node_sockaddr-inl.h', 'src/node_stat_watcher.h', diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 8ffeef0e8ae67f..9cbb3e7e05bc86 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -5,6 +5,7 @@ #include "node_external_reference.h" #include "node_internals.h" #include "node_options-inl.h" +#include "node_snapshotable.h" #include "node_v8_platform-inl.h" #include "util-inl.h" #if defined(LEAK_SANITIZER) @@ -22,7 +23,6 @@ using v8::HandleScope; using v8::Isolate; using v8::Local; using v8::Locker; -using v8::Object; std::unique_ptr NodeMainInstance::registry_ = nullptr; @@ -167,18 +167,6 @@ int NodeMainInstance::Run(const EnvSerializeInfo* env_info) { return exit_code; } -void DeserializeNodeInternalFields(Local holder, - int index, - v8::StartupData payload, - void* env) { - if (payload.raw_size == 0) { - holder->SetAlignedPointerInInternalField(index, nullptr); - return; - } - // No embedder object in the builtin snapshot yet. - UNREACHABLE(); -} - DeleteFnPtr NodeMainInstance::CreateMainEnvironment(int* exit_code, const EnvSerializeInfo* env_info) { diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc new file mode 100644 index 00000000000000..21de10868564d3 --- /dev/null +++ b/src/node_snapshotable.cc @@ -0,0 +1,39 @@ + +#include "node_snapshotable.h" +#include "base_object-inl.h" + +namespace node { + +using v8::Local; +using v8::Object; +using v8::StartupData; + +void DeserializeNodeInternalFields(Local holder, + int index, + v8::StartupData payload, + void* env) { + if (payload.raw_size == 0) { + holder->SetAlignedPointerInInternalField(index, nullptr); + return; + } + // No embedder object in the builtin snapshot yet. + UNREACHABLE(); +} + +StartupData SerializeNodeContextInternalFields(Local holder, + int index, + void* env) { + void* ptr = holder->GetAlignedPointerFromInternalField(index); + if (ptr == nullptr || ptr == env) { + return StartupData{nullptr, 0}; + } + if (ptr == env && index == ContextEmbedderIndex::kEnvironment) { + return StartupData{nullptr, 0}; + } + + // No embedder objects in the builtin snapshot yet. + UNREACHABLE(); + return StartupData{nullptr, 0}; +} + +} // namespace node diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h new file mode 100644 index 00000000000000..c45c1381b5554f --- /dev/null +++ b/src/node_snapshotable.h @@ -0,0 +1,21 @@ + +#ifndef SRC_NODE_SNAPSHOTABLE_H_ +#define SRC_NODE_SNAPSHOTABLE_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "v8.h" +namespace node { + +v8::StartupData SerializeNodeContextInternalFields(v8::Local holder, + int index, + void* env); +void DeserializeNodeInternalFields(v8::Local holder, + int index, + v8::StartupData payload, + void* env); +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_NODE_SNAPSHOTABLE_H_ diff --git a/tools/snapshot/snapshot_builder.cc b/tools/snapshot/snapshot_builder.cc index eb755b7d29ed7c..f97c646dfa17c4 100644 --- a/tools/snapshot/snapshot_builder.cc +++ b/tools/snapshot/snapshot_builder.cc @@ -6,6 +6,7 @@ #include "node_external_reference.h" #include "node_internals.h" #include "node_main_instance.h" +#include "node_snapshotable.h" #include "node_v8_platform-inl.h" namespace node { @@ -14,7 +15,6 @@ using v8::Context; using v8::HandleScope; using v8::Isolate; using v8::Local; -using v8::Object; using v8::SnapshotCreator; using v8::StartupData; @@ -75,22 +75,6 @@ const EnvSerializeInfo* NodeMainInstance::GetEnvSerializeInfo() { return ss.str(); } -static StartupData SerializeNodeContextInternalFields(Local holder, - int index, - void* env) { - void* ptr = holder->GetAlignedPointerFromInternalField(index); - if (ptr == nullptr || ptr == env) { - return StartupData{nullptr, 0}; - } - if (ptr == env && index == ContextEmbedderIndex::kEnvironment) { - return StartupData{nullptr, 0}; - } - - // No embedder objects in the builtin snapshot yet. - UNREACHABLE(); - return StartupData{nullptr, 0}; -} - std::string SnapshotBuilder::Generate( const std::vector args, const std::vector exec_args) { From 451c7a7bf126dda6e1280b608b08ee904eb7f640 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 29 Jan 2021 00:25:35 +0800 Subject: [PATCH 5/5] src: support fs and v8 binding data in snapshot --- lib/fs.js | 14 ++- lib/internal/bootstrap/node.js | 3 + lib/v8.js | 15 +-- src/aliased_buffer.h | 5 + src/base_object.h | 4 + src/env-inl.h | 11 +++ src/env.cc | 26 +++++ src/env.h | 22 +++++ src/node_dir.cc | 9 ++ src/node_external_reference.h | 3 + src/node_file.cc | 103 ++++++++++++++++++-- src/node_file.h | 15 ++- src/node_snapshotable.cc | 120 ++++++++++++++++++++++-- src/node_snapshotable.h | 88 ++++++++++++++++- src/node_stat_watcher.cc | 8 +- src/node_stat_watcher.h | 2 + src/node_v8.cc | 37 +++++++- src/node_v8.h | 7 +- src/stream_base.cc | 28 +++++- src/stream_base.h | 3 +- test/parallel/test-bootstrap-modules.js | 1 + 21 files changed, 477 insertions(+), 47 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index d01e99c21b0178..2342474ce09bcb 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -67,6 +67,10 @@ const { const pathModule = require('path'); const { isArrayBufferView } = require('internal/util/types'); + +// We need to get the statValues from the binding at the callsite since +// it's re-initialized after deserialization. + const binding = internalBinding('fs'); const { Buffer } = require('buffer'); const { @@ -81,7 +85,7 @@ const { uvException } = require('internal/errors'); -const { FSReqCallback, statValues } = binding; +const { FSReqCallback } = binding; const { toPathIfFileURL } = require('internal/url'); const internalUtil = require('internal/util'); const { @@ -1756,8 +1760,8 @@ function realpathSync(p, options) { // Continue if not a symlink, break if a pipe/socket if (knownHard[base] || cache?.get(base) === base) { - if (isFileType(statValues, S_IFIFO) || - isFileType(statValues, S_IFSOCK)) { + if (isFileType(binding.statValues, S_IFIFO) || + isFileType(binding.statValues, S_IFSOCK)) { break; } continue; @@ -1899,8 +1903,8 @@ function realpath(p, options, callback) { // Continue if not a symlink, break if a pipe/socket if (knownHard[base]) { - if (isFileType(statValues, S_IFIFO) || - isFileType(statValues, S_IFSOCK)) { + if (isFileType(binding.statValues, S_IFIFO) || + isFileType(binding.statValues, S_IFSOCK)) { return callback(null, encodeRealpathResult(p, options)); } return process.nextTick(LOOP); diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 818f51ae02ad6d..fc626635ea8848 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -347,6 +347,9 @@ process.emitWarning = emitWarning; // Note: only after this point are the timers effective } +require('fs'); +internalBinding('v8'); + function setupPrepareStackTrace() { const { setEnhanceStackForFatalException, diff --git a/lib/v8.js b/lib/v8.js index f1c624bc10add0..e7a44331b8b350 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -72,12 +72,13 @@ function getHeapSnapshot() { return new HeapSnapshotStream(handle); } +// We need to get the buffer from the binding at the callsite since +// it's re-initialized after deserialization. +const binding = internalBinding('v8'); + const { cachedDataVersionTag, setFlagsFromString: _setFlagsFromString, - heapStatisticsBuffer, - heapSpaceStatisticsBuffer, - heapCodeStatisticsBuffer, updateHeapStatisticsBuffer, updateHeapSpaceStatisticsBuffer, updateHeapCodeStatisticsBuffer, @@ -106,7 +107,7 @@ const { kCodeAndMetadataSizeIndex, kBytecodeAndMetadataSizeIndex, kExternalScriptSourceSizeIndex -} = internalBinding('v8'); +} = binding; const kNumberOfHeapSpaces = kHeapSpaces.length; @@ -116,7 +117,7 @@ function setFlagsFromString(flags) { } function getHeapStatistics() { - const buffer = heapStatisticsBuffer; + const buffer = binding.heapStatisticsBuffer; updateHeapStatisticsBuffer(); @@ -137,7 +138,7 @@ function getHeapStatistics() { function getHeapSpaceStatistics() { const heapSpaceStatistics = new Array(kNumberOfHeapSpaces); - const buffer = heapSpaceStatisticsBuffer; + const buffer = binding.heapSpaceStatisticsBuffer; for (let i = 0; i < kNumberOfHeapSpaces; i++) { updateHeapSpaceStatisticsBuffer(i); @@ -154,7 +155,7 @@ function getHeapSpaceStatistics() { } function getHeapCodeStatistics() { - const buffer = heapCodeStatisticsBuffer; + const buffer = binding.heapCodeStatisticsBuffer; updateHeapCodeStatisticsBuffer(); return { diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 134685542d4a75..53011c5fe0af25 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -198,6 +198,11 @@ class AliasedBufferBase { return js_array_.Get(isolate_); } + void Release() { + DCHECK_NULL(index_); + js_array_.Reset(); + } + /** * Get the underlying v8::ArrayBuffer underlying the TypedArray and * overlaying the native buffer diff --git a/src/base_object.h b/src/base_object.h index 3eac795e783e9d..6482bd85f86a56 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -158,6 +158,9 @@ class BaseObject : public MemoryRetainer { virtual inline void OnGCCollect(); + bool is_snapshotable() const { return is_snapshotable_; } + void set_is_snapshotable(bool val) { is_snapshotable_ = val; } + private: v8::Local WrappedObject() const override; bool IsRootNode() const override; @@ -206,6 +209,7 @@ class BaseObject : public MemoryRetainer { Environment* env_; PointerData* pointer_data_ = nullptr; + bool is_snapshotable_ = false; }; // Global alias for FromJSObject() to avoid churn. diff --git a/src/env-inl.h b/src/env-inl.h index 850363af5aeffb..86c870ca55f047 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -1085,6 +1085,17 @@ void Environment::ForEachBaseObject(T&& iterator) { } } +template +void Environment::ForEachBindingData(T&& iterator) { + BindingDataStore* map = static_cast( + context()->GetAlignedPointerFromEmbedderData( + ContextEmbedderIndex::kBindingListIndex)); + DCHECK_NOT_NULL(map); + for (auto& it : *map) { + iterator(it.first, it.second); + } +} + void Environment::modify_base_object_count(int64_t delta) { base_object_count_ += delta; } diff --git a/src/env.cc b/src/env.cc index fe774b4bf3183b..6ece645185b884 100644 --- a/src/env.cc +++ b/src/env.cc @@ -8,9 +8,11 @@ #include "node_buffer.h" #include "node_context_data.h" #include "node_errors.h" +#include "node_file.h" #include "node_internals.h" #include "node_options-inl.h" #include "node_process.h" +#include "node_v8.h" #include "node_v8_platform-inl.h" #include "node_worker.h" #include "req_wrap-inl.h" @@ -1249,6 +1251,7 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) { EnvSerializeInfo info; Local ctx = context(); + SerializeBindingData(this, creator, &info); // Currently all modules are compiled without cache in builtin snapshot // builder. info.native_modules = std::vector( @@ -1315,6 +1318,9 @@ std::ostream& operator<<(std::ostream& output, std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) { output << "{\n" + << "// -- bindings begins --\n" + << i.bindings << ",\n" + << "// -- bindings ends --\n" << "// -- native_modules begins --\n" << i.native_modules << ",\n" << "// -- native_modules ends --\n" @@ -1340,9 +1346,29 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) { return output; } +void Environment::EnqueueDeserializeRequest(DeserializeRequest request) { + deserialize_requests_.push_back(std::move(request)); +} + +void Environment::RunDeserializeRequests() { + HandleScope scope(isolate()); + Local ctx = context(); + Isolate* is = isolate(); + while (!deserialize_requests_.empty()) { + DeserializeRequest request(std::move(deserialize_requests_.front())); + deserialize_requests_.pop_front(); + Local holder = request.holder.Get(is); + request.cb(ctx, holder, request.info); + request.holder.Reset(); // unnecessary? + request.info->Delete(); + } +} + void Environment::DeserializeProperties(const EnvSerializeInfo* info) { Local ctx = context(); + RunDeserializeRequests(); + native_modules_in_snapshot = info->native_modules; async_hooks_.Deserialize(ctx); immediate_info_.Deserialize(ctx); diff --git a/src/env.h b/src/env.h index e418983a06663c..58eaee56d3cc9a 100644 --- a/src/env.h +++ b/src/env.h @@ -37,6 +37,7 @@ #include "node_main_instance.h" #include "node_options.h" #include "node_perf_common.h" +#include "node_snapshotable.h" #include "req_wrap.h" #include "util.h" #include "uv.h" @@ -936,8 +937,22 @@ struct PropInfo { SnapshotIndex index; // In the snapshot }; +typedef void (*DeserializeRequestCallback)(v8::Local, + v8::Local holder, + InternalFieldInfo* info); +struct DeserializeRequest { + DeserializeRequestCallback cb; + v8::Global holder; + InternalFieldInfo* info; // Owned by the request + + // Move constructor + DeserializeRequest(DeserializeRequest&& other) = default; +}; + struct EnvSerializeInfo { + std::vector bindings; std::vector native_modules; + AsyncHooks::SerializeInfo async_hooks; TickInfo::SerializeInfo tick_info; ImmediateInfo::SerializeInfo immediate_info; @@ -971,6 +986,8 @@ class Environment : public MemoryRetainer { void PrintAllBaseObjects(); void VerifyNoStrongBaseObjects(); + void EnqueueDeserializeRequest(DeserializeRequest request); + void RunDeserializeRequests(); // Should be called before InitializeInspector() void InitializeDiagnostics(); @@ -1408,6 +1425,9 @@ class Environment : public MemoryRetainer { void AddUnmanagedFd(int fd); void RemoveUnmanagedFd(int fd); + template + void ForEachBindingData(T&& iterator); + private: inline void ThrowError(v8::Local (*fun)(v8::Local), const char* errmsg); @@ -1496,6 +1516,8 @@ class Environment : public MemoryRetainer { bool is_in_inspector_console_call_ = false; #endif + std::list deserialize_requests_; + // handle_wrap_queue_ and req_wrap_queue_ needs to be at a fixed offset from // the start of the class because it is used by // src/node_postmortem_metadata.cc to calculate offsets and generate debug diff --git a/src/node_dir.cc b/src/node_dir.cc index a8bb2a7083c4fc..983ccd47138ab8 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -1,4 +1,5 @@ #include "node_dir.h" +#include "node_external_reference.h" #include "node_file-inl.h" #include "node_process.h" #include "memory_tracker-inl.h" @@ -362,8 +363,16 @@ void Initialize(Local target, env->set_dir_instance_template(dirt); } +void RegisterExternalReferences(ExternalReferenceRegistry* registry) { + registry->Register(OpenDir); + registry->Register(DirHandle::New); + registry->Register(DirHandle::Read); + registry->Register(DirHandle::Close); +} + } // namespace fs_dir } // end namespace node NODE_MODULE_CONTEXT_AWARE_INTERNAL(fs_dir, node::fs_dir::Initialize) +NODE_MODULE_EXTERNAL_REFERENCE(fs_dir, node::fs_dir::RegisterExternalReferences) diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 0544979dd9a6f1..d94c0fd654f65e 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -53,6 +53,8 @@ class ExternalReferenceRegistry { V(credentials) \ V(env_var) \ V(errors) \ + V(fs) \ + V(fs_dir) \ V(handle_wrap) \ V(messaging) \ V(native_module) \ @@ -65,6 +67,7 @@ class ExternalReferenceRegistry { V(trace_events) \ V(timers) \ V(types) \ + V(v8) \ V(worker) #if NODE_HAVE_I18N_SUPPORT diff --git a/src/node_file.cc b/src/node_file.cc index 46b328b50d0b14..b6efe9944acc28 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -19,10 +19,11 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "node_file.h" // NOLINT(build/include_inline) -#include "node_file-inl.h" #include "aliased_buffer.h" #include "memory_tracker-inl.h" #include "node_buffer.h" +#include "node_external_reference.h" +#include "node_file-inl.h" #include "node_process.h" #include "node_stat_watcher.h" #include "util-inl.h" @@ -2396,6 +2397,44 @@ void BindingData::MemoryInfo(MemoryTracker* tracker) const { file_handle_read_wrap_freelist); } +BindingData::BindingData(Environment* env, v8::Local wrap) + : SnapshotableObject(env, wrap, type_int), + stats_field_array(env->isolate(), kFsStatsBufferLength), + stats_field_bigint_array(env->isolate(), kFsStatsBufferLength) { + wrap->Set(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "statValues"), + stats_field_array.GetJSArray()) + .Check(); + + wrap->Set(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "bigintStatValues"), + stats_field_bigint_array.GetJSArray()) + .Check(); +} + +void BindingData::Deserialize(Local context, + Local holder, + InternalFieldInfo* info) { + HandleScope scope(context->GetIsolate()); + Environment* env = Environment::GetCurrent(context); + BindingData* binding = env->AddBindingData(context, holder); + CHECK_NOT_NULL(binding); +} + +void BindingData::PrepareForSerialization(Local context, + v8::SnapshotCreator* creator) { + CHECK(file_handle_read_wrap_freelist.empty()); + // We'll just re-initialize the buffers in the constructor since their + // contents can be thrown away once consumed in the previous call. + stats_field_array.Release(); + stats_field_bigint_array.Release(); +} + +InternalFieldInfo* BindingData::Serialize() { + InternalFieldInfo* info = InternalFieldInfo::New(type()); + return info; +} + // TODO(addaleax): Remove once we're on C++17. constexpr FastStringKey BindingData::type_name; @@ -2459,14 +2498,6 @@ void Initialize(Local target, static_cast(FsStatsOffset::kFsStatsFieldsNumber))) .Check(); - target->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "statValues"), - binding_data->stats_field_array.GetJSArray()).Check(); - - target->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "bigintStatValues"), - binding_data->stats_field_bigint_array.GetJSArray()).Check(); - StatWatcher::Initialize(env, target); // Create FunctionTemplate for FSReqCallback @@ -2530,8 +2561,62 @@ void Initialize(Local target, BindingData* FSReqBase::binding_data() { return binding_data_.get(); } + +void RegisterExternalReferences(ExternalReferenceRegistry* registry) { + registry->Register(Access); + StatWatcher::RegisterExternalReferences(registry); + + registry->Register(Close); + registry->Register(Open); + registry->Register(OpenFileHandle); + registry->Register(Read); + registry->Register(ReadBuffers); + registry->Register(Fdatasync); + registry->Register(Fsync); + registry->Register(Rename); + registry->Register(FTruncate); + registry->Register(RMDir); + registry->Register(MKDir); + registry->Register(ReadDir); + registry->Register(InternalModuleReadJSON); + registry->Register(InternalModuleStat); + registry->Register(Stat); + registry->Register(LStat); + registry->Register(FStat); + registry->Register(Link); + registry->Register(Symlink); + registry->Register(ReadLink); + registry->Register(Unlink); + registry->Register(WriteBuffer); + registry->Register(WriteBuffers); + registry->Register(WriteString); + registry->Register(RealPath); + registry->Register(CopyFile); + + registry->Register(Chmod); + registry->Register(FChmod); + // registry->Register(LChmod); + + registry->Register(Chown); + registry->Register(FChown); + registry->Register(LChown); + + registry->Register(UTimes); + registry->Register(FUTimes); + registry->Register(LUTimes); + + registry->Register(Mkdtemp); + registry->Register(NewFSReqCallback); + + registry->Register(FileHandle::New); + registry->Register(FileHandle::Close); + registry->Register(FileHandle::ReleaseFD); + StreamBase::RegisterExternalReferences(registry); +} + } // namespace fs } // end namespace node NODE_MODULE_CONTEXT_AWARE_INTERNAL(fs, node::fs::Initialize) +NODE_MODULE_EXTERNAL_REFERENCE(fs, node::fs::RegisterExternalReferences) diff --git a/src/node_file.h b/src/node_file.h index 9e652dc7a4afa4..f1515a3e25d6d1 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -3,23 +3,19 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "node.h" #include "aliased_buffer.h" #include "node_messaging.h" +#include "node_snapshotable.h" #include "stream_base.h" -#include namespace node { namespace fs { class FileHandleReadWrap; -class BindingData : public BaseObject { +class BindingData : public SnapshotableObject { public: - explicit BindingData(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - stats_field_array(env->isolate(), kFsStatsBufferLength), - stats_field_bigint_array(env->isolate(), kFsStatsBufferLength) {} + explicit BindingData(Environment* env, v8::Local wrap); AliasedFloat64Array stats_field_array; AliasedBigUint64Array stats_field_bigint_array; @@ -27,7 +23,10 @@ class BindingData : public BaseObject { std::vector> file_handle_read_wrap_freelist; - static constexpr FastStringKey type_name { "fs" }; + SERIALIZABLE_OBJECT_METHODS() + static constexpr FastStringKey type_name{"node::fs::BindingData"}; + static constexpr EmbedderObjectType type_int = + EmbedderObjectType::k_fs_binding_data; void MemoryInfo(MemoryTracker* tracker) const override; SET_SELF_SIZE(BindingData) diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index 21de10868564d3..480fb5788ccb95 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -1,39 +1,139 @@ #include "node_snapshotable.h" #include "base_object-inl.h" +#include "debug_utils-inl.h" +#include "node_file.h" +#include "node_v8.h" namespace node { using v8::Local; using v8::Object; +using v8::SnapshotCreator; using v8::StartupData; +SnapshotableObject::SnapshotableObject(Environment* env, + Local wrap, + EmbedderObjectType type) + : BaseObject(env, wrap), type_(type) { + set_is_snapshotable(true); +} + +const char* SnapshotableObject::GetTypeNameChars() const { + switch (type_) { +#define V(PropertyName, NativeTypeName) \ + case EmbedderObjectType::k_##PropertyName: { \ + return NativeTypeName::type_name.c_str(); \ + } + SERIALIZABLE_OBJECT_TYPES(V) +#undef V + default: { UNREACHABLE(); } + } +} + void DeserializeNodeInternalFields(Local holder, int index, - v8::StartupData payload, + StartupData payload, void* env) { + per_process::Debug(DebugCategory::MKSNAPSHOT, + "Deserialize internal field %d of %p, size=%d\n", + static_cast(index), + (*holder), + static_cast(payload.raw_size)); if (payload.raw_size == 0) { holder->SetAlignedPointerInInternalField(index, nullptr); return; } - // No embedder object in the builtin snapshot yet. - UNREACHABLE(); + + // TODO(joyeecheung): here we assume that + // 1. the holder is a BaseObject + // 2. the slot is the and BaseObject::kSlot and we are reviving + // the BaseObject reference + // 3. the payload contains a InternalFieldInfo whose type indicates + // the type of the BaseObject. + // Which may be expanded if we want to support non-BaseObject types + // or types with more slots. + CHECK_EQ(index, BaseObject::kSlot); + Environment* env_ptr = static_cast(env); + const InternalFieldInfo* info = + reinterpret_cast(payload.data); + + switch (info->type) { +#define V(PropertyName, NativeTypeName) \ + case EmbedderObjectType::k_##PropertyName: { \ + per_process::Debug(DebugCategory::MKSNAPSHOT, \ + "Object %p is %s\n", \ + (*holder), \ + NativeTypeName::type_name.c_str()); \ + env_ptr->EnqueueDeserializeRequest({NativeTypeName::Deserialize, \ + {env_ptr->isolate(), holder}, \ + info->Copy()}); \ + break; \ + } + SERIALIZABLE_OBJECT_TYPES(V) +#undef V + default: { UNREACHABLE(); } + } } StartupData SerializeNodeContextInternalFields(Local holder, int index, void* env) { + per_process::Debug(DebugCategory::MKSNAPSHOT, + "Serialize internal field, index=%d, holder=%p\n", + static_cast(index), + *holder); void* ptr = holder->GetAlignedPointerFromInternalField(index); - if (ptr == nullptr || ptr == env) { - return StartupData{nullptr, 0}; - } - if (ptr == env && index == ContextEmbedderIndex::kEnvironment) { + if (ptr == nullptr) { return StartupData{nullptr, 0}; } - // No embedder objects in the builtin snapshot yet. - UNREACHABLE(); - return StartupData{nullptr, 0}; + // For now, we just assume that we can only deal with kSlot fields of + // BaseObjects. + // TODO(joyeecheung): add more types for other objects with embedder fields. + CHECK_EQ(index, BaseObject::kSlot); + + DCHECK(static_cast(ptr)->is_snapshotable()); + SnapshotableObject* obj = static_cast(ptr); + per_process::Debug(DebugCategory::MKSNAPSHOT, + "Object %p is %s, ", + *holder, + obj->GetTypeNameChars()); + InternalFieldInfo* info = obj->Serialize(); + per_process::Debug(DebugCategory::MKSNAPSHOT, + "payload size=%d\n", + static_cast(info->length)); + return StartupData{reinterpret_cast(info), + static_cast(info->length)}; +} + +void SerializeBindingData(Environment* env, + SnapshotCreator* creator, + EnvSerializeInfo* info) { + size_t i = 0; + env->ForEachBindingData([&](FastStringKey key, + BaseObjectPtr binding) { + per_process::Debug(DebugCategory::MKSNAPSHOT, + "Serialize binding %i, %p, type=%s\n", + static_cast(i), + *(binding->object()), + key.c_str()); + + if (key == fs::BindingData::type_name || + key == v8_utils::BindingData::type_name) { + size_t index = creator->AddData(env->context(), binding->object()); + per_process::Debug(DebugCategory::MKSNAPSHOT, + "Serialized with index=%d\n", + static_cast(index)); + info->bindings.push_back({key.c_str(), i, index}); + SnapshotableObject* ptr = static_cast(binding.get()); + ptr->PrepareForSerialization(env->context(), creator); + } else { + UNREACHABLE(); + } + + i++; + }); } } // namespace node diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h index c45c1381b5554f..c1ab48ac6ba9c2 100644 --- a/src/node_snapshotable.h +++ b/src/node_snapshotable.h @@ -4,9 +4,92 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "v8.h" +#include "base_object.h" + namespace node { +class Environment; +struct EnvSerializeInfo; + +#define SERIALIZABLE_OBJECT_TYPES(V) \ + V(fs_binding_data, fs::BindingData) \ + V(v8_binding_data, v8_utils::BindingData) + +enum class EmbedderObjectType : uint8_t { + k_default = 0, +#define V(PropertyName, NativeType) k_##PropertyName, + SERIALIZABLE_OBJECT_TYPES(V) +#undef V +}; + +// When serializing an embedder object, we'll serialize the native states +// into a chunk that can be mapped into a subclass of InternalFieldInfo, +// and pass it into the V8 callback as the payload of StartupData. +// TODO(joyeecheung): the classification of types seem to be wrong. +// We'd need a type for each field of each class of native object. +// Maybe it's fine - we'll just use the type to invoke BaseObject constructors +// and specify that the BaseObject has only one field for us to serialize. +// And for non-BaseObject embedder objects, we'll use field-wise types. +// The memory chunk looks like this: +// +// [ type ] - EmbedderObjectType (a uint8_t) +// [ length ] - a size_t +// [ ... ] - custom bytes of size |length - header size| +struct InternalFieldInfo { + EmbedderObjectType type; + size_t length; + + InternalFieldInfo() = delete; + + static InternalFieldInfo* New(EmbedderObjectType type) { + return New(type, sizeof(InternalFieldInfo)); + } + + static InternalFieldInfo* New(EmbedderObjectType type, size_t length) { + InternalFieldInfo* result = + reinterpret_cast(::operator new(length)); + result->type = type; + result->length = length; + return result; + } + + InternalFieldInfo* Copy() const { + InternalFieldInfo* result = + reinterpret_cast(::operator new(length)); + memcpy(result, this, length); + return result; + } + + void Delete() { ::operator delete(this); } +}; + +class SnapshotableObject : public BaseObject { + public: + SnapshotableObject(Environment* env, + v8::Local wrap, + EmbedderObjectType type = EmbedderObjectType::k_default); + const char* GetTypeNameChars() const; + + virtual void PrepareForSerialization(v8::Local context, + v8::SnapshotCreator* creator) = 0; + virtual InternalFieldInfo* Serialize() = 0; + // We'll make sure that the type is set in the constructor + EmbedderObjectType type() { return type_; } + + private: + EmbedderObjectType type_; +}; + +// TODO(joyeecheung): to deal with multi-slot embedder objects, Serialize() +// and Deserialize() can take an index as argument. +#define SERIALIZABLE_OBJECT_METHODS() \ + void PrepareForSerialization(v8::Local context, \ + v8::SnapshotCreator* creator) override; \ + InternalFieldInfo* Serialize() override; \ + static void Deserialize(v8::Local context, \ + v8::Local holder, \ + InternalFieldInfo* info); + v8::StartupData SerializeNodeContextInternalFields(v8::Local holder, int index, void* env); @@ -14,6 +97,9 @@ void DeserializeNodeInternalFields(v8::Local holder, int index, v8::StartupData payload, void* env); +void SerializeBindingData(Environment* env, + v8::SnapshotCreator* creator, + EnvSerializeInfo* info); } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index 344ea6bb7ea2e6..b9f7903a2fdcb6 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -19,10 +19,11 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -#include "memory_tracker-inl.h" #include "node_stat_watcher.h" #include "async_wrap-inl.h" #include "env-inl.h" +#include "memory_tracker-inl.h" +#include "node_external_reference.h" #include "node_file-inl.h" #include "util-inl.h" @@ -55,6 +56,11 @@ void StatWatcher::Initialize(Environment* env, Local target) { env->SetConstructorFunction(target, "StatWatcher", t); } +void StatWatcher::RegisterExternalReferences( + ExternalReferenceRegistry* registry) { + registry->Register(StatWatcher::New); + registry->Register(StatWatcher::Start); +} StatWatcher::StatWatcher(fs::BindingData* binding_data, Local wrap, diff --git a/src/node_stat_watcher.h b/src/node_stat_watcher.h index c1d6ccce69bdbb..7efd22fdfdb841 100644 --- a/src/node_stat_watcher.h +++ b/src/node_stat_watcher.h @@ -35,10 +35,12 @@ class BindingData; } class Environment; +class ExternalReferenceRegistry; class StatWatcher : public HandleWrap { public: static void Initialize(Environment* env, v8::Local target); + static void RegisterExternalReferences(ExternalReferenceRegistry* registry); protected: StatWatcher(fs::BindingData* binding_data, diff --git a/src/node_v8.cc b/src/node_v8.cc index 4354e1e1772d82..8e503bdc794ad8 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -24,6 +24,7 @@ #include "env-inl.h" #include "memory_tracker-inl.h" #include "node.h" +#include "node_external_reference.h" #include "util-inl.h" #include "v8.h" @@ -45,6 +46,7 @@ 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) \ @@ -85,7 +87,7 @@ static const size_t kHeapCodeStatisticsPropertiesCount = #undef V BindingData::BindingData(Environment* env, Local obj) - : BaseObject(env, obj), + : SnapshotableObject(env, obj, type_int), heap_statistics_buffer(env->isolate(), kHeapStatisticsPropertiesCount), heap_space_statistics_buffer(env->isolate(), kHeapSpaceStatisticsPropertiesCount), @@ -105,6 +107,29 @@ BindingData::BindingData(Environment* env, Local obj) .Check(); } +void BindingData::PrepareForSerialization(Local context, + v8::SnapshotCreator* creator) { + // We'll just re-initialize the buffers in the constructor since their + // contents can be thrown away once consumed in the previous call. + heap_statistics_buffer.Release(); + heap_space_statistics_buffer.Release(); + heap_code_statistics_buffer.Release(); +} + +void BindingData::Deserialize(Local context, + Local holder, + InternalFieldInfo* info) { + HandleScope scope(context->GetIsolate()); + Environment* env = Environment::GetCurrent(context); + BindingData* binding = env->AddBindingData(context, holder); + CHECK_NOT_NULL(binding); +} + +InternalFieldInfo* BindingData::Serialize() { + InternalFieldInfo* info = InternalFieldInfo::New(type()); + return info; +} + void BindingData::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("heap_statistics_buffer", heap_statistics_buffer); tracker->TrackField("heap_space_statistics_buffer", @@ -168,7 +193,6 @@ void SetFlagsFromString(const FunctionCallbackInfo& args) { V8::SetFlagsFromString(*flags, static_cast(flags.length())); } - void Initialize(Local target, Local unused, Local context, @@ -223,7 +247,16 @@ void Initialize(Local target, env->SetMethod(target, "setFlagsFromString", SetFlagsFromString); } +void RegisterExternalReferences(ExternalReferenceRegistry* registry) { + registry->Register(CachedDataVersionTag); + registry->Register(UpdateHeapStatisticsBuffer); + registry->Register(UpdateHeapCodeStatisticsBuffer); + registry->Register(UpdateHeapSpaceStatisticsBuffer); + registry->Register(SetFlagsFromString); +} + } // namespace v8_utils } // namespace node NODE_MODULE_CONTEXT_AWARE_INTERNAL(v8, node::v8_utils::Initialize) +NODE_MODULE_EXTERNAL_REFERENCE(v8, node::v8_utils::RegisterExternalReferences) diff --git a/src/node_v8.h b/src/node_v8.h index 745c6580b844f4..5de8cb3f9c643c 100644 --- a/src/node_v8.h +++ b/src/node_v8.h @@ -5,18 +5,23 @@ #include "aliased_buffer.h" #include "base_object.h" +#include "node_snapshotable.h" #include "util.h" #include "v8.h" namespace node { class Environment; +struct InternalFieldInfo; namespace v8_utils { -class BindingData : public BaseObject { +class BindingData : public SnapshotableObject { public: BindingData(Environment* env, v8::Local obj); + SERIALIZABLE_OBJECT_METHODS() static constexpr FastStringKey type_name{"node::v8::BindingData"}; + static constexpr EmbedderObjectType type_int = + EmbedderObjectType::k_v8_binding_data; AliasedFloat64Array heap_statistics_buffer; AliasedFloat64Array heap_space_statistics_buffer; diff --git a/src/stream_base.cc b/src/stream_base.cc index 87781efb0e8111..925dc3f152e550 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -3,11 +3,12 @@ #include "stream_wrap.h" #include "allocated_buffer-inl.h" +#include "env-inl.h" +#include "js_stream.h" #include "node.h" #include "node_buffer.h" #include "node_errors.h" -#include "env-inl.h" -#include "js_stream.h" +#include "node_external_reference.h" #include "string_bytes.h" #include "util-inl.h" #include "v8.h" @@ -423,6 +424,29 @@ void StreamBase::AddMethods(Environment* env, Local t) { &Value::IsFunction>); } +void StreamBase::RegisterExternalReferences( + ExternalReferenceRegistry* registry) { + registry->Register(GetFD); + registry->Register(GetExternal); + registry->Register(GetBytesRead); + registry->Register(GetBytesWritten); + registry->Register(JSMethod<&StreamBase::ReadStartJS>); + registry->Register(JSMethod<&StreamBase::ReadStopJS>); + registry->Register(JSMethod<&StreamBase::Shutdown>); + registry->Register(JSMethod<&StreamBase::UseUserBuffer>); + registry->Register(JSMethod<&StreamBase::Writev>); + registry->Register(JSMethod<&StreamBase::WriteBuffer>); + registry->Register(JSMethod<&StreamBase::WriteString>); + registry->Register(JSMethod<&StreamBase::WriteString>); + registry->Register(JSMethod<&StreamBase::WriteString>); + registry->Register(JSMethod<&StreamBase::WriteString>); + registry->Register( + BaseObject::InternalFieldGet); + registry->Register( + BaseObject::InternalFieldSet); +} + void StreamBase::GetFD(const FunctionCallbackInfo& args) { // Mimic implementation of StreamBase::GetFD() and UDPWrap::GetFD(). StreamBase* wrap = StreamBase::FromObject(args.This().As()); diff --git a/src/stream_base.h b/src/stream_base.h index a5680ba8860d49..e0da891501ac85 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -19,6 +19,7 @@ class ShutdownWrap; class WriteWrap; class StreamBase; class StreamResource; +class ExternalReferenceRegistry; struct StreamWriteResult { bool async; @@ -308,7 +309,7 @@ class StreamBase : public StreamResource { static void AddMethods(Environment* env, v8::Local target); - + static void RegisterExternalReferences(ExternalReferenceRegistry* registry); virtual bool IsAlive() = 0; virtual bool IsClosing() = 0; virtual bool IsIPCPipe(); diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 105bfb10866499..7b6ad21443bdf6 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -31,6 +31,7 @@ const expectedModules = new Set([ 'Internal Binding types', 'Internal Binding url', 'Internal Binding util', + 'Internal Binding v8', 'Internal Binding worker', 'NativeModule buffer', 'NativeModule events',