GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: Add useful V8 option section by nimit95 · Pull Request #32262 · nodejs/node

Conversation

@nimit95

@nimit95 nimit95 commented Mar 14, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss and ronag reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
nimit95 added 2 commits March 14, 2020 22:16
@nimit95
doc: add useful v8 option section
0090f54 
This adds new section for v8 options and --max-old-space-size
Fixes: nodejs#32252
@nimit95
doc: add useful v8 option section
5209db8 
Fixed a typo addition in doc/cli.md
Fixes: nodejs#32252
@nodejs-github-bot nodejs-github-bot added cli Issues and PRs related to the Node.js command-line interface. doc Issues and PRs related to Node.js documentation. labels Mar 14, 2020
mscdex
mscdex reviewed Mar 14, 2020
View reviewed changes
Comment thread doc/api/cli.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
mscdex
mscdex reviewed Mar 14, 2020
View reviewed changes
Comment thread doc/api/cli.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
mscdex
mscdex reviewed Mar 14, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated

These are v8 options and fall outside Node.js' responsibility.

V8 options that are allowed are:

@mscdex mscdex Mar 14, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think this should either be removed completely or at least reworded. Node does not restrict the V8 options that can be passed on the command line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sam-github reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@nimit95 nimit95 Mar 17, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

There is another place in the document mentioning the same thing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
jasnell
jasnell reviewed Mar 16, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated
Comment on lines +1396 to +1398
Sets the max memory size of V8 old memory section. As memory consumption approaches the limit, V8 will spend more time on garbage collection in an effort to free unused memory.

On a machine with 2GB of memory it suggested to set 1.5GB to leave some memory for other uses and avoid swapping.

@jasnell jasnell Mar 16, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
Sets the max memory size of V8 old memory section. As memory consumption approaches the limit, V8 will spend more time on garbage collection in an effort to free unused memory.
On a machine with 2GB of memory it suggested to set 1.5GB to leave some memory for other uses and avoid swapping.
Sets the max memory size of V8 old memory section. As memory
consumption approaches the limit, V8 will spend more time on
garbage collection in an effort to free unused memory.
On a machine with 2GB of memory it suggested to set 1.5GB to
leave some memory for other uses and avoid swapping.

nit: need to wrap lines at <= 80 chars

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sam-github
sam-github reviewed Mar 16, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated

## Useful v8 options

These are v8 options and fall outside Node.js' responsibility.

@sam-github sam-github Mar 16, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
These are v8 options and fall outside Node.js' responsibility.
V8 has CLI options. Any V8 CLI option that is provided to `node` will be passed on to V8 to handle. V8's options have _no stability guarantee_. The V8 team themselves don't consider them to be part of their formal API, and reserve the right to change them at any time. The Node.js team does not consider them covered by the Node.js stability guarantees. Many of the V8 options are of interest only to V8 developers. Despite this, there is a small set of V8 options that are widely applicable to Node.js, and they are documented here.

I think we should say something like the above.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jasnell, mmarchini, and nimit95 reacted with thumbs up emoji
All reactions
  • 👍 3 reactions
Comment thread doc/api/cli.md Outdated

V8 options that are allowed are:

### `--max-old-space-size`

@sam-github sam-github Mar 16, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
### `--max-old-space-size`
### `--max-old-space-size=SIZE` (in Mbytes)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nimit95 nimit95 changed the title Doc: Add useful v8 option section doc: Add useful v8 option section Mar 17, 2020
nimit95 and others added 2 commits March 17, 2020 11:22
@nimit95 @mscdex
Update doc/api/cli.md
2e86534 
Co-Authored-By: mscdex <[email protected]>
@nimit95 @mscdex
Update doc/api/cli.md
2f36d8d 
Co-Authored-By: mscdex <[email protected]>
@HarshithaKP

HarshithaKP commented Mar 17, 2020

Copy link
Copy Markdown
Member

according to me, explaning the gc behavior is not necessary while describing the heap size flag.

also, elaborate description about API stability can confuse people?

how about just this:
size of the javascript heap (tenured heap or old space), in MBs. (V8 options are not processed by Node.js, and are passed as is to V8)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nimit95
doc: add to cli.md V8 options
0cf3b11
@nimit95

nimit95 commented Mar 17, 2020

Copy link
Copy Markdown
Contributor Author

Detailed about V8 because people can get confused about the behaviour of GC, we are explicitly mentioning that it is managed by V8 here. Also, it doesn't lie in node preview.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nimit95
doc: fix trailing whitespace in cli.md
030fe45 
Signed-off-by: Nimit <[email protected]>
mscdex
mscdex reviewed Mar 17, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated
and reserve the right to change them at any time. The Node.js team does not
consider them covered by the Node.js stability guarantees. Many of the V8
options are of interest only to V8 developers. Despite this, there is a small
set of V8 options that are widely applicable to the Node.js, and they are

@mscdex mscdex Mar 17, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
set of V8 options that are widely applicable to the Node.js, and they are
set of V8 options that are widely applicable to Node.js, and they are

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mscdex
mscdex reviewed Mar 17, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated
consider them covered by the Node.js stability guarantees. Many of the V8
options are of interest only to V8 developers. Despite this, there is a small
set of V8 options that are widely applicable to the Node.js, and they are
documented here. V8 options that are allowed are:

@mscdex mscdex Mar 17, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
documented here. V8 options that are allowed are:
documented here:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mscdex
mscdex reviewed Mar 17, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated
consumption approaches the limit, V8 will spend more time on
garbage collection in an effort to free unused memory.

On a machine with 2GB of memory it suggested to set 1.5GB

@mscdex mscdex Mar 17, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
On a machine with 2GB of memory it suggested to set 1.5GB
On a machine with 2GB of memory it is suggested to set this to 1536 (1.5GB)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mscdex
mscdex reviewed Mar 17, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated

### `--max-old-space-size=SIZE` (in Mbytes)

Sets the max memory size of V8 old memory section. As memory

@mscdex mscdex Mar 17, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
Sets the max memory size of V8 old memory section. As memory
Sets the max memory size of V8's old memory section. As memory

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nimit95
doc: update cli.md for fixes
6208729 
Signed-off-by: Nimit <[email protected]>
@mscdex

mscdex commented Mar 17, 2020

Copy link
Copy Markdown
Contributor

Just a note: when squashing/landing, make sure 'V8' is used instead of 'v8' in the commit message to avoid confusion.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nimit95 nimit95 changed the title doc: Add useful v8 option section doc: Add useful V8 option section Mar 18, 2020
@nimit95

nimit95 commented Mar 24, 2020

Copy link
Copy Markdown
Contributor Author

Anything to be done here from me?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax
addaleax approved these changes Mar 29, 2020
View reviewed changes
@addaleax addaleax added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Mar 29, 2020
Trott
Trott reviewed Mar 30, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated
Comment on lines +1393 to +1394
and reserve the right to change them at any time. The Node.js team does not
consider them covered by the Node.js stability guarantees. Many of the V8

@Trott Trott Mar 30, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Optional nit:

Suggested change
and reserve the right to change them at any time. The Node.js team does not
consider them covered by the Node.js stability guarantees. Many of the V8
and reserve the right to change them at any time. Likewise, they are not
covered by the Node.js stability guarantees. Many of the V8

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Trott
Trott reviewed Mar 30, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated
consumption approaches the limit, V8 will spend more time on
garbage collection in an effort to free unused memory.

On a machine with 2GB of memory it is suggested to set this to

@Trott Trott Mar 30, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Missing comma:

Suggested change
On a machine with 2GB of memory it is suggested to set this to
On a machine with 2GB of memory, it is suggested to set this to

Optional nit: I'm not a fan of "it is suggested" or "it is recommended" formulations. If possible, either tell people to do something or don't tell them at all. If you can't make a blanket statement, it might at least be a little more direct with something like "consider":

Suggested change
On a machine with 2GB of memory it is suggested to set this to
On a machine with 2GB of memory, consider setting this to

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@nimit95 nimit95 Apr 3, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Understood.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Trott
Trott reviewed Mar 30, 2020
View reviewed changes
Comment thread doc/api/cli.md Outdated
On a machine with 2GB of memory it is suggested to set this to
1536 (1.5GB) to leave some memory for other uses and avoid swapping.

E.g. `node --max-old-space-size=1536 index.js`

@Trott Trott Mar 30, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Optional nit: Delete this line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@nimit95 nimit95 Apr 3, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I personally feel an example helps a lot when being introduced to a flag. It clearly shows how to pass an argument. @Trott

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Trott Trott Apr 5, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I personally feel an example helps a lot when being introduced to a flag

If we want to provide an example, it should conform with the formatting of the other examples provided elsewhere in the doc. Otherwise, each entry is inventing its own example formatting, and we definitely do not want that. The existing format is more useful than this as it provides context, such as what the anticipated output might look like. So, this example might show the error one might get if running without the flag, then show the command succeeding with the flag.

(That said, most of our entries do not contain examples and that seems appropriate in this document.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@nimit95 nimit95 Apr 6, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Totally agree with your point, Can you point me to some of the correct existing formats?
What I see a mixture of formats, some given below

  1. node --name-of-flag index.mjs
  2. node --name-of-flag

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Trott Trott Apr 6, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't mean the format of the command itself. I mean the markdown formatting that denotes an example.

```console
$ node --cpu-prof index.js
$ ls *.cpuprofile
CPU.20190409.202950.15293.0.0.cpuprofile
```

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@nimit95 nimit95 Apr 8, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@Trott can you check out the formatting once now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@addaleax

addaleax commented Apr 2, 2020

Copy link
Copy Markdown
Member

@Trott do you want to apply your suggestions and merge this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nimit95

nimit95 commented Apr 2, 2020

Copy link
Copy Markdown
Contributor Author

Hey, will make the changes today. Totally forgot about this :)
addaleax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax addaleax removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Apr 2, 2020
@nimit95
fix language cli.md
fd93229 
Signed-off-by: Nimit <[email protected]>
@Trott

Trott commented Apr 5, 2020

Copy link
Copy Markdown
Member

My one remaining comment is a nit. In other words, it expresses my personal opinion, but I don't feel so strongly about it that I would block landing this if others disagree with my view. So if @addaleax or anyone else thinks this should land now as it is, go for it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nimit95
fix example formatting
aafe9fd 
Signed-off-by: Nimit <[email protected]>
gireeshpunathil
gireeshpunathil approved these changes Apr 11, 2020
View reviewed changes
gireeshpunathil pushed a commit that referenced this pull request Apr 11, 2020
@nimit95 @gireeshpunathil
doc: add useful v8 option section
df05b07 
This adds new section for v8 options and --max-old-space-size
Fixes: #32252

PR-URL: #32262
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@gireeshpunathil

gireeshpunathil commented Apr 11, 2020

Copy link
Copy Markdown
Member

landed in df05b07
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gireeshpunathil gireeshpunathil closed this Apr 11, 2020
targos pushed a commit that referenced this pull request Apr 13, 2020
@nimit95 @targos
doc: add useful v8 option section
66aafcf 
This adds new section for v8 options and --max-old-space-size
Fixes: #32252

PR-URL: #32262
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@targos targos mentioned this pull request Apr 13, 2020
v13.13.0 release proposal #32813
Merged
BethGriggs pushed a commit that referenced this pull request Apr 14, 2020
@nimit95 @BethGriggs
doc: add useful v8 option section
1894e51 
This adds new section for v8 options and --max-old-space-size
Fixes: #32252

PR-URL: #32262
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
targos pushed a commit that referenced this pull request Apr 22, 2020
@nimit95 @targos
doc: add useful v8 option section
00ce6a3 
This adds new section for v8 options and --max-old-space-size
Fixes: #32252

PR-URL: #32262
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@targos targos mentioned this pull request Apr 22, 2020
v12.16.3 release proposal #33009
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell left review comments
@Trott Trott Trott left review comments
@addaleax addaleax addaleax approved these changes
@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes
+2 more reviewers
@sam-github sam-github sam-github left review comments
@mscdex mscdex mscdex left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

cli Issues and PRs related to the Node.js command-line interface. doc Issues and PRs related to Node.js documentation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@nimit95 @HarshithaKP @mscdex @addaleax @Trott @gireeshpunathil @sam-github @jasnell @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.