Finalize profiling on the default threadpool + tweak precompiles #30
Conversation
src/ProfileEndpoints.jl
Outdated
| @static if VERSION < v"1.9" # Before Julia 1.9, precompilation didn't stick if not in __init__ | ||
| function __init__() | ||
| include("precompile.jl") | ||
| end | ||
| else | ||
| include("precompile.jl") | ||
| end |
src/precompile.jl
Outdated
| precompile(_do_alloc_profile, (Float64,Float64,)) || error("precompilation of package functions is not supposed to fail") | ||
| precompile(_start_alloc_profile, (Float64,)) || error("precompilation of package functions is not supposed to fail") | ||
| precompile(_stop_alloc_profile, ()) || error("precompilation of package functions is not supposed to fail") | ||
| end |
There was a problem hiding this comment.
missing endline. (You should be able to fix this in your editor preferences)
src/ProfileEndpoints.jl
Outdated
| @@ -94,7 +94,8 @@ function cpu_profile_stop_endpoint(req::HTTP.Request) | |||
| qp = HTTP.queryparams(uri) | |||
| with_pprof = parse(Bool, get(qp, "pprof", default_pprof())) | |||
| filename = "cpu_profile" | |||
| return _cpu_profile_response(filename; with_pprof) | |||
| # Defer the potentially expensive profile symbolication to a non-interactive thread | |||
| return fetch(Threads.@spawn _cpu_profile_response($filename; $with_pprof)) | |||
There was a problem hiding this comment.
Can you do line 107 too?
And the other endpoints need to be done as well. If you want to tackle them in this PR, that would be awesome, otherwise please file an issue and we can get to it later. Thanks!
| @@ -208,7 +209,8 @@ function allocations_start_endpoint(req::HTTP.Request) | |||
| end | |||
|
|
|||
| function allocations_stop_endpoint(req::HTTP.Request) | |||
| return _stop_alloc_profile() | |||
| # Defer the potentially expensive profile symbolication to a non-interactive thread | |||
| return fetch(Threads.@spawn _stop_alloc_profile()) | |||
There was a problem hiding this comment.
oh yes that seems wise! 💡
I suppose we could even spell this as:
| return fetch(Threads.@spawn _stop_alloc_profile()) | |
| return fetch(Threads.@spawn :default _stop_alloc_profile()) |
just so that code is even more explicit
(but maybe that's overkill since its the usual behaviour of spawn and there's the comment, i just sometimes forget whether or not spawn inherits the threadpool)
There was a problem hiding this comment.
Thanks Nick. I started with that but then I noticed the docs say that (:default if [threadpool is] unspecified) and we also need to support Julia 1.6 so it felt a bit cleaner to leave as is.
Hopefully, the first commit will help with the heartbeat losses we saw when we profiled our benchmarks. The assumption being that when we finalize the profile, we spend a lot of time looking up symbols which overwhelms the thread that is running it (and if it is an interactive thread, that could cause problems).