-
Notifications
You must be signed in to change notification settings - Fork 78
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
Make --prof toggleable #154
Comments
(also because interpreted frames – #150 (comment)) |
@hashseed would you be open to a PR making |
@hashseed ping. |
Hey @ryzokuken - Just to say I discussed this with @hashseed in person - it's really non-trivial (aside from other the internal implementation complexities think about the log format - if it's toggglable when do you get the code creation events? Those have to be collected and stored right? So even if it's off there's an increased cost, and likely more memory consumption) Even given a viable approach that doesn't have unreasonable cost, --prof is internal/private to V8 so we can't currently expect the V8 team to have the same resources to invest in this than public apis |
Just want to point out that it's possible to implement this feature with no memory overhead if you iterate the heap and get all codes generated when you start the profiler (which is already done when we use --prof to collect codes loaded from V8's snapshot). The only overhead will be for users of the toggling feature, which is acceptable. If I'm not mistaken, @mcollina was also asking for this feature. |
FYI @hashseed is presently on vacation. He may not be able to respond to this thread until he's back. |
Any updates on this? |
ping. Just ran into this. I'm trying to profile code but don't want my setup code in the profile |
@kwasimensah thera are a few alternatives to
|
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made. |
As #150 – however since perf support in V8 is supposedly deprecated (as mentioned in #148) this could be the more important flag moving forward
The text was updated successfully, but these errors were encountered: