doc: add new useful V8 option #42575

JialuZhang-intel · 2022-04-02T10:49:25Z

Add the '--max_semi_space_size' flag into useful V8 option.

Fixes: #42511

Add the '--max_semi_space_size' flag into useful V8 option. Fixes: nodejs#42511

doc/api/cli.md

Co-authored-by: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

doc/api/cli.md

Co-authored-by: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

doc/api/cli.md

aduh95 · 2022-04-02T14:24:25Z

doc/api/cli.md

+The actual throughput improvement and memory consumption
+are relevant to your application
+([reference](https://github.com/nodejs/node/issues/42511)).


I'm not sure I understand this sentence, do you mean it's relevant to any Node.js application, and if so, how do you know?

I tuned the max_semi_space_size value and tested it on two workloads, this is the results:

From the figure we can see that the trend of throughput improve are similar on both workloads, but the absolute value are different. I just want the user to test and choose the optimal configuration for their own application.

doc/api/cli.md

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

doc/api/cli.md

mhdawson · 2022-04-07T22:19:54Z

doc/api/cli.md

+
+_The default value is
+16MiB for 64-bit systems and 8MiB for 32-bit systems.
+The recommended value is 64MiB or 128MiB if your system has enough


I'm wondering if there a rational we can include for choosing these as the recommendation?

Actually, there isn't too much optional value for the 'max_semi_space_size', because V8 did the round up operation to make sure the 'max_semi_space_size' is the power of 2. So, only if we set the value of 'max_semi_space_size' to 16MiB, 32MiB, 64MiB, 128MiB, 256MiB and so on, the behavior is what we expected. I tuned these value on WebTooling and Ghost.js workloads, the result shows that 128MiB is the optimal configuration for both of these workloads.

I don't know if it is appropriate to put this image into the document. So I just put the link of issue #42511 in the document.

I'd refrain from recommending specific values, just that higher values can give better throughput but the reader should benchmark that.

It should also mention that the size is effectively tripled because there is more than one semi space. Observe:

$ for MB in 16 32 64 128; do node --max_semi_space_size=$MB --max_old_space_size=16 -p \ 'const MB = 1<<20; (v8.getHeapStatistics().heap_size_limit - 16*MB) / MB'; done 48 96 192 384

@bnoordhuis Thanks for your suggestion! I have updated the patch to guide users to choose the max_semi_space_size for themselves.

Co-authored-by: Michael Dawson <mdawson@devrus.com>

Add benchmark example to choose the best configuration instead.

lamweili

Hi @JialuZhang-intel, my humble 2 cents.
In addition, perhaps should use reference-style links instead of inline links.

doc/api/cli.md

More detailed explanation about memory consumption and semi-space size.

doc/api/cli.md

lamweili

For better clarity (from 0dfd5e5), maybe? 😅

doc/api/cli.md

lamweili

For consistency with cli.md, should use reference-style links instead of inline links.

Co-authored-by: Lam Wei Li <lam_wei_li@hotmail.com> Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

use reference-style links instead of inline links.

JialuZhang-intel · 2022-05-05T10:25:42Z

For consistency with cli.md, should use reference-style links instead of inline links.

@peteriman Thanks for your advice! I changed the links styles in this patch.
For the link of #42511, I think it's special for this option, so I kept it as inline.

JialuZhang-intel · 2022-06-09T03:39:33Z

It's been a long time and a lot of updates since this PR created. Thanks for the suggestions from all the reviewers @mhdawson @aduh95 @bnoordhuis @VoltrexMaster @peteriman @joyeecheung. Is there anything else need to fix about this PR?

bnoordhuis

Sorry for the delay, @JialuZhang-intel. Looks great!

doc/api/cli.md

Co-authored-by: James M Snell <jasnell@gmail.com>

doc/api/cli.md

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

Fix make lint errors in cli.md.

…luZhang-intel/node into document_max_semi_space_size

Add link for issue nodejs#42511.

mhdawson

LGTM

nodejs-github-bot · 2022-06-17T08:23:58Z

Commit Queue failed

- Loading data for nodejs/node/pull/42575
✔  Done loading data for nodejs/node/pull/42575
----------------------------------- PR info ------------------------------------
Title      doc: add new useful V8 option (#42575)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     JialuZhang-intel:document_max_semi_space_size -> nodejs:main
Labels     doc, cli, author ready, commit-queue-squash
Commits    21
 - doc: add new useful V8 option
 - Update doc/api/cli.md
 - Update doc/api/cli.md
 - Update doc/api/cli.md
 - Update doc/api/cli.md
 - Apply suggestions from code review
 - Update doc/api/cli.md
 - Remove recommended value for max_semi_space_size
 - Merge branch 'nodejs:master' into document_max_semi_space_size
 - doc: update format for cli.md
 - doc: add new useful V8 option
 - Apply suggestions from code review
 - doc: add new useful V8 option
 - doc: add new useful V8 option
 - Apply suggestions from code review
 - Update doc/api/cli.md
 - Update doc/api/cli.md
 - Merge branch 'nodejs:master' into document_max_semi_space_size
 - doc: add new useful V8 option
 - Merge branch 'document_max_semi_space_size' of https://github.com/Jia…
 - doc: add new useful V8 option
Committers 2
 - JialuZhang-intel 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/42575
Fixes: https://github.com/nodejs/node/issues/42511
Reviewed-By: Ben Noordhuis 
Reviewed-By: James M Snell 
Reviewed-By: Michael Dawson 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/42575
Fixes: https://github.com/nodejs/node/issues/42511
Reviewed-By: Ben Noordhuis 
Reviewed-By: James M Snell 
Reviewed-By: Michael Dawson 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 02 Apr 2022 10:49:25 GMT
   ✔  Approvals: 3
   ✔  - Ben Noordhuis (@bnoordhuis): https://github.com/nodejs/node/pull/42575#pullrequestreview-1000860391
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/42575#pullrequestreview-1003513324
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/42575#pullrequestreview-1004903706
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
   7f195189f0..7fd4cf4673  main       -> origin/main
✔  origin/main is now up-to-date
main is out of sync with origin/main. Mismatched commits:
 - 7fd4cf4673 loader: make `require.resolve` throw for unknown builtin modules
--------------------------------------------------------------------------------
HEAD is now at 7fd4cf4673 loader: make `require.resolve` throw for unknown builtin modules
   ✔  Reset to origin/main
- Downloading patch for 42575
From https://github.com/nodejs/node
 * branch                  refs/pull/42575/merge -> FETCH_HEAD
✔  Fetched commits as b6c6cc70009d..5411215a072a
--------------------------------------------------------------------------------
Auto-merging doc/api/cli.md
[main 1fea61e77a] doc: add new useful V8 option
 Author: JialuZhang-intel 
 Date: Sat Apr 2 18:35:30 2022 +0800
 1 file changed, 16 insertions(+)
Auto-merging doc/api/cli.md
[main 1a6964161e] Update doc/api/cli.md
 Author: JialuZhang-intel 
 Date: Sat Apr 2 20:08:45 2022 +0800
 1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging doc/api/cli.md
[main 73a0b57a4b] Update doc/api/cli.md
 Author: JialuZhang-intel 
 Date: Sat Apr 2 20:09:00 2022 +0800
 1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging doc/api/cli.md
[main adc451fecd] Update doc/api/cli.md
 Author: JialuZhang-intel 
 Date: Sat Apr 2 20:09:15 2022 +0800
 1 file changed, 2 insertions(+), 1 deletion(-)
Auto-merging doc/api/cli.md
[main c6690802c2] Update doc/api/cli.md
 Author: JialuZhang-intel 
 Date: Sat Apr 2 20:17:06 2022 +0800
 1 file changed, 3 insertions(+), 2 deletions(-)
Auto-merging doc/api/cli.md
[main 2cd6076c8c] Apply suggestions from code review
 Author: JialuZhang-intel 
 Date: Sun Apr 3 13:06:22 2022 +0800
 1 file changed, 8 insertions(+), 11 deletions(-)
Auto-merging doc/api/cli.md
[main fba60b1b8e] Update doc/api/cli.md
 Author: JialuZhang-intel 
 Date: Fri Apr 8 10:15:09 2022 +0800
 1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging doc/api/cli.md
error: commit d1defe3c16419451925e4e4f672a279cbcf1185d is a merge but no -m option was given.
fatal: cherry-pick failed
[main 7de3968c3a] Remove recommended value for max_semi_space_size
 Author: JialuZhang-intel 
 Date: Thu May 5 10:58:55 2022 +0800
 1 file changed, 15 insertions(+), 6 deletions(-)
   ✖  Failed to apply patches

https://github.com/nodejs/node/actions/runs/2514254971

aduh95 · 2022-06-17T08:47:33Z

Landed in f7770ed

lamweili

Thanks!

Add the `--max_semi_space_size` flag into useful V8 option. Fixes: #42511 PR-URL: #42575 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>

Add the `--max_semi_space_size` flag into useful V8 option. Fixes: nodejs/node#42511 PR-URL: nodejs/node#42575 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>

joebowbeer · 2024-11-05T05:55:15Z

@JialuZhang-intel the default max-semi-space-size is now (v20+) dependent on memory limit and is (more) likely to be sub-optimal for memory limits of 2GiB or less.

See #55487

doc: add new useful V8 option

a28ef1b

Add the '--max_semi_space_size' flag into useful V8 option. Fixes: nodejs#42511

nodejs-github-bot added cli Issues and PRs related to the Node.js command line interface. doc Issues and PRs related to the documentations. labels Apr 2, 2022

JialuZhang-intel mentioned this pull request Apr 2, 2022

Increase default 'max_semi_space_size' value to reduce GC overhead in V8 #42511

Closed

VoltrexKeyva reviewed Apr 2, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

doc/api/cli.md Outdated Show resolved Hide resolved

doc/api/cli.md Outdated Show resolved Hide resolved

JialuZhang-intel and others added 3 commits April 2, 2022 20:08

Update doc/api/cli.md

b960c66

Co-authored-by: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

Update doc/api/cli.md

3ae0f49

Co-authored-by: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

Update doc/api/cli.md

6813f99

Co-authored-by: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

VoltrexKeyva reviewed Apr 2, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

Update doc/api/cli.md

7b1739c

Co-authored-by: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

aduh95 reviewed Apr 2, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

Apply suggestions from code review

dddff28

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

JialuZhang-intel requested review from aduh95 and VoltrexKeyva April 3, 2022 05:12

mhdawson reviewed Apr 7, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

mhdawson reviewed Apr 7, 2022

View reviewed changes

Update doc/api/cli.md

a05b327

Co-authored-by: Michael Dawson <mdawson@devrus.com>

JialuZhang-intel requested a review from mhdawson April 18, 2022 01:14

JialuZhang-intel added 3 commits May 5, 2022 10:58

Remove recommended value for max_semi_space_size

eb1eec7

Add benchmark example to choose the best configuration instead.

Merge branch 'nodejs:master' into document_max_semi_space_size

d1defe3

doc: update format for cli.md

91a151a

lamweili reviewed May 5, 2022

View reviewed changes

doc: add new useful V8 option

0dfd5e5

More detailed explanation about memory consumption and semi-space size.

aduh95 reviewed May 5, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

lamweili reviewed May 5, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

doc/api/cli.md Outdated Show resolved Hide resolved

lamweili reviewed May 5, 2022

View reviewed changes

JialuZhang-intel and others added 2 commits May 5, 2022 17:34

Apply suggestions from code review

f91e536

Co-authored-by: Lam Wei Li <lam_wei_li@hotmail.com> Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

doc: add new useful V8 option

1bb2ad7

use reference-style links instead of inline links.

JialuZhang-intel requested review from bnoordhuis and aduh95 May 6, 2022 03:59

bnoordhuis approved these changes Jun 9, 2022

View reviewed changes

jasnell reviewed Jun 12, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

jasnell approved these changes Jun 12, 2022

View reviewed changes

Update doc/api/cli.md

4ef953f

Co-authored-by: James M Snell <jasnell@gmail.com>

aduh95 reviewed Jun 12, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

JialuZhang-intel and others added 5 commits June 13, 2022 08:53

Update doc/api/cli.md

5574419

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

Merge branch 'nodejs:master' into document_max_semi_space_size

1deaca4

doc: add new useful V8 option

db7d07d

Fix make lint errors in cli.md.

Merge branch 'document_max_semi_space_size' of https://github.com/Jia…

baa9c9b

…luZhang-intel/node into document_max_semi_space_size

doc: add new useful V8 option

5411215

Add link for issue nodejs#42511.

aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 13, 2022

mhdawson approved these changes Jun 13, 2022

View reviewed changes

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 17, 2022

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 17, 2022

aduh95 merged commit f7770ed into nodejs:main Jun 17, 2022

lamweili reviewed Jun 18, 2022

View reviewed changes

targos mentioned this pull request Jul 12, 2022

v18.6.0 release proposal #43789

Merged

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

ehoogeveen-medweb mentioned this pull request Aug 29, 2022

src: add --max-semi-space-size to the options allowed in NODE_OPTIONS #44436

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: add new useful V8 option #42575

doc: add new useful V8 option #42575

JialuZhang-intel commented Apr 2, 2022

aduh95 Apr 2, 2022

JialuZhang-intel Apr 3, 2022

mhdawson Apr 7, 2022

JialuZhang-intel Apr 8, 2022

bnoordhuis May 3, 2022

JialuZhang-intel May 5, 2022

lamweili left a comment

lamweili left a comment •

edited

Loading

lamweili left a comment

JialuZhang-intel commented May 5, 2022

JialuZhang-intel commented Jun 9, 2022 •

edited

Loading

bnoordhuis left a comment

mhdawson left a comment

nodejs-github-bot commented Jun 17, 2022

aduh95 commented Jun 17, 2022

lamweili left a comment

joebowbeer commented Nov 5, 2024 •

edited

Loading

doc: add new useful V8 option #42575

doc: add new useful V8 option #42575

Conversation

JialuZhang-intel commented Apr 2, 2022

aduh95 Apr 2, 2022

Choose a reason for hiding this comment

JialuZhang-intel Apr 3, 2022

Choose a reason for hiding this comment

mhdawson Apr 7, 2022

Choose a reason for hiding this comment

JialuZhang-intel Apr 8, 2022

Choose a reason for hiding this comment

bnoordhuis May 3, 2022

Choose a reason for hiding this comment

JialuZhang-intel May 5, 2022

Choose a reason for hiding this comment

lamweili left a comment

Choose a reason for hiding this comment

lamweili left a comment • edited Loading

Choose a reason for hiding this comment

lamweili left a comment

Choose a reason for hiding this comment

JialuZhang-intel commented May 5, 2022

JialuZhang-intel commented Jun 9, 2022 • edited Loading

bnoordhuis left a comment

Choose a reason for hiding this comment

mhdawson left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Jun 17, 2022

aduh95 commented Jun 17, 2022

lamweili left a comment

Choose a reason for hiding this comment

joebowbeer commented Nov 5, 2024 • edited Loading

lamweili left a comment •

edited

Loading

JialuZhang-intel commented Jun 9, 2022 •

edited

Loading

joebowbeer commented Nov 5, 2024 •

edited

Loading