jonhoo / inferno

A Rust port of FlameGraph
Other
1.65k stars 118 forks source link

Flamechart reversing is ... unexpected #236

Open itamarst opened 2 years ago

itamarst commented 2 years ago

Consider the following profiling report:

a 1
b 2
c 3
this 5
is 4
a 3
test 2

Per https://docs.rs/inferno/latest/inferno/flamegraph/struct.Options.html#structfield.flame_chart, the input will be "reversed". And indeed, the resulting SVG has frames in the reverse order "test, a, is, this, c, b, a":

output.svg

The question is, why is this in reverse order? Given time in standard charts virtually always goes from left to right, you would expect the ordering in the flamechart to match that of the input file and go from left to right, i.e. "a, b, c, this, is, a, test" in this example.

I can easily enough emit my input lines in reverse order, but this seems wrong, and will confuse anyone who uses the feature for the first time.

jonhoo commented 2 years ago

I believe this was done purely to match the behavior of the original flamegraph implementation. @AnderEnder may be able to shed more light though, as they originally contributed https://github.com/jonhoo/inferno/pull/125.

As for why the original flamegraph does this, my guess would be it's because of scrolling. By having the most recent time to the left, you can see what last happened, and thus is most likely to be relevant, without having to scroll far right. Whether that's a good reason I'm not so sure, but :shrug: I'm not opposed to changing this either if we decide non-reversed just overall makes more sense.