Skip to content
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

Improve developer experience for v8 performance related flags #43407

Closed
benjamingr opened this issue Jun 13, 2022 · 14 comments
Closed

Improve developer experience for v8 performance related flags #43407

benjamingr opened this issue Jun 13, 2022 · 14 comments
Labels
feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. performance Issues and PRs related to the performance of Node.js. stale v8 engine Issues and PRs related to the V8 dependency.

Comments

@benjamingr
Copy link
Member

benjamingr commented Jun 13, 2022

What is the problem this feature will solve?

Currently, it is very hard to use v8 flags like --print-bytecode or --log-all with Node.js because it contains a lot of information related to Node.js's bootstrap which makes reading the output significantly harder than without Node.js.

It would be really useful to be able to use v8 flags like --print-bytecode or --log-all in Node.js without having to navigate all the unrelated information. The V8 team created a lot of useful tools like https://v8.github.io/tools/head/ and it's not easy to use them in Node.js today.

What is the feature you are proposing to solve the problem?

I am proposing to somehow allow logging (--log-all) and other flags (--trace-opt --print-bytecode etc) to ignore Node.js internals code the same way --inspect-brk and other flags ignore Node internals.

This would allow users to observe the V8 compiler a lot more easily without being experts in the field.

What alternatives have you considered?

One can use the d8 shell which does not (obviously) bootstrap Node.js or another slimmer JavaScript runtime but those solutions require altering Node.js programs to achieve and thus do not provide as good performance data.


Ping @nodejs/v8 @addyosmani @bmeurer (since tooling and this can improve the experience for people needing to figure out why code is slow in Chrome).

Ping @nhelfman (is that the right GH?)

@benjamingr benjamingr added feature request Issues that request new features to be added to Node.js. v8 engine Issues and PRs related to the V8 dependency. performance Issues and PRs related to the performance of Node.js. labels Jun 13, 2022
@benjamingr
Copy link
Member Author

Maybe also cc @nodejs/diagnostics ? Not sure if that's a relevant ping

@benjamingr
Copy link
Member Author

Actually, I think this is already possible by using the V8 module, so I guess the ask here is for more docs or a nicer API since this is already possible?

require('v8').setFlagsFromString('--print-bytecode');
function foo(x) {
    return x + 1;
}

for(let i = 0; i < 100; i++) {
    foo(i);
}

What I didn't realize is that this flag can be turned on "dynamically".

@benjamingr
Copy link
Member Author

Let's try a "poll" and see what people think:

🚀 - add documentation to the V8 module docs explaining the use case and add a specific example under setFlagsFromString
👀 - add a new CLI flag --flags-after-start="--print-bytecode" for flags that run after bootstrap and use setFlags (well, SetFlagsFromString in C++ land)
🎉 - add new flags for the most common use cases e.g. --print-bytecode-after-start and --trace-opt-after-start

I'm also very open to bikeshedding the naming of flags/phrasing of docs or other ideas to improve this.

@richardlau
Copy link
Member

Let's try a "poll" and see what people think:

ICYMI GitHub now has polls, e.g. https://github.com/nodejs/node/discussions/categories/polls. See https://github.blog/2022-04-12-whats-new-in-github-discussions-organization-discussions-polls-and-more/ for the announcement.

@bnoordhuis
Copy link
Member

What I didn't realize is that this flag can be turned on "dynamically".

It's kind of dangerous though (in general, not with this particular flag at this point in time.)

For some flags V8 performs start-up initialization that's not done when you flip the flag post-startup.

You don't know until you audit deps/v8/src for all FLAG_print_bytecode code paths. Those change over time, of course.

@camillobruni
Copy link
Contributor

Limiting the output to to the v8.log can be achieved by not using --log-all but rather various --log-XXX flags.
There various filtering flags as well, like --print-bytecode-filter which can work well in some cases, maybe we could extend V8 on that front.

There is a certain level of information that has to be there for certain tools to work (for instance certain native addresses of internal functions/builtins to make the profiler work)

Also note that V8 is actively working on disallowing changing many flags at runtime.

@camillobruni
Copy link
Contributor

Or, even differently, maybe we should just have a better way of filtering the node-internal events in the performance tools.

@benjamingr
Copy link
Member Author

Also note that V8 is actively working on disallowing changing many flags at runtime.

That's probably fine if there is a workaround but that's motivation not to encourage it in the docs so at the very least we should hold-off on that.

Or, even differently, maybe we should just have a better way of filtering the node-internal events in the performance tools.

Basically the ask is for end users to be able to use tools (like those on v8.dev) to get insight on how code is run without seeing a lot of Node core. Most JavaScript devs have Node installed but few have d8 installed and most app code (both frontend and backend) typically can run in Node.js already (e.g. for unit tests).

So any way that results in creating a log that filters out Node core (and then Node can just opt into that or provide guidance on how users should use the flags) would be good.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Dec 12, 2022
@benjamingr
Copy link
Member Author

Not stale

@github-actions github-actions bot removed the stale label Dec 13, 2022
@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jun 11, 2023
@benjamingr
Copy link
Member Author

@nodejs/performance is there any willingness to tackle this?

@anonrig anonrig added the performance-agenda Performance Team Agenda label Jun 24, 2023
@Qard
Copy link
Member

Qard commented Jun 24, 2023

Might make sense to have a construction option for scripts/modules to specify a script as an embedder-supplied script or module rather than user code and then have a new flag to toggle embedder script or module presence in other flags. There's value in still being able to get that data from core as it can help us debug core issues, but I definitely agree to most users it's noise.

Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. performance Issues and PRs related to the performance of Node.js. stale v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

6 participants