src: make --no-node-snapshot a per-process option

We enable the shared read-only heap which currently requires that the snapshot used in different isolates in the same process to be the same. Therefore --no-node-snapshot is a per-process option. PR-URL: #42864 Refs: #42809 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
nodejs · Apr 28, 2022 · 9c60ae2 · 9c60ae2
1 parent 63503a4
commit 9c60ae2
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 17 deletions.
diff --git a/src/node.cc b/src/node.cc
@@ -1169,8 +1169,7 @@ int Start(int argc, char** argv) {
   }
 
   {
-    bool use_node_snapshot =
-        per_process::cli_options->per_isolate->node_snapshot;
+    bool use_node_snapshot = per_process::cli_options->node_snapshot;
     const SnapshotData* snapshot_data =
         use_node_snapshot ? SnapshotBuilder::GetEmbeddedSnapshotData()
                           : nullptr;

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -650,10 +650,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
             "track heap object allocations for heap snapshots",
             &PerIsolateOptions::track_heap_objects,
             kAllowedInEnvironment);
-  AddOption("--node-snapshot",
-            "",  // It's a debug-only option.
-            &PerIsolateOptions::node_snapshot,
-            kAllowedInEnvironment);
 
   // Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
   AddOption("--abort-on-uncaught-exception",
@@ -755,7 +751,10 @@ PerProcessOptionsParser::PerProcessOptionsParser(
             "Currently only supported in the node_mksnapshot binary.",
             &PerProcessOptions::build_snapshot,
             kDisallowedInEnvironment);
-
+  AddOption("--node-snapshot",
+            "",  // It's a debug-only option.
+            &PerProcessOptions::node_snapshot,
+            kAllowedInEnvironment);
   // 12.x renamed this inadvertently, so alias it for consistency within the
   // release line, while using the original name for consistency with older
   // release lines.

diff --git a/src/node_options.h b/src/node_options.h
@@ -203,7 +203,6 @@ class PerIsolateOptions : public Options {
  public:
   std::shared_ptr<EnvironmentOptions> per_env { new EnvironmentOptions() };
   bool track_heap_objects = false;
-  bool node_snapshot = true;
   bool report_uncaught_exception = false;
   bool report_on_signal = false;
   bool experimental_top_level_await = true;
@@ -231,7 +230,11 @@ class PerProcessOptions : public Options {
   bool zero_fill_all_buffers = false;
   bool debug_arraybuffer_allocations = false;
   std::string disable_proto;
-  bool build_snapshot;
+  bool build_snapshot = false;
+  // We enable the shared read-only heap which currently requires that the
+  // snapshot used in different isolates in the same process to be the same.
+  // Therefore --node-snapshot is a per-process option.
+  bool node_snapshot = true;
 
   std::vector<std::string> security_reverts;
   bool print_bash_completion = false;

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -147,14 +147,7 @@ class WorkerThreadData {
     SetIsolateCreateParamsForNode(&params);
     params.array_buffer_allocator_shared = allocator;
 
-    bool use_node_snapshot = true;
-    if (w_->per_isolate_opts_) {
-      use_node_snapshot = w_->per_isolate_opts_->node_snapshot;
-    } else {
-      // IsolateData is created after the Isolate is created so we'll
-      // inherit the option from the parent here.
-      use_node_snapshot = per_process::cli_options->per_isolate->node_snapshot;
-    }
+    bool use_node_snapshot = per_process::cli_options->node_snapshot;
     const SnapshotData* snapshot_data =
         use_node_snapshot ? SnapshotBuilder::GetEmbeddedSnapshotData()
                           : nullptr;