-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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.
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
src: allow --interpreted-frames-native-stack in NODE_OPTIONS #27744
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
doc/api/cli.md
Outdated
@@ -932,6 +932,7 @@ V8 options that are allowed are: | |||
- `--perf-basic-prof-only-functions` | |||
- `--perf-prof` | |||
- `--perf-prof-unwinding-info` | |||
- `--interpreted-frames-native-stack` |
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.
The flags are currently in alphabetical order. Can you move this one to line 930 to maintain alphabetical listing? Or is it better that it be below --perf-*
flags?
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.
LGTM although I'd prefer the flag list be kept alphabetical in cli.md
. Not blocking on that though.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@mmarchini this needs a rebase. |
I'll rebase and address @Trott in the next few days. |
Small bump @mmarchini ? :) |
61e2daa
to
59fd2ba
Compare
59fd2ba
to
f078ea1
Compare
PR-URL: #27744 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Landed in 0150490 🎉 Thank you for the reviews and sorry for the delay! |
PR-URL: #27744 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #27744 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
We have
--perf-basic-prof
and family in NODE_OPTIONS, but these flags might not give the desired result without--interpreted-frames-native-stack
, thus adding this flag in NODE_OPTIONS as well makes sense.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes