Skip to content

Commit

Permalink
src: make --no-node-snapshot a per-process option
Browse files Browse the repository at this point in the history
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 <targos@protonmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
joyeecheung authored and targos committed May 2, 2022
1 parent 1102922 commit 37ca110
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 17 deletions.
3 changes: 1 addition & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 1 addition & 8 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 37ca110

Please sign in to comment.