Add an option to filter and truncate stacks to a provided symbol

[saethlin]

I'm not really sure what to call this, but I've been using this out of my fork for a while and I think it's pretty useful. Especially when working on a codebase that is calling the function of interest inside a rayon parallel iterator.

[codecov]

Codecov Report

Base: 90.28% // Head: 90.79% // Increases project coverage by +0.51% 🎉

Coverage data is based on head (5cac2b1) compared to base (198a6a3).
Patch coverage: 16.66% of modified lines in pull request are covered.

❗ Current head 5cac2b1 differs from pull request most recent head 765760d. Consider uploading reports for the commit 765760d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #274      +/-   ##
==========================================
+ Coverage   90.28%   90.79%   +0.51%     
==========================================
  Files          19       19              
  Lines        4219     4236      +17     
==========================================
+ Hits         3809     3846      +37     
+ Misses        410      390      -20     

Impacted Files	Coverage Δ
src/flamegraph/mod.rs	95.25% <16.66%> (-2.70%)	⬇️
src/collapse/perf.rs	89.20% <0.00%> (+0.49%)	⬆️
src/flamegraph/color/mod.rs	87.39% <0.00%> (+1.68%)	⬆️
src/collapse/sample.rs	97.76% <0.00%> (+2.98%)	⬆️
src/collapse/vtune.rs	96.90% <0.00%> (+3.09%)	⬆️
src/collapse/vsprof.rs	97.12% <0.00%> (+3.34%)	⬆️
src/differential/mod.rs	99.14% <0.00%> (+4.27%)	⬆️
src/collapse/guess.rs	91.80% <0.00%> (+13.11%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jonhoo]

Hmm, this feels very similar to the --skip-after argument to collapse-perf (#222 + #224, #231, cc @romange) — I wonder if it still makes sense to have both?

With something like this I also worry about it giving an inconsistent view since it doesn't trim things to a particular stack, it picks things from potentially unrelated stacks and combines them (which, to be fair, --skip-after also does).

[saethlin]

it picks things from potentially unrelated stacks and combines them

Yeah, that's what I'm going for here. The idea is that when you're using this flag you don't care how control flow arrived at the function of interest. Of course the path taken to the function might matter, but for the cases I'm interested in the alternative is having a number of low-signal versions of the flames scattered all over, which in my experience is much worse because people tend to just search for the symbol and only look at the biggest pink rectangle. Maybe the biggest two.

I'll admit I didn't notice that --skip-after exists. Unfortunately I'd rather not use it :/ . I'm using the folded stacks files as a portable store of profile data. perf.data files aren't portable, the final SVGs are too big, and the simple text nature of the folded stacks format lends itself to ad-hoc analysis with command-line tools when the SVGs aren't up to the task. So a flag on inferno-collapse-perf is at the wrong stage of the analysis. I can't take one perf recording and render two --skip-after flamegraphs from it without running the collapser twice. And if I all I have is the folded stacks (as opposed to the output of perf script, or perf.data in the requisite environment), --skip-after is no help.

---

I also want filtering behavior which --skip-after does not provide, so that my browser doesn't crash. For whatever reason chromium gives up on SVGs over too many MB in size. Though of course I can put grep between the collapser and inferno-flamegraph to get that effect.

[jonhoo]

That's a good point. Makes me wonder if we should mark --skip-after as deprecated and encourage the use of this new flag instead. What do you think? Also cc @bsilver8192.

Another option here is to provide this as a separate binary, like inferno-diff-folded, where you pass in one folded file and it produces another one. That way the tool would actually be useful even for folks who use "standard" flamegraph.

[bsilver8192]

That's a good point. Makes me wonder if we should mark --skip-after as deprecated and encourage the use of this new flag instead. What do you think? Also cc @bsilver8192.

I'd be fine doing the filtering at either point. I do need the ability to pass multiple strings though, not just one like this PR currently supports. I end up using --skip-after with the top-level function for each thread.

Specifically, I run into issues with stack unwinding from some places not making it all the way through the thread startup code, while it does from other places. That means what is in fact the same stack looks different because some of them are missing a few of the highest levels. The resulting stack traces look like the ones mentioned in this PR, but it's not actually merging unrelated stacks, it's merging the same stack that looks different.

[jonhoo]

@saethlin How do you feel about adjusting this so that it can handle more than one filter to also cater to @bsilver8192's use-case?

[saethlin]

I'm not entirely sure how this should be tested. I could factor out the filtering logic into a helper function and unit test it? I couldn't find any tests for skip_after, I was thinking the easiest thing would be to verify that it works exactly the same.

[jonhoo]

I would be happy with just integration here that specifically check that with a given input file, we produce the right collapse when 1/n bases are provided. Should be a pretty straightforward variant of the existing collapse integration tests I think.

[jonhoo]

Thanks!

[jonhoo]

Released as 0.11.14 🎉