jonhoo / inferno

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

Support for simplifying recursive function calls as stackcollapse perl scripts #275

Closed akanalytics closed 1 year ago

akanalytics commented 1 year ago

Is there a way of collapsing recursive calls? The original perl scripts have some support here https://github.com/brendangregg/FlameGraph/blob/master/stackcollapse-recursive.pl

Its different from --skip-after. I dont want to ignore recursion, I want to "compress" it to a single layer.

jonhoo commented 1 year ago

That's interesting. No, we don't currently have a port of that script, but if you want to take a stab at porting it, I'd love to take a look at a PR!

jacksonriley commented 1 year ago

Have made a start on this at https://github.com/jacksonriley/inferno/compare/main...collapse-recursive - will get a PR up for it at some point (unless you want to take over, @akanalytics!)

akanalytics commented 1 year ago

It's great that this is being tackled. I had hoped to do something, but my rust learning, and especially git knowhow only progresses slowly... If there's a way I could help without knowing git/PRs or advanced rust let me know - testing perhaps?

jacksonriley commented 1 year ago

It's great that this is being tackled. I had hoped to do something, but my rust learning, and especially git knowhow only progresses slowly... If there's a way I could help without knowing git/PRs or advanced rust let me know - testing perhaps?

Thanks for letting me know! I’ve gotten a PR up at https://github.com/jonhoo/inferno/pull/291 which has some testing, but if you’d like, I’m sure it would be useful as the issue opener to

Only if you feel like it though! :)

akanalytics commented 1 year ago

OK. Initial thoughts...

Hoping to get my Linux box upgraded and back working later this week, and I will then be able to compile and test.

BTW - an obvious enhancement is to fold non-direct recursive functions (eg a pair of recursive functions which call one another). Its straightforward to flag non adjacent duplicates in a stackframe, but hard I think to do this nicely, especially in cases where the initial entrant function (first called recursive function of a pair) varies... Definitely for another time!

akanalytics commented 1 year ago

Checked out, built and tested (on my chess engine which utilizing alpha-beta search, uses recursion extensively). Resulting flamegraph looks good - works great!

jacksonriley commented 1 year ago

Thank you very much for reading the code and trying it out - great to know that it works for you. I can't claim any real experience with Rust so you're at least as qualified as I am!

It will work less well, should any of the profiling tools have inbuilt support for recursion detection which somehow is incorporated into the file format - but I've never heard of such a thing, so I suspect the approach you have taken works best.

I don't think it should make a difference to how it works - It feels more like a question of how the UX should be to me. Maybe we'll just wait and see what Jon thinks.

BTW - an obvious enhancement is to fold non-direct recursive functions (eg a pair of recursive functions which call one another). Its straightforward to flag non adjacent duplicates in a stackframe, but hard I think to do this nicely, especially in cases where the initial entrant function (first called recursive function of a pair) varies... Definitely for another time!

Yes I thought about this but it seems less obvious what the correct behaviour is. For example if you have the stacks

A;B;C;B;E 1
A;B;D;B;E 1

it feels like the only possible folding would be to cut out the B->...->B recursion, to give

A;B;E 2

but then it feels a little strange to me that these stacks would end up folding down to the same thing. What if all the time in your program is being spent in the body of C but this isn't visible in the flamegraph? (I guess the answer there is not to use this script!)

This is the first case that I thought of that felt of dubious value, I'm sure there are more (your point about the initial entrant function varying is another good one!).

Couple this with my expectation that this sort of co-recursion is going to crop up less frequently than direct recursion (citation needed), I agree that it does seem like one for another time if at all :)