-
Notifications
You must be signed in to change notification settings - Fork 30k
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
cli: --perf-prof only works on Linux #31892
Conversation
79375c3
to
3fba7b4
Compare
they (v8) did have a (documented?) promise that they would consider Node's compatibility as a major factor while implementing features? |
@gireeshpunathil i'm pretty sure they don't consider this an issue with compat considering Node.js was piping straight through to V8's own bespoke option; we also already gate some |
|
thanks @mmarchini 🙇♀ |
ok, thanks @mmarchini, that will really help! for the context, my use case is here: https://github.com/nodejs/diagnostics/tree/master/documentation/profiling - attempt to document diagnostic best practices around profiling. These are aimed at user-journey driven, endorsed by node project, and planned to be supported for long term. as that is a conversation with v8, and this PR is just documenting the code behavior that was uncovered in the field, it looks good to me! |
Landed in 72b6cea |
PR-URL: #31892 Reviewed-By: Matheus Marchini <mat@mmarchini.me> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: #31892 Reviewed-By: Matheus Marchini <mat@mmarchini.me> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: #31892 Reviewed-By: Matheus Marchini <mat@mmarchini.me> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: #31892 Reviewed-By: Matheus Marchini <mat@mmarchini.me> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: #31892 Reviewed-By: Matheus Marchini <mat@mmarchini.me> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
--perf-prof
-related flags have been removed in V8 on non-linux devices, and so we should note that in testing here as well as in docs.See:
This broke in Electron, which is how i discovered it.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes