Show categories breakdown in flamegraph tooltip if implementation breakdown not present

[parttimenerd]

Adds a breakdown based on categories to the tooltips if the implementation breakdown cannot be shown (no implementation data present). Showing both at the same time would clutter the UI too much. Useful especially for imported profiles where the implementation data is missing.

Related to #4189 which added an option to omit the implementation data.

Sample profile

Current production Deploy preview

[codecov[bot]]

Codecov Report

Base: 88.49% // Head: 88.31% // Decreases project coverage by -0.18% :warning:

Coverage data is based on head (4213f87) compared to base (4965f7a). Patch coverage: 17.18% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4193      +/-   ##
==========================================
- Coverage   88.49%   88.31%   -0.19%     
==========================================
  Files         282      282              
  Lines       25107    25170      +63     
  Branches     6756     6778      +22     
==========================================
+ Hits        22219    22228       +9     
- Misses       2683     2729      +46     
- Partials      205      213       +8     

Impacted Files	Coverage Δ
src/profile-logic/profile-data.js	90.41% <ø> (ø)
src/utils/index.js	75.60% <0.00%> (-12.97%)	:arrow_down:
src/components/tooltip/CallNode.js	58.74% <18.96%> (-28.47%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[mstange]

Here's a deploy preview link with a Firefox profile to which I manually added "doesNotUseFrameImplementation":true.

[mstange]

This is pretty cool!

With the Firefox profile I noticed some confusing output where categories were seemingly duplicated, e.g. there are two rows for "Graphics". I think the first row is the "Graphics" category, and the second row is the "Graphics: Other" subcategory. It would be great if the Other subcategory was spelled out here.

For categories for which only one subcategory is displayed, I wonder if it makes sense to leave out the category row and only display the single subcategory row.

And the following could be future improvements (not handled in this PR):

• Use the category colors instead of blue. (Requires both an "intense" and a "soft" version of each color)
• Improve the display for subcategories, maybe with indentation or other visual grouping so that the category name doesn't have to be repeated for each subcategory.

[parttimenerd]

With the Firefox profile I noticed some confusing output where categories were seemingly duplicated, e.g. there are two rows for "Graphics". I think the first row is the "Graphics" category, and the second row is the "Graphics: Other" subcategory. It would be great if the Other subcategory was spelled out here.

This is due to the built in mapping of the "other" category to "", but I can change it.

For categories for which only one subcategory is displayed, I wonder if it makes sense to leave out the category row and only display the single subcategory row. Useful.

[julienw]

I haven't looked to the code or even tried it (as I'm in holidays 😁). I just wanted to give the feedback that 1/ this is pretty cool, thanks for working on this! And 2/ what do you think of showing the categories by default, and the implementation only if there's no category data ?

[parttimenerd]

And 2/ what do you think of showing the categories by default, and the implementation only if there's no category data ?

I have no real opinion on it, as in my use case there is no implementation data and introduced a flag in #4189 to essentially hide all implementation data.

I think #3713 is the place to discuss these kinds of issues.

[parttimenerd]

I updated it, it now looks as follows:

[parttimenerd]

Now with correct colors:

[parttimenerd]

I added tests and fixed the formatting.

[fqueze]

The vertical alignment between the blue or grey bars and the text looks like it could be improved. I think it would look better if the bottom of the colored bar was aligned with the baseline of the text. I saw this both in the screenshots here and when trying your deploy preview.

[parttimenerd]

You're right, thanks for spotting it.

[julienw]

Looking at the deploy preview, I can see the following issues that will need to be resolved:

1. We don't see the difference between self and running time anymore. I believe you could use the category color with some transparency for the running time.
2. We don't see the grey background bar, which was very useful to relate the numbers on the right with the labels, especially with the values are small (which makes the bar small too). Of course then there's a problem with the grey category; for that one I thought it could be white and light grey with a border.
3. The overview bar is now missing.
4. There's the alignment problem that Florian already reported.
5. I think we should keep the word "samples" in the table rather than the title. I understand this shows repeatition, but we believed the table is more balanced that way, otherwise it looks a bit broken.
6. We need the table to breathe some more so that we see the structure of categories/subcategories better. We could think of adding a top margin (4 or 8px) for the category title, as well as a right padding (4 or 8 px too) to move it slightly to the left. Not 100% sure but I'd like to see the result of this.

Thanks!

[parttimenerd]

Thanks for the suggestions, I'm currently working on them.

[parttimenerd]

It now looks like:

With automatic brightening of colors, surrounding problematic lighter color bars with a grey line.

[julienw]

Mmm from the screenshot it looks like the bars are a bigger height for the category than for the subcategory, also it doesn't seem to end at the same position either.

[parttimenerd]

also it doesn't seem to end at the same position either

That's fixed in the most recent commit.

Mmm from the screenshot it looks like the bars are a bigger height for the category than for the subcategory

That's on purpose: making the category labels bold, increases the height of the label font. This solves the centering issue

[parttimenerd]

I improved the style of categories:

[julienw]

I think I'll give it a go myself this week, so that we can maybe reach a consensus next week.

[parttimenerd]

Sorry, I focused on the other PRs (like sorting and resizing), but I'll comeback to this PR tomorrow.

[julienw]

Sorry, I focused on the other PRs (like sorting and resizing), but I'll comeback to this PR tomorrow.

no worries, my comment came after a discussion with @canova: because we're not sure of the right path, it's probably a good idea that I make my hand dirty and try the things myself :-)

[parttimenerd]

So I should not work on this tomorrow?

[parttimenerd]

About the styling, it's definitely much better than before. I'm not a big fan of the underlines though.

I replaced it my making the category headers bold.

when the only subcategory is the Other subcategory, skip it

That is reasonable.

when there's only one subcategory, merge the category with its subcategory

I merged it as "{category}: {subcategory}"

when there are several subcategories with the other subcategory, it should come last

Ok.

One quick thought, maybe it's bad but you'll tell me: instead of having one bar, with 2 delimitations inside, we could have 2 bars with half height.

That sounds quite reasonable and color agnostic:

I integrated all the other requested changes too.

[parttimenerd]

This looks reasonable (and usable) to me now.

[parttimenerd]

I integrated all your suggestions, the code is now far better readable and less complex.

[julienw]

Sorry, I missed you pushed new commits, I'll try to look at this tomorrow.

[parttimenerd]

Thanks for the fixes, they look good.