have implemented the use of inferno for the https://github.com/rust-lang/measureme/issues/42 but now it feels strange to call the tool stack_collapse when it generates the flamegraph svg file directly. shall I change the name to flamegraph in the same PR?
That sounds fine to me!
before I spend to much time on this. have been looking in to the "missing data in flamegraph" https://github.com/rust-lang/rust/issues/65788 and have made a test that I thinks is correct but do not pass https://github.com/rust-lang/measureme/compare/master...andjo403:collapse_threads can someone check if I'm thinking correct that it shall pass
moved some of the events from the old test to a new thread and as I think that the order between threads is not guaranteed I change the order for two events
I think that looks correct to me
I'm trying to update
collapse_stacks for https://github.com/rust-lang/measureme/pull/76 but I don't quite understand what the output file is supposed to look like.
is there documentation for it somewhere?
there is some description on https://github.com/brendangregg/FlameGraph#2-fold-stacks
but it is a semicolon separated string of the stack followed by an number that represent how many times that stack is present with some sample interval in the log resulting in that if there is multiple threads the number of stacks is larger then the wall time divided by the sample interval
also each stack + count is one line in the file
ok, that sounds simple enough. thanks, @Andreas Jonson !
@mw have added some more description in the stack collapse tests see https://github.com/rust-lang/measureme/pull/78
@mw think that the algorithm in perform_analysis in summerize can be reused for the stack_collaps, use the self time together with the stack to get a total time per stack and then at the end divide the time with the interval to get the count instead of time.
I wonder if there is a problem with using a sampling interval of 1ns. Then
time == count.
@andjo403 I looked at the implementation of the algorithm you are proposing and I think it's very smart!
It is actually simpler than my implementation
I'm wondering though: is there any benefit in the scaling step?
it will only cost accuracy and won't produce the same results as sampling anyway
I do not know of any use case for the interval so think that it can be removed
it might be somehow related to flamegraphs also supporting sampling profilers
but I don't think that really applies with our kind of data
I added the scaling to match the old implementation
I agree with you that it do not apply as we have exact data
let's try without the scaling and we'll see if we run into problems
I could imagine that there is some upper limit for the counters
or that we get to many samples
but we can add some kind of scaling/filtering back in if needed
Thanks for providing the alternative implementation!
@andjo403 I've removed the WIP commit from my branch. Do you want to update your PR? Then the commit has your name on it.
For me it do not matter if my name is there or not kan not do any updates until after work so you can take and change as you like
OK, then I'll just copy and paste your code and mention your contribution in the commit message