[DataGrid] Vertical scrolling performance
Follow-up of https://github.com/mui/mui-x/pull/10059#issuecomment-1904425888
Restore the vertical scrolling performance.
Changes:
- Remove the
transform: translate(...)style on cells and use an offset filler cell instead - Remove the virtual scroller render zone DOM node, keep only the virtual scroller content and rename it to "centerContainer" to match the "topContainer" and "bottomContainer"
Deploy preview: https://deploy-preview-11924--material-ui-x.netlify.app/
Generated by :no_entry_sign: dangerJS against 35ff0de049e4744b348efaf3244f333691c7cf2b
Follow-up of https://github.com/mui/mui-x/pull/10059#issuecomment-1904389411
Once this style removed, the computation bottleneck seems to then move to recalculateStyles, this is very odd to me:
More in context:
I don't know why. I ran out of time I can allocate to this 😄
I've inspected a bit the "recalculate styles" phase, it's triggered by the .removeChild to remove rows that are scrolled out of the viewport. Leaving the rows in (while still performing the rest of the virtualization logic) yields dramatic improvements:
But this seems fairly logical to me: because the rows that are removed are the first childs of their container (these tests were all scrolling down), the browser invalidates the layout of all the siblings that come after. In other words, all the rows are invalidated because they all come after the first rows, which are removed. I'll try to figure if we can find a way to have the browser avoid redoing the style/layout for all those elements.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
I have reduced the scope of this PR, I'll just do the removal of of the transform style on the cells, which dimishes the number of layers and the Layerize phase in the browser rendering.
| before | after |
|---|---|
The style recalculation phase is still taking too long, because the number of invalidated elements is higher than before. I still need to experiment more to understand how to reduce that cost. From this exploration I have however learned that removing nodes from the DOM is actually very expensive: about 60-80% of the virtualization DOM changes cost comes just from removeChild() calls. I have benchmarked ag-grid, and they also have a similar performance profile. They do have a much smaller "Recalculate styles" phase though, which might have to do with the fact that the use transform to position the rows, which could allow for style invalidation to not touch row siblings (aka all the rows). But their styles are also much cleaner, they use flat class selectors everywhere, which also facilitate style invalidation for browsers.
Anyway, this is ready for review.
Great, a nice step forward 👍. From what I can benchmark, this makes us not too far from where we were before #10059. I see a reduction of about 100ms spent in "Rendering", for every 1,000ms of rendering.
I was curious, since this change, when we enable
we see the whole grid container light up 💡:
https://github.com/mui/mui-x/assets/3165635/18506f75-2600-4b46-90a6-20cc8d7382fc
Options:
- I tested if adding a
translate3don each row (rather than on each cell before this PR), helped. It doesn't seem to. We spent less time in the Paint phase, but more in the Layout phase, overall it's more. So doesn't work. - What AG Grid and https://github.com/adazzle/react-data-grid seem to do to avoid that paint is to recycle their DOM nodes. We don't, we unmount/remount them in the DOM. I didn't benchmark it, but maybe it's faster to avoid the pain and mutate the DOM attributes than to remove/inject a new DOM node.
- If 2. speed JavaScript, then it could also be combined with this one: giving each row a
position: absolute+top: x, it seems to skip the Paint when removing/adding DOM nodes at the top of the container. I would expect this to speed things up if we can easily set these absolute coordinates.
More in context:
I don't know why. I ran out of time I can allocate to this 😄