Fix #188 : Flame Graphs

[PhRX]

• Fixed the incorrect aggregation underlying the flamegraph rendering

• Implemented different color scheme for flamegraphs, with an eye on making it configurable from the UI in the future

• Added Flame Diff View comparing flamegraphs, (red/green indicating regression/improvement)

• Allow Flame Graph filtering

• Enable various aggregations (method name, method name + line nr etc.)

• Fixed one non-flamegraph-related regression : in diff view column headers the profile numbers went missing

[RichardWarburton]

Hi @PhRX,

Thanks for the PR. It looks like generally useful stuff, but I've got a few concerns that I've commented on inline in the codebase. I also have a bigger concern - which is that there is no automated tests to demonstrate that these features work. This means that if the code got refactored at a later date then we wouldn't know if it continued to work. Again, thanks for the fix, but I really want to make sure that we're not only adding features but also not just adding technical debt.

Richard

[PhRX]

Hi @RichardWarburton,

I'm not sure which comments in the codebase you're referring to - I'll address them as soon as I know where to find them :)

I'm aware of the lack of tests. It's a byproduct of moving forward fast. I'd be interested in getting pointers to a good JavaFX test framework, I don't know any yet. For now, I can still manually test everything in a couple of minutes, and given that I have a vested interest in having the profiler run correctly, I'll keep doing that until I find the time to automate testing.

Xeers Philip

[RichardWarburton]

Hi @PhRX,

I understand what you mean about moving fast, but I don't think its possible to move forward fast in a sustainable way without having some automated tests. I'm sure you're manually testing things, and that's most appreciated but I think its important to try and keep tests for key functionality automated. https://github.com/TestFX/TestFX looks like it would fit the bill for being able to test Javafx code, how do you feel about it?

[PhRX]

Hi @RichardWarburton,

Economically, it made more sense to code first in this case. Adding UI tests would up to now only have generated an extra maintenance load (given that I didn't have an up-front design of the projected end state) with not much benefit in return. Also, my bandwidth is limited. Now that things are that much closer to being stable, it makes more sense to start a test effort.

I'll have a look at TestFX. I'm OK with any framework, as long as it's something which will still be well-maintained in the future. The more it is accepted as "industry standard" the better. The first tests though will be core tests - the fundaments are the most important thing if the house needs to stay up.

[PhRX]

Superseded by upcoming, cleaner PR.