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: #42360 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
nodejs · Jul 31, 2022 · ca57563 · ca57563
1 parent 3438689
commit ca57563
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 95 deletions.
diff --git 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<size_t> isolate_data_indices;
   EnvSerializeInfo env_info;

diff --git a/src/node.cc b/src/node.cc
@@ -1185,28 +1185,19 @@ int Start(int argc, char** argv) {
   }
 
   {
-    Isolate::CreateParams params;
-    const std::vector<size_t>* 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(&params,
+    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
@@ -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<IsolateData>(isolate_, event_loop, platform, nullptr);
 
@@ -61,77 +60,81 @@ std::unique_ptr<NodeMainInstance> 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<std::string>& args,
-    const std::vector<std::string>& exec_args,
-    const std::vector<size_t>* per_isolate_data_indexes)
+NodeMainInstance::NodeMainInstance(const SnapshotData* snapshot_data,
+                                   uv_loop_t* event_loop,
+                                   MultiIsolatePlatform* platform,
+                                   const std::vector<std::string>& args,
+                                   const std::vector<std::string>& 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<Isolate::CreateParams>()),
+      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<intptr_t>& external_references =
         CollectExternalReferences();
-    params->external_references = external_references.data();
+    isolate_params_->external_references = external_references.data();
+    isolate_params_->snapshot_blob =
+        const_cast<v8::StartupData*>(&(snapshot_data->blob));
   }
 
   isolate_ = Isolate::Allocate();
   CHECK_NOT_NULL(isolate_);
   // 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<IsolateData>(isolate_,
-                                                event_loop,
-                                                platform,
-                                                array_buffer_allocator_.get(),
-                                                per_isolate_data_indexes);
+  isolate_data_ = std::make_unique<IsolateData>(
+      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<Environment, FreeEnvironment> 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<Environment, FreeEnvironment>
-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> context;
   DeleteFnPtr<Environment, FreeEnvironment> 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
@@ -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<std::string>& args,
-      const std::vector<std::string>& exec_args,
-      const std::vector<size_t>* per_isolate_data_indexes = nullptr);
+  NodeMainInstance(const SnapshotData* snapshot_data,
+                   uv_loop_t* event_loop,
+                   MultiIsolatePlatform* platform,
+                   const std::vector<std::string>& args,
+                   const std::vector<std::string>& 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<Environment, FreeEnvironment> 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<size_t>* GetIsolateDataIndices();
-  static v8::StartupData* GetEmbeddedSnapshotBlob();
-  static const EnvSerializeInfo* GetEnvSerializeInfo();
+  static const SnapshotData* GetEmbeddedSnapshotData();
   static const std::vector<intptr_t>& CollectExternalReferences();
 
   static const size_t kNodeContextIndex = 0;
@@ -93,8 +90,8 @@ class NodeMainInstance {
   v8::Isolate* isolate_;
   MultiIsolatePlatform* platform_;
   std::unique_ptr<IsolateData> isolate_data_;
-  bool owns_isolate_ = false;
-  bool deserialize_mode_ = false;
+  std::unique_ptr<v8::Isolate::CreateParams> isolate_params_;
+  const SnapshotData* snapshot_data_ = nullptr;
 };
 
 }  // namespace node

diff --git 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<size_t>* 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
@@ -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<size_t> 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<size_t>* 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
@@ -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