jlfwong / speedscope

🔬 A fast, interactive web-based viewer for performance profiles.
https://www.speedscope.app
MIT License
5.59k stars 246 forks source link

Stackprof mode: Garbage collection frames should not be stacked on their previous frame #361

Open osyoyu opened 3 years ago

osyoyu commented 3 years ago

Stackprof (Ruby) mode displays garbage collection frames on top of the last recorded frame, but this is an inaccurate representation of the actual state. This is because Stackprof is a sampling profiler, which means that consecutive frames in the dump were not necessarily executed consecutively. Garbage collection frames are not an execption here.

Stackprof's built-in flamegraph displays garbage collection frames on the root level (notice the (garb...) in the bottom left). I believe this style promotes the flamegraph's accuracy (which is important); what do you think?

Stackprof's --d3-flamegraph Speedscope
Screen Shot 2021-08-20 at 18 49 24 Screen Shot 2021-08-20 at 18 40 15

Related code here:

https://github.com/jlfwong/speedscope/blob/6d02bf510fe8a0c29491eed176a36689927bfbb1/src/import/stackprof.ts#L38-L40

djudd commented 3 years ago

For what it's worth, I strongly prefer Speedscope's view of this; I find that putting the GC frame back at the bottom of the stack makes the flamegraphs much harder to read in practice, even if it's technically more correct. (I've actually hacked the current Speedscope behavior into an internal tool before in order to improve UX.) And I'm not sure that the heuristic which says the GC interrupted the particular stacks which were running before & after is any worse than the heuristic which lets us connect stacks to draw a flamegraph with long horizontal bars, instead of just a series of time points, in the first place?

A configuration option might be nice if there's a reasonable way to support one that's language/source-specific.