Add support for --reverse option

[jasonrhansen]

Fixes #21

[codecov-io]

Codecov Report

Merging #99 into master will increase coverage by 0.22%.
The diff coverage is 98.21%.

@@            Coverage Diff             @@
##           master      #99      +/-   ##
==========================================
+ Coverage    88.9%   89.13%   +0.22%     
==========================================
  Files          12       12              
  Lines        1379     1408      +29     
==========================================
+ Hits         1226     1255      +29     
  Misses        153      153

Impacted Files	Coverage Δ
src/flamegraph/merge.rs	95.41% <100%> (+0.46%)	⬆️
src/flamegraph/mod.rs	96.15% <97.14%> (+0.27%)	⬆️

---

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 03f5ad4...7f183a9. Read the comment docs.

[jasonrhansen]

Currently the performance when using the --reverse option is terrible. I'd like to figure out a way to reduce allocations.

[jasonrhansen]

I reduced allocations, but didn't see a huge speedup.

It looks like the main reason the performance can be much worse with the --reverse option is not from parsing and reversing the stacks. Of course that will slow it down some, and we can probably make it faster, but no matter how much we optimize that part it won't help much.

The big slowdown is because not many frames get merged when the stack order is reversed. It makes sense and is really obvious when you look at the resulting flame graph.

As an example, when creating a flame graph using tests/data/collapse-perf/results/example-perf-stacks-collapsed.txt as input, merge::frames returns only 1,139 frames without --reverse, but 18,247 frames with --reverse. That's a big difference.

Just to make sure that's what was making it slower, I created a version of example-perf-stacks-collapsed.txt where the stack orders were already reversed. I benchmarked flamegraph using this as input and it was almost as bad as the benchmark where I used the original file and passed the --reverse flag. I also benchmarked using the reversed file AND the --reverse flag and it was pretty fast. Not as fast as the original benchmark without --reverse, but not much worse.

[jonhoo]

It'd be interesting to see why we're so much slower when producing many frames. Surely it isn't just from having to write more bytes to the output? If so, maybe we need a BufWriter?

[jasonrhansen]

Yes, it would be worth investigating why producing more frames is so much slower. It would be nice to improve the performance overall. I was just initially surprised when I benchmarked with the --reverse option and the performance difference was much larger than I expected. I was thinking I must be doing something really bad.

[jasonrhansen]

I tried using a BufWriter and it actually slowed it down.

Also, just to be clear, inferno-flamegraph --reverse is still so fast on a single run that it might not even matter. When I run it on example-perf-stacks-collapsed.txt it appears to finish instantly. It's only when I run the benchmark (with 5050 iterations) that I can see it's much slower.

flamegraph/dtrace       time:   [7.6151 ms 7.6532 ms 7.6919 ms]
                        thrpt:  [9.8767 MiB/s 9.9267 MiB/s 9.9763 MiB/s]
                 change:
                        time:   [+466.22% +471.31% +476.06%] (p = 0.00 < 0.05)
                        thrpt:  [-82.641% -82.496% -82.339%]
                        Performance has regressed.

flamegraph/perf         time:   [41.598 ms 41.804 ms 42.030 ms]
                        thrpt:  [14.669 MiB/s 14.748 MiB/s 14.821 MiB/s]
                 change:
                        time:   [+721.96% +727.51% +733.03%] (p = 0.00 < 0.05)
                        thrpt:  [-87.996% -87.916% -87.834%]
                        Performance has regressed.

[jonhoo]

This is benchmarking --reverse against not --reverse, or just against the codebase as of this PR?

[jasonrhansen]

That was benchmarking --reverse against not --reverse on this branch.

Here is this branch against master (not using --reverse with either one).

flamegraph/dtrace       time:   [1.3086 ms 1.3230 ms 1.3421 ms]
                        thrpt:  [56.606 MiB/s 57.425 MiB/s 58.056 MiB/s]
                 change:
                        time:   [-3.5107% -2.5537% -1.6082%] (p = 0.00 < 0.05)
                        thrpt:  [+1.6344% +2.6206% +3.6385%]
                        Performance has improved.

flamegraph/perf         time:   [4.8170 ms 4.8403 ms 4.8662 ms]
                        thrpt:  [126.69 MiB/s 127.37 MiB/s 127.99 MiB/s]
                 change:
                        time:   [-2.0915% -1.4600% -0.8109%] (p = 0.00 < 0.05)
                        thrpt:  [+0.8175% +1.4817% +2.1362%]
                        Change within noise threshold.

[jonhoo]

@jasonrhansen fwiw, I opened #102.