perf: 优化100万数据下的滚动性能 #3128
perf: 优化100万数据下的滚动性能 #3128
Conversation
|
你好 @Alexzjt，非常感谢你的贡献. Hello, @Alexzjt, Thanks for your contribution. In order to make the code more robust, please add the corresponding unit tests, and update the docs if there are API changes. |
Walkthrough此PR通过引入 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
|
Size Change: +606 B (+0.1%) Total Size: 625 kB
ℹ️ View Unchanged
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #3128 +/- ##
==========================================
+ Coverage 75.77% 79.86% +4.09%
==========================================
Files 257 218 -39
Lines 11994 11939 -55
Branches 2464 2673 +209
==========================================
+ Hits 9088 9535 +447
+ Misses 1398 790 -608
- Partials 1508 1614 +106 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
这个PR 为什么不用了 ？这样实现会有什么问题吗 ？ |
|
@Create-Peace 对继承不太友好，风险高，收益一般。近期会优化渲染引擎G的部分实现以提升滚动帧率 |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant performance optimizations for scrolling, especially with large datasets. The core improvements come from object reuse, notably through the new DataCellPool and by updating existing canvas shapes instead of recreating them. These are excellent changes that directly address the performance goals.
I've identified a couple of areas where performance could be further improved. My feedback focuses on optimizing data structure operations within the new pooling and rendering logic to reduce computational complexity during scrolling.
| static pool: DataCell[] = []; | ||
|
|
||
| static acquire(): DataCell | undefined { | ||
| return DataCellPool.pool.shift(); |
There was a problem hiding this comment.
For performance, it's better to use pop() instead of shift() for acquiring a cell from the pool.
Array.prototype.shift() has a time complexity of O(n), where n is the number of elements in the array. This can be inefficient for a large pool of cells. In contrast, Array.prototype.pop() has a time complexity of O(1).
Using pop() will change the pool's behavior from a FIFO queue to a LIFO stack, which is a common and more performant approach for object pools.
| return DataCellPool.pool.shift(); | |
| return DataCellPool.pool.pop(); |
| const mountedDataCell = find( | ||
| allDataCells, | ||
| (cell) => cell.name === `${rowIndex}-${colIndex}`, | ||
| ); |
There was a problem hiding this comment.
The use of find inside this loop to locate the cell to be removed can lead to performance issues on large viewports. The complexity of this part of the code is O(M*N), where M is the number of cells to remove and N is the total number of visible cells (allDataCells).
To optimize this, you could create a Map of cells keyed by their name (${rowIndex}-${colIndex}) before the loop. This would reduce the lookup time to O(1) for each cell, making the overall complexity for removing cells O(M).
Example:
// Before the loop
const dataCellMap = new Map(allDataCells.map(cell => [cell.name, cell]));
// Inside the loop
const mountedDataCell = dataCellMap.get(`${rowIndex}-${colIndex}`);
👀 PR includes
✨ Feature
🎨 Enhance
🐛 Bugfix
🔧 Chore
📝 Description
🖼️ Screenshot
🔗 Related issue link
🔍 Self-Check before the merge