-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[two_dimensional_scrollables] optimizes tableview janks with >250k rows #10738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant performance optimization for TableView when dealing with a large number of rows. The linear search for visible cells has been replaced with a more efficient binary search, which will reduce scrolling jank. The implementation of the binary search is sound and new tests have been added to ensure correctness. My feedback includes a few suggestions related to repository contribution guidelines for the CHANGELOG.md and pubspec.yaml files, as well as a minor refactoring opportunity in the new test to improve maintainability.
|
|
||
| * Updates minimum supported SDK version to Flutter 3.35/Dart 3.9. | ||
| * Updates examples to use the new RadioGroup API instead of deprecated Radio parameters. | ||
| * Optimizes tableview janks with >250k rows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot append to releases that have already shipped. You'll need to create a new release and update the version in the pubspec as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Piinks, I've fixed the above issues in the new commit. Could you please review it again? Thanks so much for your review!
…x release version
Description:
This PR optimizes the scrolling jank issue of the TableView component when the number of rows exceeds 250,000.
Root Cause:
The _updateFirstAndLastVisibleCell method in RenderTableViewport uses linear for-loop traversal on _columnMetrics and _rowMetrics to locate the visible boundary cells: _firstNonPinnedRow, _lastNonPinnedRow, _firstNonPinnedColumn, and _lastNonPinnedColumn. When the number of rows/columns is extremely large (e.g., >250k rows), this linear traversal causes significant main-thread blocking and scrolling jank.
Solution:
Replace the linear for-loop with binary search algorithm to find the visible boundary cells (_firstNonPinnedRow, _lastNonPinnedRow, _firstNonPinnedColumn, _lastNonPinnedColumn). Binary search reduces the time complexity from O(n) to O(log n), effectively optimizing the scrolling jank issue under large data volumes.
Fixes: #138271
Video performance comparison
before:
https://github.com/user-attachments/assets/ca5b8821-4bdb-411f-bb2c-63998ac7c0d9
after:
https://github.com/user-attachments/assets/ebbf96d9-9e04-4ede-ae79-cd433885d3ab
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the [pub versioning philosophy], or I have commented below to indicate which [version change exemption] this PR falls under[^1].CHANGELOG.mdto add a description of the change, [following repository CHANGELOG style], or I have commented below to indicate which [CHANGELOG exemption] this PR falls under[^1].///).