[two_dimensional_scrollables] optimizes tableview janks with >250k rows#10737
[two_dimensional_scrollables] optimizes tableview janks with >250k rows#10737wangfeihang wants to merge 3 commits intoflutter:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant performance optimization for TableView with a large number of rows by replacing a linear search with a binary search to find visible cells. The implementation is correct and includes relevant tests. I've provided a few suggestions to simplify the new binary search helper function and its call sites for improved code clarity and to better reflect the underlying data invariants.
| int? _binarySearchFirstFromMap(Map<int, _Span> map, int length, bool Function(_Span) condition) { | ||
| if (map.isEmpty || length <= 0) { | ||
| return null; | ||
| } | ||
| var low = 0; | ||
| int high = length - 1; | ||
| int? result; | ||
| while (low <= high) { | ||
| final int mid = low + ((high - low) >> 1); | ||
| final _Span? span = map[mid]; | ||
| if (span == null) { | ||
| low = mid + 1; | ||
| continue; | ||
| } | ||
| if (condition(span)) { | ||
| result = mid; | ||
| high = mid - 1; | ||
| } else { | ||
| low = mid + 1; | ||
| } | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
The implementation of _binarySearchFirstFromMap can be simplified. The length parameter is redundant as it's always map.length. Additionally, the _rowMetrics and _columnMetrics maps are populated with contiguous integer keys starting from 0, making them dense. Therefore, the span == null check is unnecessary and can be removed, which also allows for using a non-nullable _Span type for span. This improves code clarity by reflecting the data structure's invariants. I'll suggest updates to the call sites in subsequent comments.
int? _binarySearchFirstFromMap(Map<int, _Span> map, bool Function(_Span) condition) {
if (map.isEmpty) {
return null;
}
var low = 0;
int high = map.length - 1;
int? result;
while (low <= high) {
final int mid = low + ((high - low) >> 1);
final _Span span = map[mid]!;
if (condition(span)) {
result = mid;
high = mid - 1;
} else {
low = mid + 1;
}
}
return result;
}| _firstNonPinnedColumn = _binarySearchFirstFromMap( | ||
| _columnMetrics, | ||
| _columnMetrics.length, | ||
| (span) => !span.isPinned && span.trailingOffset >= _targetLeadingColumnPixel, | ||
| ); |
There was a problem hiding this comment.
| _lastNonPinnedColumn = _binarySearchFirstFromMap( | ||
| _columnMetrics, | ||
| _columnMetrics.length, | ||
| (span) => !span.isPinned && span.trailingOffset >= _targetTrailingColumnPixel, | ||
| ); |
| _firstNonPinnedRow = _binarySearchFirstFromMap( | ||
| _rowMetrics, | ||
| _rowMetrics.length, | ||
| (span) => !span.isPinned && span.trailingOffset >= _targetLeadingRowPixel, | ||
| ); |
| _lastNonPinnedRow = _binarySearchFirstFromMap( | ||
| _rowMetrics, | ||
| _rowMetrics.length, | ||
| (span) => !span.isPinned && span.trailingOffset >= _targetTrailingRowPixel, | ||
| ); |
[two_dimensional_scrollables] optimizes tableview janks with >250k rows
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].///).