[Datagrid] Performance issue during scrolling with many TextField

[reigj1]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Scrolling and moving the datagrid makes the whole grid lag when using Autocomplete inside the cell.

Expected Behavior 🤔

Scrolling the datagrid should be smooth and without major lags.

Steps to Reproduce 🕹

Go to:
https://codesandbox.io/s/material-demo-forked-r4wv7?file=/demo.js
Try to move from left to right on the datagrid
The datagrid will lag a lot

Your Environment 🌎

Custom column code:

{
    field: "nameA",
    headerName: "Name A",
    disableClickEventBubbling: true,
    width: 200,
    renderCell: (params1) => {
      return (
        <Autocomplete
          options={["Susan", "Mark"]}
          defaultValue={params1.getValue(params1.id, "nameA")}
          getOptionLabel={(option) => option}
          getOptionSelected={(option, value) => option.value === value.value}
          style={{ width: "100%", paddingTop: 20 }}
          renderInput={(params) => <TextField {...params} variant="outlined" />}
        />
      );
    }
  } 

`npx @material-ui/envinfo`

  Don't forget to mention which browser you used.
  Output from `npx @material-ui/envinfo` goes here.

Order id 💳

[oliviertassinari]

It seems that TextField is simply too slow for this use case.

#569 (comment)

I would propose we close as "won't fix": Don't use the grid this way.

[dtassone]

Your renderCell functions are not memoized, and the components get redrawn at every render hence the issue.

const MyAutoComplete = React.memo((params) => {
  return (
    <Autocomplete
      options={["Susan", "Mark"]}
      defaultValue={params.getValue(params.id, "nameA")}
      getOptionLabel={(option) => option}
      getOptionSelected={(option, value) => option.value === value.value}
      style={{ width: "100%", paddingTop: 20 }}
      renderInput={(params) => <TextField {...params} variant="outlined" />}
    />
  );
});

//then 
...
  {
    field: "nameA",
    headerName: "Name A",
    disableClickEventBubbling: true,
    width: 200,
    renderCell: (params) => <MyAutoComplete {...params} />
  },
...

Check out
https://codesandbox.io/s/xgrid-render-autocomplete-sxi42

[oliviertassinari]

Check out https://codesandbox.io/s/xgrid-render-autocomplete-sxi42

@dtassone I don't feel any difference between the two codesandboxes. When I enable the FPS counter or use Chrome Dev tools performance feature I also see no differences.

Actually, in both cases with:

It's lags.

I also don't understand why React.memo would help. Intuitively, I would have expected it to harm slightly as it leads to more CPU cycles spent on comparing props for nothing. Consider the following: https://codesandbox.io/s/xgrid-render-autocomplete-sxi42?file=/demo.js. We get the same number of renders when we scroll, memo or not (different story when I use the keyboard navigation, but it's off-topic).

[oliviertassinari]

It's lags.

I have spent 10 minutes looking at why the performance issue as I wasn't satisfied with "use react memo". What I can see is a major rendering (user-agent, not React) step when scrolling starts, and when it stops:

This forced reflow is triggered by:

https://github.com/mui-org/material-ui-x/blob/fa346f0fbe3d9b9eea9bb403fe4675f544d6abf9/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts#L31-L33

[oliviertassinari]

This forced reflow is triggered by:

Commenting on these 3 lines leads to interesting behavior.

diff --git a/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts b/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts
index 86943b6a..ad533f7e 100644
--- a/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts
+++ b/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts
@@ -28,9 +28,9 @@ export function useGridScrollFn(

       if (renderingZoneElementRef && renderingZoneElementRef.current) {
         logger.debug(`Moving ${renderingZoneElementRef.current.className} to: ${v.left}-${v.top}`);
-        if (renderingZoneElementRef.current!.style.pointerEvents !== 'none') {
-          renderingZoneElementRef.current!.style.pointerEvents = 'none';
-        }
+        // if (renderingZoneElementRef.current!.style.pointerEvents !== 'none') {
+        //   renderingZoneElementRef.current!.style.pointerEvents = 'none';
+        // }
         // Force the creation of a layer, avoid paint when changing the transform value.
         renderingZoneElementRef.current!.style.transform = `translate3d(-${v.left}px, -${v.top}px, 0)`;
         columnHeadersElementRef.current!.style.transform = `translate3d(-${v.left}px, 0, 0)`;

We remove the upfront major rendering step, but it seems we add more rendering effort along the way:

---

Overall it seems smoother and to have more creative hover feedback => better?

The only other leverages I can think of are:

1. simplify the TextField to require less rendering effort, I have no idea where we could start
2. render a placeholder in the cells when scrolling instead of the text field

"won't fix" could still be an acceptable outcome for this issue.

[dtassone]

The goal of the memo is to avoid to redraw the cell so I would have expected slightly better perf. It would be easier to see maybe with more data...
Interesting found on the pointer-events, are you saying it creates issues? It should make things smoother as it blocks the over events...

[oliviertassinari]

to avoid to redraw the cell

@dtassone Could we expand on the why? By redraw, do you mean at the browser level? I really can't connect the dots.

It would be easier to see maybe with more data...

We could consider what happen with more data indeed, however:

• The users doesn't seem to render more data to see the performance issue
• Why would more data make a difference? Maybe a codesandbox with a reproduction could illustrate it.

are you saying it creates issues? It should make things smoother as it blocks the over events

I would recommend you try the change out (apply the git diff) in your local environment to get a feeling of the difference of rendering experience. It definitely causes an expensive redraw before any scroll action can be performed by the browser. I don't know if it's the problem the author is talking about, still it's feels laggy with a low-end CPU.

The other advantage seems to help with the hover feedback on the cells, it feels more responsive (you get the hover styles as you scroll for instance)

[dtassone]

I think before the performance fixes #1513, the pointer events helped, as mouse events would make cells to rerender due to different cell params. Now it seems fine, over events should not cause cells to rerender therefore we can leave the pointer events while scrolling. I had a play locally and it seems smooth without.

For the text box issue, I have updated the component to render basic input text and it definitely feels much faster.
It seems that the autocomplete takes a while to render and that it causes the perf bottleneck.
https://sxi42.csb.app/

It doesn't seem to be an issue with the grid.

[oliviertassinari]

Now it seems fine, over events should not cause cells to rerender therefore we can leave the pointer events while scrolling. I had a play locally and it seems smooth without.

Could we remove it then? :D

diff --git a/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts b/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts
index 86943b6a..ad533f7e 100644
--- a/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts
+++ b/packages/grid/_modules_/grid/hooks/utils/useGridScrollFn.ts
@@ -28,9 +28,9 @@ export function useGridScrollFn(

       if (renderingZoneElementRef && renderingZoneElementRef.current) {
         logger.debug(`Moving ${renderingZoneElementRef.current.className} to: ${v.left}-${v.top}`);
-        if (renderingZoneElementRef.current!.style.pointerEvents !== 'none') {
-          renderingZoneElementRef.current!.style.pointerEvents = 'none';
-        }
+        // if (renderingZoneElementRef.current!.style.pointerEvents !== 'none') {
+        //   renderingZoneElementRef.current!.style.pointerEvents = 'none';
+        // }
         // Force the creation of a layer, avoid paint when changing the transform value.
         renderingZoneElementRef.current!.style.transform = `translate3d(-${v.left}px, -${v.top}px, 0)`;
         columnHeadersElementRef.current!.style.transform = `translate3d(-${v.left}px, 0, 0)`;

It seems that the autocomplete takes a while to render and that it causes the perf bottleneck.

@dtassone Yeah, there is some truth to it. First, I think that we can rule out the Autocomplete and reduce the issue to the TextField as React doesn't render anything, and only the TextField is visible. See the difference when scrolling. Chrome spend more time "rendering":

TextField: https://r4wv7.csb.app/

---

raw input: https://sxi42.csb.app/

I suspect Chrome lazy renders the content as it comes visible on screen (when scrolling). And since TextField is more complex, it's slower.

[dtassone]

With Ant design autocomplete https://qhlrp.csb.app/

[oliviertassinari]

I have built more or less the same reproduction with v5 and Autocomplete: https://codesandbox.io/s/material-demo-forked-o3vyd?file=/demo.js. Now, it's in dev mode, so it doesn't tell too much. The same version in prod mode:

• Antd https://codesandbox.io/s/material-demo-forked-qhlrp?file=/demo.js -> https://csb-j38kb.netlify.app/
• Autocomplete https://codesandbox.io/s/material-demo-forked-o3vyd?file=/demo.js -> https://csb-o3vyd.netlify.app/

None seems great. We might want to offer option 2 of #1812 (comment)

2. render a placeholder in the cells when scrolling instead of the text field

I could find this in react-virtualized, with a isScrolling prop: https://bvaughn.github.io/react-virtualized/#/components/WindowScroller.

[edudar]

JFYI, playing with pointerEvents might have rather unexpected side effects. I'm using testing library and got a handful of test failures after upgrading to the latest version that introduced support for pointer-events: 'none' because nothing in the grid was suddenly clickable. Upgrading to alpha31 with #1829 fixes this because 'none' is gone and elements are considered clickable again.

[dancernogorsky]

  {
    field: "nameA",
    headerName: "Name A",
    disableClickEventBubbling: true,
    width: 200,
    renderCell: (params) => <MyAutoComplete {...params} />
  },

@dtassone I was testing this out and noticed if you pass in {...params} to the component, it will re-render every time. This is because you are passing the api, and entire grid state which is going to be different every time.

I tried just passing in {params.row}, and it seems to be working as expected. Still needs to be improved, but atleast I am seeing a bump in performance.

[oliviertassinari]

@dancernogorsky Are you using the latest version? If you have a reproduction it would help. This should no longer be true with the latest version.

I'm closing as the conclusion was that the issue is not with the data grid, but the text field.

[dildamaara]

Not sure why it's closed. It's still happening.
Just use any component within renderCell and it will slow down on scrolling horizontally/vertically.
Issue is not with Textfield

[sebabratakundu]

  {
    field: "nameA",
    headerName: "Name A",
    disableClickEventBubbling: true,
    width: 200,
    renderCell: (params) => <MyAutoComplete {...params} />
  },

@dtassone I was testing this out and noticed if you pass in {...params} to the component, it will re-render every time. This is because you are passing the api, and entire grid state which is going to be different every time.

I tried just passing in {params.row}, and it seems to be working as expected. Still needs to be improved, but atleast I am seeing a bump in performance.

@dancernogorsky Yes, non-primitive props will always trigger re-rendering. So we have to memo its value before pass it to our custom comp. In this case its not possible. so we can pass the prop and inside our component and memo it , and then return the memo render result. This helps me to get a performance boost by pausing down the re-rendering of the elements which are returned by our custom comp.

Here DataTableActionButton is the custom element. its re-rendering everytime. but the returned compent result is memoized