JuliaPerf / PProf.jl

Export Julia profiles to the pprof format
MIT License
155 stars 17 forks source link

Add support for directly displaying FlameGraphs.jl graphs. #25

Closed NHDaly closed 3 years ago

NHDaly commented 3 years ago

Add support for directly displaying FlameGraphs.jl graphs.

This makes PProf fit into the standard julia profiling infrastructure.

For example:

julia> using Profile, FlameGraphs, PProf, Serialization

julia> @profile peakflops()
1.1991880148053252e11

julia> fg = FlameGraphs.flamegraph()
Node(FlameGraphs.NodeData(ip:0x0, 0x00, 1:670))

julia> pprof(webport=11111, fg)
"profile.pb.gz"

julia> Main binary filename not available.
Serving web UI on http://localhost:11111
Screen Shot 2020-10-05 at 11 14 45 PM

Unfortunately, PProf doesn't retain the ordering information that the FlameGraphs have, so it's not as good of a format for FlameGraphs directly.

But the graph view and source views are still useful, even when building from FlameGraphs.

NHDaly commented 3 years ago

Also, after my last rewrite, i have checked and confirmed that it does indeed produce almost exact same results whether building directly from Profile or going through FlameGraphs - so maybe we want to simplify our code and always go through FlameGraphs? I'm not sure! I kind of like having to separate implementations to validate in case either one gets an error, but ALSO i feel like FlameGraphs has many more eyes on it, and is therefore much more vetted 😅

I guess there are some inefficiencies of going through FlameGraphs, though, since we have to essentially deconstruct the aggregated view back into the samples view, so that's a bit disappointing. Oh, and also we lose individual samples, and can only report a span.

This reminds me:


Here's the only one weird difference i found when comparing data from peakflops() that went through FlameGraphs vs the data that built went directly off of Profile (it's probably easier to see if you pull up the images side-by-side):

pprof(fg):

Screen Shot 2020-10-05 at 11 21 19 PM

pprof():

Screen Shot 2020-10-05 at 11 21 27 PM

I'm not sure if this is another instance of us incorrectly duplicating a frame or something weird? But when we build off of Profile directly we duplicate the next frame down, instead of having the correct frame there.

NHDaly commented 3 years ago

I don't really have sufficient experience with FlameGraphs.jl. It's a shame that you have to "unprocess" the graph. So I would definitly have both versions.

Yeah, agreed. But it does seem like this contains all of the same information we need, so it might be worth it to go this hop through FlameGraphs just to have a single, consistent transformation from Profile, which gets more eyes? 🤷


Also, I think the issue I showed in the images above is the only remaining issue before this is ready to merge!

vchuravy commented 3 years ago

I totally understand wanting to render FlameGraph data, especially since you can use it for other data sources.

In the long run I always wanted to include the JIT_PROFILE output for perf, and for that one likely would need to go from the raw form with the instruction pointers.

NHDaly commented 3 years ago

Okay, methinks this is good to go! :)

Thanks for the suggestions

NHDaly commented 3 years ago

After looking at the above discrepancy in the two images I posted, I think the FlameGraphs image is more likely to be right, and the PProf one is simply a dropped frame due to the inlining bug. It's maybe just that i ran this on an old commit (maybe on this branch) without merging in #27.