From a12c4070e224621e88d4fd2d1ab1337447c5c137 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 15 Mar 2022 17:30:12 +0800 Subject: [PATCH] bootstrap: use SnapshotData to pass snapshot data around Instead of passing the snapshot blob, the per-isolate data indices and the EnvSerializeInfo separately, use the aggregate type Snapshot to carry these around, and refactor NodeMainInstance so that it owns the v8::Isolate::CreateParams when it owns its isolate. This also gets rid of the owns_isolate_ and deserialize_mode_ booleans in NodeMainInstance since NodeMainInstance can compute these by just checking if it has pointers to the CreateParams or the SnapshotData. PR-URL: https://github.com/nodejs/node/pull/42360 Reviewed-By: Anna Henningsen Reviewed-By: Darshan Sen --- src/env.h | 2 +- src/node.cc | 21 ++++-------- src/node_main_instance.cc | 71 ++++++++++++++++++++------------------- src/node_main_instance.h | 25 ++++++-------- src/node_snapshot_stub.cc | 10 +----- src/node_snapshotable.cc | 35 +++++++++---------- tools/snapshot/README.md | 3 +- 7 files changed, 72 insertions(+), 95 deletions(-) diff --git a/src/env.h b/src/env.h index cda7a52fa1ffc6..e0b078c8cb85c2 100644 --- a/src/env.h +++ b/src/env.h @@ -36,6 +36,7 @@ #include "node_binding.h" #include "node_external_reference.h" #include "node_main_instance.h" +#include "node_native_module.h" #include "node_options.h" #include "node_perf_common.h" #include "node_snapshotable.h" @@ -972,7 +973,6 @@ struct EnvSerializeInfo { }; struct SnapshotData { - SnapshotData() { blob.data = nullptr; } v8::StartupData blob; std::vector isolate_data_indices; EnvSerializeInfo env_info; diff --git a/src/node.cc b/src/node.cc index 64a910e3a283ad..e3249d8236cca7 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1145,28 +1145,19 @@ int Start(int argc, char** argv) { } { - Isolate::CreateParams params; - const std::vector* indices = nullptr; - const EnvSerializeInfo* env_info = nullptr; bool use_node_snapshot = per_process::cli_options->per_isolate->node_snapshot; - if (use_node_snapshot) { - v8::StartupData* blob = NodeMainInstance::GetEmbeddedSnapshotBlob(); - if (blob != nullptr) { - params.snapshot_blob = blob; - indices = NodeMainInstance::GetIsolateDataIndices(); - env_info = NodeMainInstance::GetEnvSerializeInfo(); - } - } + const SnapshotData* snapshot_data = + use_node_snapshot ? NodeMainInstance::GetEmbeddedSnapshotData() + : nullptr; uv_loop_configure(uv_default_loop(), UV_METRICS_IDLE_TIME); - NodeMainInstance main_instance(¶ms, + NodeMainInstance main_instance(snapshot_data, uv_default_loop(), per_process::v8_platform.Platform(), result.args, - result.exec_args, - indices); - result.exit_code = main_instance.Run(env_info); + result.exec_args); + result.exit_code = main_instance.Run(); } TearDownOncePerProcess(); diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index ce319cca3edca3..0d7c5d5a0ec8d4 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -36,8 +36,7 @@ NodeMainInstance::NodeMainInstance(Isolate* isolate, isolate_(isolate), platform_(platform), isolate_data_(nullptr), - owns_isolate_(false), - deserialize_mode_(false) { + snapshot_data_(nullptr) { isolate_data_ = std::make_unique(isolate_, event_loop, platform, nullptr); @@ -61,28 +60,28 @@ std::unique_ptr NodeMainInstance::Create( new NodeMainInstance(isolate, event_loop, platform, args, exec_args)); } -NodeMainInstance::NodeMainInstance( - Isolate::CreateParams* params, - uv_loop_t* event_loop, - MultiIsolatePlatform* platform, - const std::vector& args, - const std::vector& exec_args, - const std::vector* per_isolate_data_indexes) +NodeMainInstance::NodeMainInstance(const SnapshotData* snapshot_data, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform, + const std::vector& args, + const std::vector& exec_args) : args_(args), exec_args_(exec_args), array_buffer_allocator_(ArrayBufferAllocator::Create()), isolate_(nullptr), platform_(platform), - isolate_data_(nullptr), - owns_isolate_(true) { - params->array_buffer_allocator = array_buffer_allocator_.get(); - deserialize_mode_ = per_isolate_data_indexes != nullptr; - if (deserialize_mode_) { + isolate_data_(), + isolate_params_(std::make_unique()), + 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(); - params->external_references = external_references.data(); + isolate_params_->external_references = external_references.data(); + isolate_params_->snapshot_blob = + const_cast(&(snapshot_data->blob)); } isolate_ = Isolate::Allocate(); @@ -90,48 +89,52 @@ NodeMainInstance::NodeMainInstance( // Register the isolate on the platform before the isolate gets initialized, // so that the isolate can access the platform during initialization. platform->RegisterIsolate(isolate_, event_loop); - SetIsolateCreateParamsForNode(params); - Isolate::Initialize(isolate_, *params); + SetIsolateCreateParamsForNode(isolate_params_.get()); + Isolate::Initialize(isolate_, *isolate_params_); // If the indexes are not nullptr, we are not deserializing - CHECK_IMPLIES(deserialize_mode_, params->external_references != nullptr); - isolate_data_ = std::make_unique(isolate_, - event_loop, - platform, - array_buffer_allocator_.get(), - per_isolate_data_indexes); + isolate_data_ = std::make_unique( + isolate_, + event_loop, + platform, + array_buffer_allocator_.get(), + snapshot_data == nullptr ? nullptr + : &(snapshot_data->isolate_data_indices)); IsolateSettings s; SetIsolateMiscHandlers(isolate_, s); - if (!deserialize_mode_) { + if (snapshot_data == nullptr) { // If in deserialize mode, delay until after the deserialization is // complete. SetIsolateErrorHandlers(isolate_, s); } isolate_data_->max_young_gen_size = - params->constraints.max_young_generation_size_in_bytes(); + isolate_params_->constraints.max_young_generation_size_in_bytes(); } void NodeMainInstance::Dispose() { - CHECK(!owns_isolate_); + // This should only be called on a main instance that does not own its + // isolate. + CHECK_NULL(isolate_params_); platform_->DrainTasks(isolate_); } NodeMainInstance::~NodeMainInstance() { - if (!owns_isolate_) { + if (isolate_params_ == nullptr) { return; } + // This should only be done on a main instance that owns its isolate. platform_->UnregisterIsolate(isolate_); isolate_->Dispose(); } -int NodeMainInstance::Run(const EnvSerializeInfo* env_info) { +int NodeMainInstance::Run() { Locker locker(isolate_); Isolate::Scope isolate_scope(isolate_); HandleScope handle_scope(isolate_); int exit_code = 0; DeleteFnPtr env = - CreateMainEnvironment(&exit_code, env_info); + CreateMainEnvironment(&exit_code); CHECK_NOT_NULL(env); Context::Scope context_scope(env->context()); @@ -167,8 +170,7 @@ void NodeMainInstance::Run(int* exit_code, Environment* env) { } DeleteFnPtr -NodeMainInstance::CreateMainEnvironment(int* exit_code, - const EnvSerializeInfo* env_info) { +NodeMainInstance::CreateMainEnvironment(int* exit_code) { *exit_code = 0; // Reset the exit code to 0 HandleScope handle_scope(isolate_); @@ -179,16 +181,15 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code, isolate_->GetHeapProfiler()->StartTrackingHeapObjects(true); } - CHECK_IMPLIES(deserialize_mode_, env_info != nullptr); Local context; DeleteFnPtr env; - if (deserialize_mode_) { + if (snapshot_data_ != nullptr) { env.reset(new Environment(isolate_data_.get(), isolate_, args_, exec_args_, - env_info, + &(snapshot_data_->env_info), EnvironmentFlags::kDefaultFlags, {})); context = Context::FromSnapshot(isolate_, @@ -200,7 +201,7 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code, Context::Scope context_scope(context); CHECK(InitializeContextRuntime(context).IsJust()); SetIsolateErrorHandlers(isolate_, {}); - env->InitializeMainContext(context, env_info); + env->InitializeMainContext(context, &(snapshot_data_->env_info)); #if HAVE_INSPECTOR env->InitializeInspector({}); #endif diff --git a/src/node_main_instance.h b/src/node_main_instance.h index 047bdca873ebfd..fa4c4db0a886fb 100644 --- a/src/node_main_instance.h +++ b/src/node_main_instance.h @@ -15,6 +15,7 @@ namespace node { class ExternalReferenceRegistry; struct EnvSerializeInfo; +struct SnapshotData; // TODO(joyeecheung): align this with the Worker/WorkerThreadData class. // We may be able to create an abstract class to reuse some of the routines. @@ -48,29 +49,25 @@ class NodeMainInstance { void Dispose(); // Create a main instance that owns the isolate - NodeMainInstance( - v8::Isolate::CreateParams* params, - uv_loop_t* event_loop, - MultiIsolatePlatform* platform, - const std::vector& args, - const std::vector& exec_args, - const std::vector* per_isolate_data_indexes = nullptr); + NodeMainInstance(const SnapshotData* snapshot_data, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform, + const std::vector& args, + const std::vector& exec_args); ~NodeMainInstance(); // Start running the Node.js instances, return the exit code when finished. - int Run(const EnvSerializeInfo* env_info); + int Run(); void Run(int* exit_code, Environment* env); IsolateData* isolate_data() { return isolate_data_.get(); } DeleteFnPtr CreateMainEnvironment( - int* exit_code, const EnvSerializeInfo* env_info); + int* exit_code); // If nullptr is returned, the binary is not built with embedded // snapshot. - static const std::vector* GetIsolateDataIndices(); - static v8::StartupData* GetEmbeddedSnapshotBlob(); - static const EnvSerializeInfo* GetEnvSerializeInfo(); + static const SnapshotData* GetEmbeddedSnapshotData(); static const std::vector& CollectExternalReferences(); static const size_t kNodeContextIndex = 0; @@ -93,8 +90,8 @@ class NodeMainInstance { v8::Isolate* isolate_; MultiIsolatePlatform* platform_; std::unique_ptr isolate_data_; - bool owns_isolate_ = false; - bool deserialize_mode_ = false; + std::unique_ptr isolate_params_; + const SnapshotData* snapshot_data_ = nullptr; }; } // namespace node diff --git a/src/node_snapshot_stub.cc b/src/node_snapshot_stub.cc index 7c13d4e8c602c8..f167c46a68de22 100644 --- a/src/node_snapshot_stub.cc +++ b/src/node_snapshot_stub.cc @@ -6,15 +6,7 @@ namespace node { -v8::StartupData* NodeMainInstance::GetEmbeddedSnapshotBlob() { - return nullptr; -} - -const std::vector* NodeMainInstance::GetIsolateDataIndices() { - return nullptr; -} - -const EnvSerializeInfo* NodeMainInstance::GetEnvSerializeInfo() { +const SnapshotData* NodeMainInstance::GetEmbeddedSnapshotData() { return nullptr; } diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index 6f45ce537907cd..71b025df81eead 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -53,31 +53,28 @@ static const char blob_data[] = { static const int blob_size = )" << data->blob.raw_size << R"(; -static v8::StartupData blob = { blob_data, blob_size }; -)"; - - ss << R"(v8::StartupData* NodeMainInstance::GetEmbeddedSnapshotBlob() { - return &blob; -} -static const std::vector isolate_data_indices { +SnapshotData snapshot_data { + // -- blob begins -- + { blob_data, blob_size }, + // -- blob ends -- + // -- isolate_data_indices begins -- + { )"; WriteVector(&ss, data->isolate_data_indices.data(), data->isolate_data_indices.size()); - ss << R"(}; - -const std::vector* NodeMainInstance::GetIsolateDataIndices() { - return &isolate_data_indices; + ss << R"(}, + // -- isolate_data_indices ends -- + // -- env_info begins -- +)" << data->env_info + << R"( + // -- env_info ends -- +}; + +const SnapshotData* NodeMainInstance::GetEmbeddedSnapshotData() { + return &snapshot_data; } - -static const EnvSerializeInfo env_info )" - << data->env_info << R"(; - -const EnvSerializeInfo* NodeMainInstance::GetEnvSerializeInfo() { - return &env_info; -} - } // namespace node )"; diff --git a/tools/snapshot/README.md b/tools/snapshot/README.md index fb22c03ed50b88..5792ede4499f33 100644 --- a/tools/snapshot/README.md +++ b/tools/snapshot/README.md @@ -22,8 +22,7 @@ In the default build of the Node.js executable, to embed a V8 startup snapshot into the Node.js executable, `libnode` is first built with these unresolved symbols: -- `node::NodeMainInstance::GetEmbeddedSnapshotBlob` -- `node::NodeMainInstance::GetIsolateDataIndices` +- `node::NodeMainInstance::GetEmbeddedSnapshotData` Then the `node_mksnapshot` executable is built with C++ files in this directory, as well as `src/node_snapshot_stub.cc` which defines the unresolved