Improve performance for code size screen

[peterdjlee]

• Improve performance when switching between tabs.
  • Keep each tab view persistent between tab switches.
• Improve performance when switching between different diff types on the diff tab.
  • Ensure loading for each type only happens once by saving each tree in a node.

[peterdjlee]

Is there a way to keep tabs truly persistent?

I've tried looking at this article which states that using AutomaticKeepAliveClientMixin should help. I was able to stop the tabs from rebuilding, but that didn't seem to noticeably improve the performance. Either rebuilding isn't the problem or my method of checking if the widget is rebuilding is wrong.

[peterdjlee]

I've tried looking at this article which states that using AutomaticKeepAliveClientMixin should help. I was able to stop the tabs from rebuilding, but that didn't seem to improve the performance. Either rebuilding isn't the problem or my method of checking if the widget is rebuilding is wrong.

Rebuilding widgets:

Not rebuilding widgets:

[kenzieschmoll]

Is each tree map cell wrapped in a Tooltip?

[peterdjlee]

Is each tree map cell wrapped in a Tooltip?

Yup. All cells and also the title bars are wrapped in a Tooltip.

[peterdjlee]

Yup. All cells and also the title bars are wrapped in a Tooltip.

Can confirm that removing all Tooltip widgets and Inkwell widget shaves off about a second each, reducing around 66% of the loading time. So one optimization we can do would be to limit the number of widgets we wrap with Tooltip and Inkwell.

[kenzieschmoll]

It is very expensive to initialize all those listeners. We had to do something creative in the flame chart where we wrapped the whole chart in a MouseRegion widget and tracked the (x, y) hover position. Then based on that position, we did an efficient search for the element that would be at that position, rebuilding the widget that should then be wrapped in a tooltip. Something like this may be tricky for the treemap, as I don't know that we'd be able to use binary search like we were able to in the flame chart. For this to be useful, we'd have to have an efficient way to find a treemap element based on an (x, y) position.

[peterdjlee]

I've tried looking at this article which states that using AutomaticKeepAliveClientMixin should help. I was able to stop the tabs from rebuilding, but that didn't seem to noticeably improve the performance. Either rebuilding isn't the problem or my method of checking if the widget is rebuilding is wrong.

Update: The tabs were not rebuilding but each treemap cell was still rebuilding. I tried building Treemap widgets AutomaticKeepAliveClientMixin but for some reason that did not prevent the Treemap widgets from rebuilding.

[peterdjlee]

Would it be okay to only wrap the 1st level items in Inkwell and Tooltip? Essentially, clicking on a cell won't make the node of that cell the root of the treemap. If removing the ability to jump to a cell on click isn't a big inconvenience to the users, I think it'd be a good optimization. I'm not really sure what the users would want though.

[kenzieschmoll]

The Treemap navigation is really the only way a user can dig through the data, so we don't want to remove the ability for the user to change the root and "zoom" into the tree.

[peterdjlee]

Sorry, I meant that the user will still be able to zoom in but only a level deeper. For example in the screenshot below, clicking on 'src' under package:flutter will reroot the treemap to the package:flutter node instead of src node. So if a user tried clicking on any of the small cells under 'dart:typed_data', the treemap would reroot with the dart:typed_data node, not the actual cell the user clicked on.

[kenzieschmoll]

hmm.. that feels like it may be confusing to a user that what they click doesn't actually become the root. If you remove just the tooltips and leave the inkwells, does that improve the perf?

[peterdjlee]

I agree. Maybe adding some sort of color filter to indicate on hover that the selection applies to the entire rectangle would help with that. Yep, removing both seems to reduce the loading duration down to at least 50%.