[DataGrid] Lower filter debounce delay #9712

romgrk · 2023-07-18T03:14:06Z

Closes #9657
Closes #9157

Turn SUBMIT_FILTER_STROKE_TIME into a prop and lower it to 50ms by default, and lower the QF equivalent.

packages/grid/x-data-grid/src/models/props/DataGridProps.ts

packages/grid/x-data-grid/src/hooks/utils/useTimeout.ts

mui-bot · 2023-07-18T03:19:53Z

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-9712--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	-57.7	316.2	168.2	111.12	127.809
Sort 100k rows	ms	633.3	1,119.1	633.3	918.9	165.362
Select 100k rows	ms	193.2	282.5	206.3	229.58	36.141
Deselect 100k rows	ms	136.6	371.1	208	224.74	78

Generated by 🚫 dangerJS against e6abe96

scripts/x-data-grid-premium.exports.json

cherniavskii

Thanks, @romgrk!
Quick filter feels much better with lower debounce 👍

packages/grid/x-data-grid/src/hooks/utils/useTimeout.ts

packages/grid/x-data-grid/src/models/props/DataGridProps.ts

scripts/x-data-grid-premium.exports.json

docs/data/data-grid/filtering/CustomMultiValueOperator.js

docs/pages/x/api/data-grid/data-grid-premium.json

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx

packages/grid/x-data-grid/src/models/props/DataGridProps.ts

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterPanel.tsx

cherniavskii · 2023-07-26T11:37:43Z

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx

-  React.useEffect(() => {
-    const itemValue = item.value ?? '';
-    setFilterValueState(String(itemValue));
-  }, [item.value]);


I believe this effect was covering the use case with a controlled filter model, where the filter model could change while the user has the filter panel open.
Without this effect, the filter input will be out of sync with the actual filter model:

With effect: https://codesandbox.io/s/gracious-khayyam-2xqw3n?file=/demo.tsx
Without effect: https://codesandbox.io/s/dreamy-glitter-h8h4yj?file=/demo.tsx

I've adapted the effect to respond to the model changes specifically, what do you think?

I'll apply to the other equivalent components if it LGTY.

The effect was also making sure the value is correctly synced with header filter (which is another rendered instance of GridFilterInputValue), removing it also impacted it too, as it's actually fromInput but it still needs to be synced 😄

Before:

After:

Ooh good catch, there's so many parts to this thing ^^

I've pushed a fix for that.

Thanks, works as expected now!

@romgrk It had another aspect which I missed catching 😄
Since the rendering of GridFilterInputValue is being done from GridHeaderFilterCell, the item change is incorrectly reflected.
I am thinking of disabling this optimization (fromInput) for the headerFilters, let me know if you have better ideas.

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterPanel.tsx

github-actions · 2023-07-27T12:43:21Z

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

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterPanel.tsx

MBilalShafi

LGTM, the filter response for both the quick filter and normal filter is almost instant now.
Feels much faster 🚀

oliviertassinari · 2023-08-14T17:10:48Z

Nice, this feels more responsive.

Can we create a new issue for the rest of #9657? The input lags behind now, similar to #9657 (comment). I imagine it why we can't use 50ms, and we have to stick to 150ms for now, and why we are using a debounce rather than a throttle. Or we could reopen it, as this PR gets us one step closer to do better than AG Grid.

oliviertassinari · 2023-08-14T17:16:30Z

packages/grid/x-data-grid/src/hooks/utils/useTimeout.ts

+    if (this.currentId !== 0) {
+      clearTimeout(this.currentId);
+      this.currentId = 0;
+    }


Would it be equality fast but less JavaScript code with?

Suggested change

if (this.currentId !== 0) {

clearTimeout(this.currentId);

this.currentId = 0;

}

clearTimeout(this.currentId);

It's a small cost but in some places it matters.
Details here: mui/material-ui#37512

I have a few of these utils that should be migrated to core at some point.

It's a small cost but in some places it matters. Details here: mui/material-ui#37512

quasarframework/quasar#15203 seem to have faced something similar. Fair enough.

I have a few of these utils that should be migrated to core at some point.

useLazyRef: it could be great to comment on it to link Lazy useRef instance variables facebook/react#14490 I guess it's why it's here. Preferred when doing something a bit heavy to initialize the ref. Done in b9018a4

useTimeout: yeah, I see how this could be used in quite more places in Core.

oliviertassinari · 2023-08-14T23:15:05Z

packages/grid/x-data-grid/src/hooks/utils/useTimeout.ts

+    if (this.currentId !== 0) {
+      clearTimeout(this.currentId);
+      this.currentId = 0;
+    }


It's a small cost but in some places it matters. Details here: mui/material-ui#37512

quasarframework/quasar#15203 seem to have faced something similar. Fair enough.

I have a few of these utils that should be migrated to core at some point.

useLazyRef: it could be great to comment on it to link Lazy useRef instance variables facebook/react#14490 I guess it's why it's here. Preferred when doing something a bit heavy to initialize the ref. Done in b9018a4

useTimeout: yeah, I see how this could be used in quite more places in Core.

oliviertassinari · 2023-08-14T23:15:33Z

packages/grid/x-data-grid/src/hooks/utils/useTimeout.ts

+export function useTimeout() {
+  const timeout = useLazyRef(Timeout.create).current;
+
+  useOnMount(timeout.disposeEffect);


Why not?

Suggested change

useOnMount(timeout.disposeEffect);

React.useEffect(timeout.disposeEffect, []);

useOnMount has a clear semantic. It also doesn't re-allocate the [] object on every render, so less work for the GC. Very small cost again, but these add up and end up creating more frequent and longer GC pauses if we allocate objects for no reason.

I'm a bit worried that it's not a problem worth an abstraction (1. It could be hard for team members to learn to use it. 2. When reviewing the code, it's someone new reviewers need to learn 3. I wouldn't be surprised if it will be missed, and even once known, the old habits will persist).

But I won't push against it, why not give it a try 👍

We had that discussion when I added useOnMount in some older PR, but since I've seen other members of the team adopt it, so I think it's beneficial. React hooks are a bit raw, having a small set of primitives added on top of them isn't a bad idea, imho.

A follow-up on #9712 (comment).

romgrk added 3 commits July 17, 2023 23:04

feat: make SUBMIT_FILTER_STROKE_TIME into a prop

66c12f4

feat

3bdf00c

feat

9795860

romgrk added component: data grid This is the name of the generic UI component, not the React module! feature: Filtering Related to the data grid Filtering feature labels Jul 18, 2023

romgrk requested review from MBilalShafi and cherniavskii July 18, 2023 03:14

romgrk commented Jul 18, 2023

View reviewed changes

packages/grid/x-data-grid/src/models/props/DataGridProps.ts Outdated Show resolved Hide resolved

romgrk commented Jul 18, 2023

View reviewed changes

packages/grid/x-data-grid/src/hooks/utils/useTimeout.ts Outdated Show resolved Hide resolved

lint

0d8babc

romgrk commented Jul 18, 2023

View reviewed changes

scripts/x-data-grid-premium.exports.json Outdated Show resolved Hide resolved

romgrk added 2 commits July 17, 2023 23:54

lint

776062d

build: docs:api

67fe5b4

cherniavskii reviewed Jul 18, 2023

View reviewed changes

romgrk added 4 commits July 18, 2023 16:56

refactor

db2c5d9

lint

8b6e63e

Merge branch 'master' into feat-quick-filter-delay

40ae023

refactor

01f92ac

romgrk mentioned this pull request Jul 18, 2023

[discussion] Preparing v7 #7902

Open

45 tasks

romgrk added 3 commits July 18, 2023 17:15

lint

810a028

lint

5d3c395

lint

6fe923f

romgrk requested a review from cherniavskii July 19, 2023 20:07

cherniavskii reviewed Jul 20, 2023

View reviewed changes

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx Show resolved Hide resolved

packages/grid/x-data-grid/src/models/props/DataGridProps.ts Outdated Show resolved Hide resolved

fix: input re-type bug

6a1ad56

romgrk commented Jul 26, 2023

View reviewed changes

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterPanel.tsx Show resolved Hide resolved

ci: run

a6cdda2

cherniavskii reviewed Jul 26, 2023

View reviewed changes

fix: add effect back

9185ab2

lint

08315bf

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 27, 2023

romgrk added 3 commits August 1, 2023 03:22

Merge branch 'master' into feat-quick-filter-delay

5b52b74

Merge branch 'master' into feat-quick-filter-delay

9bd9dee

build

5db6cbb

romgrk requested a review from cherniavskii August 1, 2023 07:54

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 1, 2023

cherniavskii reviewed Aug 2, 2023

View reviewed changes

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx Outdated Show resolved Hide resolved

packages/grid/x-data-grid/src/components/panel/filterPanel/GridFilterPanel.tsx Show resolved Hide resolved

fix: filter panel re-type bug

a42ae12

romgrk requested a review from cherniavskii August 3, 2023 23:49

romgrk added 2 commits August 11, 2023 14:37

fix

f1d05aa

lint

e6abe96

MBilalShafi approved these changes Aug 14, 2023

View reviewed changes

romgrk merged commit a1f6d7e into mui:master Aug 14, 2023
4 checks passed

romgrk deleted the feat-quick-filter-delay branch August 14, 2023 15:49

oliviertassinari added performance design: ux labels Aug 14, 2023

oliviertassinari reviewed Aug 14, 2023

View reviewed changes

oliviertassinari added design This is about UI or UX design, please involve a designer and removed design: ux labels Aug 18, 2023

oliviertassinari mentioned this pull request Aug 20, 2023

[data grid] Improve quick filter performance (INP) #9657

Open

oliviertassinari added a commit that referenced this pull request Aug 20, 2023

[core] Document a bit useLazyRef

b9018a4

A follow-up on #9712 (comment).

MBilalShafi mentioned this pull request Sep 14, 2023

[DataGridPro] Make default filter items have stable references in header filters #10338

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Lower filter debounce delay #9712

[DataGrid] Lower filter debounce delay #9712

romgrk commented Jul 18, 2023

mui-bot commented Jul 18, 2023 •

edited

Loading

cherniavskii left a comment

cherniavskii Jul 26, 2023

romgrk Jul 26, 2023

romgrk Jul 26, 2023

MBilalShafi Aug 10, 2023 •

edited

Loading

romgrk Aug 11, 2023

MBilalShafi Aug 14, 2023

MBilalShafi Sep 13, 2023

github-actions bot commented Jul 27, 2023

MBilalShafi left a comment

oliviertassinari commented Aug 14, 2023 •

edited

Loading

oliviertassinari Aug 14, 2023

romgrk Aug 14, 2023

oliviertassinari Aug 14, 2023 •

edited

Loading

oliviertassinari Aug 14, 2023 •

edited

Loading

oliviertassinari Aug 14, 2023

romgrk Aug 15, 2023

oliviertassinari Aug 20, 2023 •

edited

Loading

romgrk Aug 28, 2023

	useOnMount(timeout.disposeEffect);
	React.useEffect(timeout.disposeEffect, []);

[DataGrid] Lower filter debounce delay #9712

[DataGrid] Lower filter debounce delay #9712

Conversation

romgrk commented Jul 18, 2023

mui-bot commented Jul 18, 2023 • edited Loading

Netlify deploy preview

Updated pages

cherniavskii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MBilalShafi Aug 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 27, 2023

MBilalShafi left a comment

Choose a reason for hiding this comment

oliviertassinari commented Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

oliviertassinari Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Aug 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mui-bot commented Jul 18, 2023 •

edited

Loading

MBilalShafi Aug 10, 2023 •

edited

Loading

oliviertassinari commented Aug 14, 2023 •

edited

Loading

oliviertassinari Aug 14, 2023 •

edited

Loading

oliviertassinari Aug 14, 2023 •

edited

Loading

oliviertassinari Aug 20, 2023 •

edited

Loading