FlameGraph Issues in FX UI ?

[PhRX]

I just read @nitsanw 's great blog post about Flamegraphs, and after having another look at the flamegraph rendering in the honest profiler JavaFX front-end, it seems to me that there's an issue : the stacks do not seem to be aggregated together properly.

Fundamentally, if I understand it correctly, a flamegraph is "just" a graphic rendition of the tree view (aggregated by method id), with the roots at the bottom, and rectangles for frames, the width being in proportion to the sample count % in their parent.

If I read the code correctly though, the flame graph collector starts a new FlameTrace whenever the next received stack isn't identical to the previous one. This is then rendered as such without further aggregation.

As a consequence, if only 2 stacks appear in the sample, but they alternate, the result will be highly fragmented.

To put it more explicitly, say that we see : a.b.c() (stack 1) and d.e.f() being called alternatingly 3 times :

• Log events (simplified) : --- Trace Start --- Frames for a.b.c --- Trace Start --- Frames for d.e.f --- (repeat the above two more times)

My understanding is that this should be rendered as 2 vertical columns, each with 3 layers, and every block has a sample count of 3.

However, from the code, it seems that this will be rendered as 6 columns, with every block having a sample count of 1.

I'm about to try and "fix" this, but before checking anything in I'd like to get some confirmation that this is indeed an issue, and not based on my misunderstanding the principles.

[nitsanw]

@PhRX The issue you refer to is not a problem for the command line tool producing collapsed stacks file (I think). At least from what I have seen. The JavaFX rendering is broken. Please hold fire until your MEGA PR is accepted.

[PhRX]

@nitsanw OK I'll fix it on a new branch.

[PhRX]

Fixed in https://github.com/PhRX/honest-profiler-fx-ui/commit/459331053e357d895fa47b50f984d29740917ee2

Will close after #184 is accepted + new PR created for this fix + accepted.

[PhRX]

@nitsanw you might be interested in my latest commit on the flame branch since it adds FlameGraph diffs. Also, the flamegraphs are filterable and groupable, so you can switch between aggregate-by-thread or aggregate-all-threads at the flick of a, well, switch... :) The coloring logic might not necessarily be to your taste but it is easily tweakable.

[epabst]

I'm excited for this fix.

[PhRX]

Can be tested in PR #192

[PhRX]

Fixed independently in #210