[DataGrid] Skip filtering if model didn't change

[m4theushw]

Related to #8225

We're applying the filter whenever the model changes, which means that only opening the panel already triggers the filtering, even though the filter item has no valid value yet. With 100k rows this causes a perceptible freeze as can be seen in https://master--material-ui-x.netlify.app/x/react-data-grid/#commercial-version and reported in the issue above. This PR skips the filtering step if the new filter model, containing only items that have a value or don't need one, is different from the last time the filtering ran. As can be see below the performance improved a bit. I wished we could properly fix this issue by keeping a copy of the filter model used for the filter panel and only calling onFilterModelChange when the value is changed.

The comparison below was done against https://mui.com/x/react-data-grid/#commercial-version by measuring the time to change the column inside the filter panel.

Before:

After:

[mui-bot]

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-9112--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	309.1	470.8	358.3	383.96	64.335
Sort 100k rows	ms	428.2	913.6	750.7	702.02	192.283
Select 100k rows	ms	196.5	281.1	198.1	216.96	32.589
Deselect 100k rows	ms	128.7	298.9	212.8	208.96	57.273

Generated by 🚫 dangerJS against a02d4ad

[romgrk]

I'll just comment because I'm not familiar enough with that code yet. But I generally prefer to fix this sort of problem by avoiding touching the model until it needs updating, by keeping a local version of the model in the editing component that is only written to the model once it's ready to be written. Then again, I don't know the code so maybe this approach is easier, in which case this PR LGTM.

[m4theushw]

It's opt-in because the filtering state is also used to track which groups are expanded or not. If we always run the check, when rowExpansionChange is fired it may not update the state. Maybe after #8671 we can revisit this part.

[MBilalShafi]

Yes, I think after #8671 is merged, we can make it true by default.

[github-actions]

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

[karanaditya993]

Hey @m4theushw @flaviendelangle we are running into issues where the onFilterModelChange callback is fired when you first open the filter panel

Do ya'll have a general estimate as to when this fix will go out? Thanks for your work on this, really appreciate it

[m4theushw]

@karanaditya993 I don't have any estimation of when this PR will be merged, but note that it doesn't solve the problem you mentioned. When opening the filter panel and filterModel.items is empty, onFilterModelChange will still be called. It's only not called now if the model is the same from the last time that the filtering ran.

[karanaditya993]

@karanaditya993 I don't have any estimation of when this PR will be merged, but note that it doesn't solve the problem you mentioned. When opening the filter panel and filterModel.items is empty, onFilterModelChange will still be called. It's only not called now if the model is the same from the last time that the filtering ran.

@m4theushw interesting...is this intentional? It seems like an unnecessary call (especially if onFilterModelChange is what dictates an external API call for server-side filtering) to make when you initially open the filter panel.

The idea here is that we'd want to only have the onFilterModelChange fired if you indeed do change any filters on the filter panel, but if they are initially being opened (and instantiated with empty values for the filter), calling onFilterModelChange seems redundant?

[karanaditya993]

I'm also noticing that when you open the filter panel from any of the columns (via the action menu on the column), the filterModel.items are never empty because they contain the field and operator of the column that you opened the panel from, even if the filter value is blank...so perhaps this PR does fix our use case

I will keep any eye out on the next few MUI releases to see if this is part of the release. In the meantime, I might copy paste the code you have here on an experimental fork to see if it resolves the issue we're having

Thanks again for your help and quick reply!

[github-actions]

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