search

flutter / devtools

Performance tools for Flutter
https://flutter.dev/docs/development/tools/devtools/
BSD 3-Clause "New" or "Revised" License
1.53k stars 313 forks source link

Optimize how the inspector V2 tree is built and updated #7978

Closed elliette closed 4 days ago

elliette commented 5 days ago

Fixes https://github.com/flutter/devtools/issues/7983 Work towards https://github.com/flutter/devtools/issues/7980

This PR makes a couple changes to how the Inspector V2 tree is updated to make it easier to implement expanding and collapsing the hidden implementation widgets (https://github.com/flutter/devtools/issues/7911).

Changes to the inspector tree traversal logic:

Previously there were two methods for traversing the tree, childrenCount and getRowIndex. The logic for both needed to be consistent otherwise they would disagree on which nodes were included in the tree. Furthermore, getRowIndex was called for every node in the tree, therefore traversing the tree n times.

For the gallery app, this meant that when the tree was loaded and a node was expanded and collapsed, the MaterialApp node was traversed 1345 times.

These two methods have been combined into a single method, _buildRows. With this new method, when the gallery app tree is loaded and then a node is collapsed and expanded, MaterialApp is only traversed 6 times.

Note: More optimization could be done here to only rebuild a single branch of the tree on updates, added a TODO in the code. See https://github.com/flutter/devtools/issues/7980 for details.

Changes to how tree updates are triggered:

Previously the tree was getting rebuilt via a call to setState with an empty callback. Instead, this PR switches to using a ValueListenable for the rows in the tree, and rebuilding when those rows change. See https://github.com/flutter/devtools/issues/7983 for details.

kenzieschmoll commented 4 days ago

this PR switches to using a ValueListenable for the rows in the tree

~DBC: this concerns me that having this number of listeners (one for each row in the tree) may cause performance issues. Using too many listeners is something we have bumped up against in the past. We should validate that this change does not negatively affect the performance of the inspector.~

EDIT: I see now after reviewing that we are using one ValueListenableBuilder for the whole tree, not one for each row. Never mind then, no concerns for that 😄