jonhoo / inferno

A Rust port of FlameGraph
Other
1.68k stars 125 forks source link

Correctly annotate jitdump stack frames #202

Closed asherkin closed 3 years ago

asherkin commented 3 years ago

When using perf with the modern jitdump format, perf inject creates an ELF object for each JITted function and replaces the function entries in the recording to point to those SOs. This causes stackcollapse-perf to not recognise the functions as the module is no longer displayed as being the perf JIT map for the process.

codecov[bot] commented 3 years ago

Codecov Report

Merging #202 (7e2da84) into master (f2f1e9a) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #202   +/-   ##
=======================================
  Coverage   90.08%   90.08%           
=======================================
  Files          17       17           
  Lines        2239     2239           
=======================================
  Hits         2017     2017           
  Misses        222      222           
Impacted Files Coverage Δ
src/collapse/perf.rs 97.35% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f2f1e9a...7e2da84. Read the comment docs.

jonhoo commented 3 years ago

Good catch! Could you please include a test case with this new format as well?

asherkin commented 3 years ago

Very happy to add some tests, but it looks like the existing test infrastructure does not test with any of the options enabled (so this would be a no-op) - so I could do with some guidance on how you'd like them added.

jonhoo commented 3 years ago

Not sure I follow? The Java flamegraph test for example specifically runs with the java color palette: https://github.com/jonhoo/inferno/blob/f2f1e9ab07b0f7a669d290c1552024f9b7ddf5ae/tests/flamegraph.rs#L135-L146

asherkin commented 3 years ago

Test added - I had only seen the unit tests and completely missed the integration tests 😄

jonhoo commented 3 years ago

I also just realized I linked you to the flamegraph tests :facepalm: But glad you found them!

jonhoo commented 3 years ago

Would you mind also updating the CHANGELOG?