DataGrid - Scrolling performance

[romgrk]

This is a summary of the observations I've made regarding scrolling performance. The performance profiles were done with a datagrid filled with either <button> elements or MUI <Button> components, on an Intel Core i7 10th gen CPU, and are there to give an approximate idea of the CPU costs. They all show one scroll event call.

1. Re-renders

This is the biggest cost by far. Currently, we React.memo the cells but not the rows, which means that on every scroll event all our rows are re-rendered. Below is a flamegraph where the GridRow render calls are highlighted. It is very apparent that we're doing more work than we need to do, we must therefore focus on avoiding it.

Without memo	With memo

Adopting React.memo: issues & solutions

It is currently not straightforward to adopt React.memo because our internal API state uses an imperative mutable state that lives inside of a React context, but outside of React's reach. From React's POV, the API handle never changes. The reason why the datagrid currently works despite the external state is that we re-render more than we need to, which gives React a chance to read the API state often enough. To make the React.memo optimization work, we must therefore make our API state accessible to React. To do so, the plan is to use a Redux-like store within React, whose core logic is demonstrated here. In a few words, when the API state changes, it will publish an update to its listeners. The listeners will conveniently be created by useGridSelector(...), which will also be storing the selected value in a React.useState, which will allow React to update components granularly. This architectural change will also unlock improvements in many other use cases, for example it would become reasonable to have a cell focus change event that would only re-render the 2 cells that changed focus.

We could also apply the same pattern to the grid root props, where components could select a slice of the props and update based on it. Currently, whenever the root props change, any component that uses them needs to re-render, regardless if the props they're actually using have changed.

// With the proposed change, this component would only re-render if `props.slots.cell` changes
function GridRow(props) {
  const CellComponent = useGridRootPropsSelector(props => props.slots.cell);
  // ...
}

Plan

I plan to start working on these changes as soon as possible, unless there are objections.

---

NOTE: The next points are presented for completeness' sake, but are by far less important than the first one above.

2. _objectSpread2

Babel's _objectSpread2 is what { ...object } desugars to. Unfortunately, the ECMA spec defines the spread operator in terms of Object.defineProperty, which means it is nearly an order of magnitude slower than Object.assign. And our codebase is littered with it.

Scroll event call, with React.memo optimization

Possible improvement

While there are subtle differences between { ...o } and Object.assign({}, o), it might be worth considering adding a babel transform to turn our spread operators into Object.assign.

Plan

I don't plan to implement this change unless there is positive feedback from the team.

3. MUI Core components & styling

While not technically related to the datagrid, I'll point out that using MUI components is very expensive. The biggest cost they have is by far the styling engine. The report below represent one scroll event callback with a datagrid filled with <Button> components.

Plan

I'll discuss with the core team, they are investigating griffel and we should also switch to it if/when they make the switch in core.

[oliviertassinari]

These points make sense to me 👍. I didn't dive deep.

---

To measure success, I think that we need to measure two things:

1. FPS => aim for 120 FPS at all time
2. Area of white screen => no white areas.

In the very first implementation of the data grid, we used a trick that would not scroll the rows during very fast scroll events, to guarantee 2. We have reverted this behavior since https://mui.com/x/react-data-grid/#commercial-version:

Screen.Recording.2023-05-15.at.23.15.25.mov

AG Grid is taking an opposite UX tradeoff, while we debounce the rendering (at least it's how it feels, we abord rendering if another update tells us to stop), it feels like they throttle, what's displayed is wrong (feels like it's lagging), but not white areas:

Screen.Recording.2023-05-15.at.23.24.19.mov

https://ag-grid.com/example/ You can most feel the throttle behavior around the end, it takes the grid one or two rerenders to display the final correct rows.

I think that the direction they took could maybe make sense (no sure). To pull this off, I suspect that they keep the current rows in their current viewport position for the next pain until the new rows are rendered, they might also predict the future scroll position based on speed (with a low pass filter http://phrogz.net/js/framerate-independent-low-pass-filter.html). I mean, see this recording, how is it possible that the same rows are visible for different scroll positions:

Screen.Recording.2023-05-15.at.23.29.11.mov

So overall, what we could measure with 2. is the speed at which end-users can scan the results without having to wait for the paint:

Screen.Recording.2023-05-15.at.23.33.41.mov

[romgrk]

Yeah I've already investigated this behavior as well. I've been considering if we should remove the React.flushSync to prevent scroll lagging, but the trade-off would be that we get white areas.

mui-x/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx

Lines 430 to 432 in ebf2715

	ReactDOM.flushSync(() => {
	updateRenderContext(nextRenderContext);
	});

You're raising very interesting ideas, I like trying to predict the future position. One concern I have here is that we currently have trouble rendering too many rows at once, which is the source of scroll lag. I fear that trying to predict the position will only add rows to render. Once I have completed the raw performance improvements above, I will put some research into this to see if we can find an implementation that improves the perceived scroll latency.

[romgrk]

I've also been thinking of a way to control rendering scheduling. For example, if we can render additional rows one at a time, then we are more likely to be able to fit the rendering within one browser render frame budget (~16ms). We'd be doing the same amount of work (maybe slightly more) but we would avoid causing frame drops, thus avoid lag. This might require an exotic implementation though.

If we want to be really clever about it, we could also further refine rendering to one cell at a time, for low-end devices that have less CPU power. And we could benchmark the rendering cost at runtime and only render about the amount of cells that fit within a frame for a particular device.

There isn't an official way to start work at the start of a frame, but in practice this generally works:

function afterAnimationFrame(fn: Function) {
  requestAnimationFrame(() => {
    setTimeout(fn)
  })
}

[romgrk]

@oliviertassinari I've been exploring this problem further. One thing is clear, at some point we don't have the computational resources to do both 1 and 2 (60 FPS and no white areas). If we need to pick which one of them we prioritize, which one is most important?

I feel like 60 FPS is best, and no white areas is secondary to that.

[oliviertassinari]

If we need to pick which one of them we prioritize, which one is most important?

@romgrk This sounds to me like a UX design tradeoff cc @gerdadesign. Going for 60 FPS over a white area makes more sense for me as well. In the idea that it's always better to give back the main thread control once we reach the point where it needs to hand to not drop a frame.

Then it's a matter of: can the grid render the next row faster or slower than the person can scan it? We want it to be faster, so the person's ability to scan rows sets the threshold on how fast they are scrolling.

When scrolling so quickly that someone wouldn't be able to see a given row scroll, I think that debouncing the scroll makes a lot of sense, what AG Grid seems to have done. We actually want to give them time to see what they are doing 😁, hence debounce (well, keep the rows in the same position on the screen for like 160ms rather than 16ms).

[romgrk]

Makes sense. Debouncing sounds feasable when the user scrolls via scrollbar (which I've been exploring in #9171), but I don't see how we'd do it for wheel or touchmove scrolls, unless we try to compute the scroll amount from the event.delta(X|Y) but I've never made that work as well as native.

I've also been thinking about rendering the cell/row borders with a repeating image background (something like a computed SVG or maybe a CSS pattern), this would allow the browser to render something inside the white areas rather than nothing. The perceived lag would probably be lower if we don't have fully white areas. This of course applies only if there are no 'auto' height rows. I'll explore some more in #9171 and post conclusions there.

[cherniavskii]

the plan is to use a Redux-like store within React, whose core logic is demonstrated here. In a few words, when the API state changes, it will publish an update to its listeners.

I wonder what would these changes mean for the apiRef, which is our public imperative API:

1. Currently, the grid state is stored in apiRef.current.state. You can access it both inside and outside of the grid - see https://mui.com/x/react-data-grid/api-object/. What would be the alternative once we move the state to the context?
   To give a specific example, here's a demo that is using apiRef outside of the grid: https://mui.com/x/react-data-grid/scrolling/#scrolling-to-specific-cells
2. API methods defined in apiRef get the state from apiRef.current.state. Once the state is moved to the context, how would the methods access the state? Something like this?

   +const columnMenu = useGridSelector(StateContext, state => state.columnMenu)
   
   const toggleColumnMenu = React.useCallback<GridColumnMenuApi['toggleColumnMenu']>(
     (field) => {
   -    const columnMenu = gridColumnMenuSelector(apiRef.current.state);
       if (!columnMenu.open || columnMenu.field !== field) {
         showColumnMenu(field);
       } else {
         hideColumnMenu();
       }
     },
   -  [apiRef, logger, showColumnMenu, hideColumnMenu],
   +  [apiRef, logger, showColumnMenu, hideColumnMenu, columnMenu],
   );

3. Backward compatibility. Given that apiRef.current.state is part of our public API, we cannot remove it in v6.
   This gives us 2 options:
   a) Move the state to the context in v6 and update apiRef.current.state with the latest state value each time it changes for backward compatibility. The risk here is that once we start memoizing more aggressively, some components using apiRef.current.state might not get rerendered.
   b) Move the state to the context in v7 and remove apiRef.current.state.

[romgrk]

1. There would be no change at that level, the state stays where it is and we're not adding another context. All that needs to happen at this time is that we add a Store instance on apiRef.current. The store instance is really just a pub/sub interface, that follows React.useSyncExternalStore requirements (in case we use it, but also because it's more idiomatic to follow what React does). (Link to our other discussion, for the others). We could also just add the subscribe and getSnapshot functions directly on the apiRef, what's important is to have those interface methods.
2. There are two use-cases: imperative access, and render access.
   a) It will still be allowed to use gridColumnMenuSelector(apiRef.current.state), but this implies that there is no reactive binding between that part of the state and the component/location where such access is made.
   b) If a component needs to re-render when part of the state changes, then it becomes mandatory to use useGridSelector(apiRef, gridColumnMenuSelector).
3. Answered by the points above. I don't plan to introduce breaking changes. The only thing that could be considered a breaking change is that we re-render less often, and our excessive re-rendering is a public & observable behavior that some users might rely on.

[cherniavskii]

There would be no change at that level, the state stays where it is and we're not adding another context. All that needs to happen at this time is that we add a Store instance on apiRef.current.

Gotcha, thanks for the clarification!

[cherniavskii]

2. _objectSpread2
   Babel's _objectSpread2 is what { ...object } desugars to.

I think we can drop this plugin:

mui-x/babel.config.js

Lines 142 to 143 in 52330d2

	// IE11 support
	'@babel/plugin-transform-object-assign',

We clearly don't support IE 11 anymore (we started using ResizeObserver recently, which is not supported by IE 11)

[oliviertassinari]

It's for the legacy bundle: https://mui.com/material-ui/guides/minimizing-bundle-size/#legacy-bundle. Yeah, not sure we need to keep this bundle.

[romgrk]

That plugin doesn't transform the spread operator, it transforms Object.assign. We can remove it, but it won't affect _objectSpread2.

[cherniavskii]

Right, the spread operator is probably transformed by @babel/preset-env

[m4theushw]

How did you measure the performance? Inside the benchmark folder there's a demo that runs the DataGridPro with 100k rows and 100k columns. I added it to compare our performance with other vendors. I ran it again now and the scroll is terrible. It used to be much better before. You can see in #2683 the metrics when I added the script and in #2673 the metrics after rewriting the virtualization logic. Here're the metrics today:

DataGridPro:
  Vertical scroll:
       Min: 0.882 fps (σ = 0.078)
       Max: 55.917 fps (σ = 1.158)
    Median: 1.795 fps (σ = 0.167)
      Mean: 11.794 fps (σ = 1.001)
  Horizontal scroll:
       Min: 0.969 fps (σ = 0.088)
       Max: 54.314 fps (σ = 0.958)
    Median: 2.143 fps (σ = 0.132)
      Mean: 16.42 fps (σ = 1.497)

I don't change my machine since 2021. Among the features we released something impacted the scrolling performance.

[romgrk]

I've done a quick profile of the benchmark you're talking about, it seems that getRows is doing expensive work, it's calling calculateColSpan a lot. I can investigate this issue further after I complete the current round of improvements.

[m4theushw]

While not technically related to the datagrid, I'll point out that using MUI components is very expensive. The biggest cost they have is by far the styling engine.

The issue with the styling engine is because emotion serializes all styles to derive a key for caching, however, it does this every time the component is rendered. Making cells and rows a styled component would make the problem even worse, so we moved their styles to GridRootStyles. Since the virtualization component has its own state, and it's a child of GridRootStyles, we shouldn't see mentions to emotion internals when scrolling in a simple demo (no features that have custom cells, e.g. aggregation). In the past I tried using css instead of styled to improve the performance for features with custom cells but it would be too complex to implement. I also thought about improving emotion to memoize the serialized styles.

[oliviertassinari]

We can connect this problem with mui/material-ui#34826

[martinhonza]

Hey, regarding scrolling performance, wouldn't it be possible to make column virtualization available also for rows if their height is set to auto ?
Currently the column virtualization get disabled, but It seems to me that it could be recalculated on initialization and then only when data are changed ?

[romgrk]

I'm not sure I understand. If you have an issue to report, open an issue with an example.