-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[DataGrid] Performance: Improve virtualization overscan logic #11344
Comments
Option 2 sounds better to me |
Just to clarify, the points above aren't an either/or, I think we should do both. To add a justification for point 1, some tables might have columns that are very wide, say 600px. If there is a 600px column with a 10px area barely poking to the left of the grid's viewport, it doesn't makes sense to fill two additional columns, because we may be filling an overscan window of say 1000px total, where we only really need say 200px for a scroll+render to complete without the user having noticeable white areas. |
I would definitely support increasing rowBuffer default value from 3 to something higher (max 10). The proposed changes in this issue sounds even better. But I don't know if we can pull it off with a simple enough implementation (for the bundling size, computation cost, bug surface to be worth it), so to see. |
The problem with a simple 3 -> 10 change for the row buffer is that it would cause horizontal scroll to be much slower, because each time we scroll by 1 column we need to render 20 more cells outside the viewport (10 rows above & below), whereas the cost to scroll by 1 column is only 6 cells outside the viewport right now. Bundle size & computation cost aren't an issue, we only need one function that takes as input the scroll direction, scroll speed, current render context & dimension, and returns the new render context. Finding a good heuristic for that might be a bit tricky but sounds doable. |
I think this is a breaking change, because we need to either change the @mui/xgrid Wdyt about including it in v7? |
I have added a first implementation for this in #12353. |
How did we do @romgrk? |
Oh nice, a lot less white screens 👌 |
Yes 🌈 This is actually more UX than performance because in that PR I've spent some of the performance gains we've made elsewhere to display more cells where we need them to be. There is still more performance improvements we can do which will hopefully enable us to use the CPU for what we need it, hopefully I can get to them soon. |
I have made a quick (took 1 hour) thread to communicate about these changes https://twitter.com/olivtassinari/status/1774145311419634101. |
Right now, the grid virtualization logic keeps 3 buffer rows and buffer cols around the rendered cells (the props
rowBuffer
andcolumnBuffer
. But because columns are wider in their dimension than rows are in theirs, it means we allocate more resources for horizontal scrolling than for vertical scrolling.To improve the situation we should:
This proposition would synergize well with the scroll locking of #11230.
Search keywords:
The text was updated successfully, but these errors were encountered: