-
Notifications
You must be signed in to change notification settings - Fork 39
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
Feature: delay() function for marking key points in the flame graph #141
Comments
Some extra thoughts to make this a more powerful feature:
|
I'm not sure I quite get this without more context. Why not just use a regular function call without adding any additional delay? |
Could you try this? p <- profvis(...)
print(p, aggregate = TRUE) I think that's likely to give you a more useful display by pulling together the repeated function calls. (It's not very well documented currently but fixing that is on my to do list) |
I could definitely see that being a useful feature. But in a case like mine, the time and memory costs aren't in function calls. It's mostly in complicated loops calculating values for extremely large vectors without unnecessary memory allocations. Most of the optimization is in how indices are calculated, memory is accessed, and how the values are calculated. My |
If that's all you're using it for, isn't that a single that you should just pull out some named functions to make the structure of your code more clear to profvis? I'm not convinced that this is a general problem that profvis needs to provide help for. |
In general, that would be an option. In my case, removing the chunks of code from their own functions was itself a significant optimization because of R's copy behavior. It's also now at a point where many bits and pieces are precalculated, and putting it back into a dedicated function would involve passing in about a dozen objects. To be clear, I don't view this as a problem that needs to be solved for me. Rather, it's a feature suggestion that maybe some other people might find useful. My approach has been helpful for me, but the idea of injecting markers into the flamegraph/callstack could be way more elegant if it were a package feature. Something like a If you decide it's not worth implementing, that's fine. It's not a critical feature, and I wouldn't fight it; I'm just hoping I've managed to convey the idea properly. |
I've written a function that might be worth including in this package (or something similar). I sometimes end up in a situation where it's very difficult to distinguish between parts of a function inside the flame graph (especially because the bulk of my code is just loops, assignments, and basic operators), so I have to add code to help visually separate sections of the function so I can figure out which part actually needs optimization. To help with that, I've created the following function that I can call at key points in the code I'm trying to profile:
The
text
parameter gets assigned as a function name that will then show up in the flame graph. Here's an example:Basically, I called
delay("CRW Lookup")
anddelay("Build TR")
in places that delimit important sections. I thought my "CRW Lookup" section might be a possible bottleneck because it's a complex nested loop, but it turns out it's the opposite case, and there was no way I could ever figure this out based on the rest of the flame graph data.Edit: calling it something like
marker()
might make more senseThe text was updated successfully, but these errors were encountered: