Skip to content

src: increase default semi space size to 64mb #47277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

src: increase default semi space size to 64mb #47277

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,8 @@ static ExitCode InitializeNodeWithArgsInternal(
// is security relevant, for Node it's less important.
V8::SetFlagsFromString("--no-freeze-flags-after-init");

V8::SetFlagsFromString("--max_semi_space_size=64");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still possible to override it from the command line? Initialization is so spaghetti-like these days I have a hard time telling if this comes before or after V8::SetFlagsFromCommandLine().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes V8::SetFlagsFromCommandLine() is being called at ProcessGlobalArgsInternal which is called after this line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joyeecheung or @addaleax I think can validate if am correct

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just set this in the resource constraints in the isolate’s Isolate::CreateParams (https://v8.github.io/api/head/structv8_1_1Isolate_1_1CreateParams.html), which can be done in

node/src/api/environment.cc

Line 229 in fe449a2

void SetIsolateCreateParamsForNode(Isolate::CreateParams* params) {
, this allows us to no go over the system memory available and makes it possible for users to override it with a flag

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey so using the ResourceConstraints class we should set set_max_young_generation_size_in_bytes value then? to update the value on semi space? the doc says it includes two semi-spaces and a large object space. is the same as setting --max_semi_space_size=64 or will it be different?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried something like this

diff --git a/src/api/environment.cc b/src/api/environment.cc
index fd187f7342..409795b7a4 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -231,12 +231,14 @@ void SetIsolateCreateParamsForNode(Isolate::CreateParams* params) {
   const uint64_t total_memory = constrained_memory > 0 ?
       std::min(uv_get_total_memory(), constrained_memory) :
       uv_get_total_memory();
+  const uint64_t max_allowed_young_gen_size_in_bytes = 64 * 1024 * 1024;
   if (total_memory > 0 &&
       params->constraints.max_old_generation_size_in_bytes() == 0) {
     // V8 defaults to 700MB or 1.4GB on 32 and 64 bit platforms respectively.
     // This default is based on browser use-cases. Tell V8 to configure the
     // heap based on the actual physical memory.
     params->constraints.ConfigureDefaults(total_memory, 0);
+    params->constraints.set_max_young_generation_size_in_bytes(params->constraints.max_old_generation_size_in_bytes() / 64);
   }
   params->embedder_wrapper_object_index = BaseObject::InternalFields::kSlot;
   params->embedder_wrapper_type_index = std::numeric_limits<int>::max();
diff --git a/src/node.cc b/src/node.cc
index 7fcc60cf46..2aba7333d9 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -779,8 +779,6 @@ static ExitCode InitializeNodeWithArgsInternal(
   // is security relevant, for Node it's less important.
   V8::SetFlagsFromString("--no-freeze-flags-after-init");

The perf improvement seems to be the same, but cant understand whether its setting the semi size or size of the entire scavenger heap (i think?)


#if defined(NODE_V8_OPTIONS)
// Should come before the call to V8::SetFlagsFromCommandLine()
// so the user can disable a flag --foo at run-time by passing
Expand Down