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.

Sign up for GitHub

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

Finalize profiling on the default threadpool + tweak precompiles #30

Merged
merged 3 commits into from
Apr 10, 2024
Merged

Finalize profiling on the default threadpool + tweak precompiles #30

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ea3ee1
Finalize profiles on a non-interactive thread
Drvi Apr 9, 2024
5653cdb
Don't defer precompiles to `__init__` time on Julia 1.9+
Drvi Apr 9, 2024
c1c6d36
PR feedback
Drvi Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions src/ProfileEndpoints.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

end

function _do_cpu_profile(n, delay, duration, with_pprof)
Expand Down Expand Up @@ -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())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes that seems wise! 💡

I suppose we could even spell this as:

Suggested change
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)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

end

function _do_alloc_profile(duration, sample_rate)
Expand Down Expand Up @@ -263,25 +265,12 @@ end

# Precompile the endpoints as much as possible, so that your /profile attempt doesn't end
# up profiling compilation!
function __init__()
precompile(serve_profiling_server, ()) || error("precompilation of package functions is not supposed to fail")

precompile(cpu_profile_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(cpu_profile_start_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(cpu_profile_stop_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(_do_cpu_profile, (Int,Float64,Float64,Bool)) || error("precompilation of package functions is not supposed to fail")
precompile(_start_cpu_profile, (Int,Float64,)) || error("precompilation of package functions is not supposed to fail")

precompile(heap_snapshot_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")

precompile(allocations_profile_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(allocations_start_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(allocations_stop_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
if isdefined(Profile, :Allocs) && isdefined(PProf, :Allocs)
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")
@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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️


end # module ProfileEndpoints
18 changes: 18 additions & 0 deletions src/precompile.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
precompile(serve_profiling_server, ()) || error("precompilation of package functions is not supposed to fail")

precompile(cpu_profile_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(cpu_profile_start_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(cpu_profile_stop_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(_do_cpu_profile, (Int,Float64,Float64,Bool)) || error("precompilation of package functions is not supposed to fail")
precompile(_start_cpu_profile, (Int,Float64,)) || error("precompilation of package functions is not supposed to fail")

precompile(heap_snapshot_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")

precompile(allocations_profile_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(allocations_start_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
precompile(allocations_stop_endpoint, (HTTP.Request,)) || error("precompilation of package functions is not supposed to fail")
if isdefined(Profile, :Allocs) && isdefined(PProf, :Allocs)
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing endline. (You should be able to fix this in your editor preferences)

Loading