-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add more profiling events #49493
Add more profiling events #49493
Conversation
7778b1b
to
8183637
Compare
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.
Nice additions!
Could you provide screenshots showing the new added/moved events in action?
Also given that we're seeing millions of lock events per minute now, I've gone ahead and stripped out the case where we acquire the lock immediately separately from when we actually spin to wait. |
While it was a lot spin events I do wonder what those are, because they aren't from the runtime at least. |
@@ -3195,6 +3194,7 @@ static jl_value_t *jl_validate_cache_file(ios_t *f, jl_array_t *depmods, uint64_ | |||
// TODO?: refactor to make it easier to create the "package inspector" | |||
static jl_value_t *jl_restore_package_image_from_stream(ios_t *f, jl_image_t *image, jl_array_t *depmods, int completeinfo) | |||
{ | |||
JL_TIMING(LOAD_IMAGE, LOAD_Pkgimg); |
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.
For a future PR it would be good I guess to also annotate this trace point with the name of the package getting loaded.
Turning on stack collection on tracy I can see that the main uses of it are the locks when inserting backedges. Those being in |
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.
Looking good! Might be nice to cover jl_insert_methods
as well
P.S. In future PR's I'd like to explore removing the subsystem name that was added to most of the timing events (at least for the Tracy backend) - I think it makes the events significantly less "friendly" to view in the profiler
The new events center mostly around annotating load time contributors.