From 27bed94debd9768d8552a6ba0280c4939b146d30 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 12 Apr 2022 00:38:57 +0800 Subject: [PATCH] bootstrap: move embedded snapshot to SnapshotBuilder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So that the embedded snapshot can be reused by the worker. PR-URL: https://github.com/nodejs/node/pull/42702 Refs: https://github.com/nodejs/node/issues/35711 Reviewed-By: Chengzhong Wu Reviewed-By: Anna Henningsen Reviewed-By: Tobias Nießen Reviewed-By: James M Snell --- node.gyp | 1 + src/node.cc | 5 ++-- src/node_external_reference.cc | 8 +++--- src/node_main_instance.cc | 19 +++----------- src/node_main_instance.h | 5 ---- src/node_snapshot_builder.h | 43 +++++++++++++++++++++++++++++++ src/node_snapshot_stub.cc | 4 +-- src/node_snapshotable.cc | 23 ++++++++++++++--- src/node_snapshotable.h | 12 +-------- tools/snapshot/node_mksnapshot.cc | 2 +- 10 files changed, 78 insertions(+), 44 deletions(-) create mode 100644 src/node_snapshot_builder.h diff --git a/node.gyp b/node.gyp index 650db9543503f4..5e94c34e71686a 100644 --- a/node.gyp +++ b/node.gyp @@ -555,6 +555,7 @@ 'src/node_revert.h', 'src/node_root_certs.h', 'src/node_snapshotable.h', + 'src/node_snapshot_builder.h', 'src/node_sockaddr.h', 'src/node_sockaddr-inl.h', 'src/node_stat_watcher.h', diff --git a/src/node.cc b/src/node.cc index 12ff10b5e189a7..254dbe47069781 100644 --- a/src/node.cc +++ b/src/node.cc @@ -25,8 +25,8 @@ #include "debug_utils-inl.h" #include "env-inl.h" -#include "memory_tracker-inl.h" #include "histogram-inl.h" +#include "memory_tracker-inl.h" #include "node_binding.h" #include "node_errors.h" #include "node_internals.h" @@ -38,6 +38,7 @@ #include "node_process-inl.h" #include "node_report.h" #include "node_revert.h" +#include "node_snapshot_builder.h" #include "node_v8_platform-inl.h" #include "node_version.h" @@ -1158,7 +1159,7 @@ int Start(int argc, char** argv) { bool use_node_snapshot = per_process::cli_options->per_isolate->node_snapshot; const SnapshotData* snapshot_data = - use_node_snapshot ? NodeMainInstance::GetEmbeddedSnapshotData() + use_node_snapshot ? SnapshotBuilder::GetEmbeddedSnapshotData() : nullptr; uv_loop_configure(uv_default_loop(), UV_METRICS_IDLE_TIME); diff --git a/src/node_external_reference.cc b/src/node_external_reference.cc index 94198719b6a002..9a89977094bb55 100644 --- a/src/node_external_reference.cc +++ b/src/node_external_reference.cc @@ -7,9 +7,11 @@ namespace node { const std::vector& ExternalReferenceRegistry::external_references() { - CHECK(!is_finalized_); - external_references_.push_back(reinterpret_cast(nullptr)); - is_finalized_ = true; + if (!is_finalized_) { + external_references_.push_back(reinterpret_cast(nullptr)); + is_finalized_ = true; + } + return external_references_; } diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 0d7c5d5a0ec8d4..07dc4d88a7f8b4 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -4,6 +4,7 @@ #include "node_external_reference.h" #include "node_internals.h" #include "node_options-inl.h" +#include "node_snapshot_builder.h" #include "node_snapshotable.h" #include "node_v8_platform-inl.h" #include "util-inl.h" @@ -23,8 +24,6 @@ using v8::Isolate; using v8::Local; using v8::Locker; -std::unique_ptr NodeMainInstance::registry_ = - nullptr; NodeMainInstance::NodeMainInstance(Isolate* isolate, uv_loop_t* event_loop, MultiIsolatePlatform* platform, @@ -43,13 +42,6 @@ NodeMainInstance::NodeMainInstance(Isolate* isolate, SetIsolateMiscHandlers(isolate_, {}); } -const std::vector& NodeMainInstance::CollectExternalReferences() { - // Cannot be called more than once. - CHECK_NULL(registry_); - registry_.reset(new ExternalReferenceRegistry()); - return registry_->external_references(); -} - std::unique_ptr NodeMainInstance::Create( Isolate* isolate, uv_loop_t* event_loop, @@ -75,13 +67,8 @@ NodeMainInstance::NodeMainInstance(const SnapshotData* snapshot_data, snapshot_data_(snapshot_data) { isolate_params_->array_buffer_allocator = array_buffer_allocator_.get(); if (snapshot_data != nullptr) { - // TODO(joyeecheung): collect external references and set it in - // params.external_references. - const std::vector& external_references = - CollectExternalReferences(); - isolate_params_->external_references = external_references.data(); - isolate_params_->snapshot_blob = - const_cast(&(snapshot_data->blob)); + SnapshotBuilder::InitializeIsolateParams(snapshot_data, + isolate_params_.get()); } isolate_ = Isolate::Allocate(); diff --git a/src/node_main_instance.h b/src/node_main_instance.h index fa4c4db0a886fb..fc44bc9df6f030 100644 --- a/src/node_main_instance.h +++ b/src/node_main_instance.h @@ -65,11 +65,6 @@ class NodeMainInstance { DeleteFnPtr CreateMainEnvironment( int* exit_code); - // If nullptr is returned, the binary is not built with embedded - // snapshot. - static const SnapshotData* GetEmbeddedSnapshotData(); - static const std::vector& CollectExternalReferences(); - static const size_t kNodeContextIndex = 0; NodeMainInstance(const NodeMainInstance&) = delete; NodeMainInstance& operator=(const NodeMainInstance&) = delete; diff --git a/src/node_snapshot_builder.h b/src/node_snapshot_builder.h new file mode 100644 index 00000000000000..183c39bdd76219 --- /dev/null +++ b/src/node_snapshot_builder.h @@ -0,0 +1,43 @@ + +#ifndef SRC_NODE_SNAPSHOT_BUILDER_H_ +#define SRC_NODE_SNAPSHOT_BUILDER_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include +#include "node_mutex.h" +#include "v8.h" + +namespace node { + +class ExternalReferenceRegistry; +struct SnapshotData; + +class SnapshotBuilder { + public: + static std::string Generate(const std::vector args, + const std::vector exec_args); + + // Generate the snapshot into out. + static void Generate(SnapshotData* out, + const std::vector args, + const std::vector exec_args); + + // If nullptr is returned, the binary is not built with embedded + // snapshot. + static const SnapshotData* GetEmbeddedSnapshotData(); + static void InitializeIsolateParams(const SnapshotData* data, + v8::Isolate::CreateParams* params); + + private: + // Used to synchronize access to the snapshot data + static Mutex snapshot_data_mutex_; + static const std::vector& CollectExternalReferences(); + + static std::unique_ptr registry_; +}; +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_NODE_SNAPSHOT_BUILDER_H_ diff --git a/src/node_snapshot_stub.cc b/src/node_snapshot_stub.cc index f167c46a68de22..664f878c0671db 100644 --- a/src/node_snapshot_stub.cc +++ b/src/node_snapshot_stub.cc @@ -2,11 +2,11 @@ // NODE_WANT_INTERNALS, so we define it here manually. #define NODE_WANT_INTERNALS 1 -#include "node_main_instance.h" +#include "node_snapshot_builder.h" namespace node { -const SnapshotData* NodeMainInstance::GetEmbeddedSnapshotData() { +const SnapshotData* SnapshotBuilder::GetEmbeddedSnapshotData() { return nullptr; } diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index 9330da6568ce20..1938e1b143ae2e 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -12,6 +12,7 @@ #include "node_internals.h" #include "node_main_instance.h" #include "node_process.h" +#include "node_snapshot_builder.h" #include "node_v8.h" #include "node_v8_platform-inl.h" @@ -49,7 +50,7 @@ std::string FormatBlob(SnapshotData* data) { ss << R"(#include #include "env.h" -#include "node_main_instance.h" +#include "node_snapshot_builder.h" #include "v8.h" // This file is generated by tools/snapshot. Do not edit. @@ -78,11 +79,12 @@ SnapshotData snapshot_data { // -- isolate_data_indices ends -- // -- env_info begins -- )" << data->env_info - << R"( + << R"( // -- env_info ends -- }; -const SnapshotData* NodeMainInstance::GetEmbeddedSnapshotData() { +const SnapshotData* SnapshotBuilder::GetEmbeddedSnapshotData() { + Mutex::ScopedLock lock(snapshot_data_mutex_); return &snapshot_data; } } // namespace node @@ -91,6 +93,19 @@ const SnapshotData* NodeMainInstance::GetEmbeddedSnapshotData() { return ss.str(); } +Mutex SnapshotBuilder::snapshot_data_mutex_; + +const std::vector& SnapshotBuilder::CollectExternalReferences() { + static auto registry = std::make_unique(); + return registry->external_references(); +} + +void SnapshotBuilder::InitializeIsolateParams(const SnapshotData* data, + Isolate::CreateParams* params) { + params->external_references = CollectExternalReferences().data(); + params->snapshot_blob = const_cast(&(data->blob)); +} + void SnapshotBuilder::Generate(SnapshotData* out, const std::vector args, const std::vector exec_args) { @@ -104,7 +119,7 @@ void SnapshotBuilder::Generate(SnapshotData* out, { const std::vector& external_references = - NodeMainInstance::CollectExternalReferences(); + CollectExternalReferences(); SnapshotCreator creator(isolate, external_references.data()); Environment* env; { diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h index 30b684eb68a2d6..f0a8bce215e027 100644 --- a/src/node_snapshotable.h +++ b/src/node_snapshotable.h @@ -12,6 +12,7 @@ namespace node { class Environment; struct EnvSerializeInfo; struct SnapshotData; +class ExternalReferenceRegistry; #define SERIALIZABLE_OBJECT_TYPES(V) \ V(fs_binding_data, fs::BindingData) \ @@ -122,17 +123,6 @@ void SerializeBindingData(Environment* env, EnvSerializeInfo* info); bool IsSnapshotableType(FastStringKey key); - -class SnapshotBuilder { - public: - static std::string Generate(const std::vector args, - const std::vector exec_args); - - // Generate the snapshot into out. - static void Generate(SnapshotData* out, - const std::vector args, - const std::vector exec_args); -}; } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/tools/snapshot/node_mksnapshot.cc b/tools/snapshot/node_mksnapshot.cc index 60062854327868..d166559a715b14 100644 --- a/tools/snapshot/node_mksnapshot.cc +++ b/tools/snapshot/node_mksnapshot.cc @@ -7,7 +7,7 @@ #include "libplatform/libplatform.h" #include "node_internals.h" -#include "node_snapshotable.h" +#include "node_snapshot_builder.h" #include "util-inl.h" #include "v8.h"