[data grid] Performance: separate row cleanup

[romgrk]

Stacked on #12013 and #12019 to explore #11866

Experiment with scrolling performance.

The idea behind this is that most of our scroll event callback runtime is spent removing DOM nodes, while appending new ones is much less expensive. This is a characteristic of the DOM spec and outside of our control. The graph below shows the amount of time spent adding new rows (green) and removing old rows (red). Other virtualized grids such as AG-grid have the same performance profile. The goal here is to try to remove the red section from the flushSync path, and try to pay that cost at a different moment, possibly with requestIdleCallback.

Before: https://next.mui.com/x/react-data-grid/virtualization/#column-virtualization
After: https://deploy-preview-12023--material-ui-x.netlify.app/x/react-data-grid/virtualization/#column-virtualization

Changes

• Rendered rows are stored in a RowIntervalList, an interval list data structure that holds row indexes and their render context.
• Avoid colSpan computations in scroll event, those are very expensive and unnecessary unless there is a .colSpan somewhere
• Rows and detail panels are now position: absolute; top: ... positioned.

[mui-bot]

Deploy preview: https://deploy-preview-12023--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against f82c480

[romgrk]

Index should be -1, not 0

[romgrk]

Unmount cleanup logic?

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[romgrk]

I've finished implementing the cleanup logic, here is a quick performance comparison. The new implementation does its cleanup after 1s without user interaction.

Kooha-2024-02-22-23-04-28.webm

[oliviertassinari]

I've finished implementing the cleanup logic, here is a quick performance comparison. The new implementation does its cleanup after 1s without user interaction.

@romgrk When I run #11866 (comment), the benchmark, and UX is noticeably worse on this PR. Not sure it's working 😁

[romgrk]

Weird results, this PR makes it smoother for me. Any chance you can send me a recording and a profile of the benchmark? (the "Save profile..." button in the Performance tab)

[cherniavskii]

@romgrk I can reproduce this as well. It gets slower the more you scroll:

https://next--material-ui-x.netlify.app/x/react-data-grid/#pro-plan

Screen.Recording.2024-02-26.at.12.46.20.mov

---

https://deploy-preview-12023--material-ui-x.netlify.app/x/react-data-grid/#pro-plan

Screen.Recording.2024-02-26.at.12.46.49.mov

Here's the profile: https://drive.google.com/file/d/1in_kUcfi9s4_zv06mobVPAPNsXjCzPTT/view?usp=drive_link

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[romgrk]

I WAS WRONG.

Anyway long story short I have a benchmarking chrome user-profile that is usually clean (no react-dev-tools extension or anything else that affects runtime performance) so I can benchmark precisely. Well I had activated the devtools experimental flag Timeline: invalidation tracking a few weeks back for the other performance issues (style recalculation). Turns out it was slowing down considerably the removeChild operation but only in profiling mode, which I had been running in the whole time while benchmarking this PR. So yes, the row cleanup thing is useless.

There's a few other performance improvements on this PR that I'll pick up separately.

[kurtextrem]

@romgrk maybe worth reporting as crbug? I'm not sure if it's expected that removeChild becomes slower with invalidation tracking enabled

[romgrk]

Don't think so. Invalidation doesn't just get the experimental warning:

It gets an additional 2nd warning about their unstability: