Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
oliviertassinari opened this issue Jul 12, 2023 · 12 comments · Fixed by #9712
Open

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

oliviertassinari opened this issue Jul 12, 2023 · 12 comments · Fixed by #9712
Labels
component: data grid This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer enhancement This is not a bug, nor a new feature feature: Filtering Related to the data grid Filtering feature performance

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 12, 2023

Summary 💡

We have a 500ms input debounce that feels like the biggest chunk of time where end-users are waiting for the data grid to return results. I believe that even with an instant filtering, this denounce makes it impossible for the search input to feel delightful.

I suspect that we could fix it like this:

  • lower the debounce to 50ms, we would save 450ms! (0ms would probably be fine too), for comparison, the description of [DataGrid] Filtering performance #9120 talks about a win of up to 300ms.
  • turn the search to be performed in chunks, to yield to the main thread every now and then
  • start the search as soon as the end of the 50ms debounce but if a new search is input before the end of the previous one, interrupt it and restart from scratch.

Examples 🌈

https://mui.com/x/react-data-grid/filtering/quick-filter/

Motivation 🔦

No response

Order ID 💳 (optional)

No response

@oliviertassinari oliviertassinari added performance 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 design: ux enhancement This is not a bug, nor a new feature labels Jul 12, 2023
@MBilalShafi MBilalShafi changed the title [data grid] Reduce filter denounce from 500ms to 50ms [data grid] Reduce filter debounce from 500ms to 50ms Jul 12, 2023
@romgrk
Copy link
Contributor

romgrk commented Jul 12, 2023

Linked #9157

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 12, 2023

A quick performance benchmark to illustrate the problem. In both cases 100,000 rows, and using the quick filter search

AG Grid

Screenshot 2023-07-13 at 00 22 27

https://www.ag-grid.com/example/

Screenshot 2023-07-13 at 00 20 00

From the last key down to the first frame, we wait 277ms. It feels like they don't have any debounce at all, but it's doesn't feel great, the input update is lagging, ewww 🙈

MUI X

Screenshot 2023-07-13 at 00 23 14

https://mui.com/x/react-data-grid/#commercial-version, using a modified version of the demo to add

        slots={{ toolbar: GridToolbar }}
        slotProps={{
          toolbar: {
            showQuickFilter: true,
          },
        }}
Screenshot 2023-07-13 at 00 08 10

From the last key down to the first frame, we wait 1,430ms.

With the work on #9120 and related to it, I imagine we could move from 894ms spent on JS, down to 194ms (for those not doing the filter server-side), we would then get the wait from 1430ms to 730ms. By reducing the debounce, we could save another 450ms, going down to 280ms. At that point, it will be great, making you want to actually explore the data 👌😍

@ithrforu
Copy link
Contributor

What if replace debounce with useDefferedValue? It helps us a lot with "heavy" inputs. Maybe it's similar situation.

@oliviertassinari
Copy link
Member Author

@ithrforu useDefferedValue would solve AG Grid's problem (the input text that lags behind). In our case, I don't know, we could also consider a throttle, like it feels Netflix https://www.youtube.com/watch?v=FAZJsxcykPs or even our Algolia search on the docs is doing (though, they can run queries in parallel in the server, we can't really client side).

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

oliviertassinari commented Aug 20, 2023

I'm reopening. We had a step forward in #9712, but:

  1. The input lags now, similar to [data grid] Improve quick filter performance (INP) #9657 (comment). This doesn't feel responsive.
  2. We can't start searches without stopping them in the middle. We could save a lot of work with large tables.
  3. The debounce is at 150ms. It would likely be better at a lower value, like 50ms or lower, saving >100ms. We would need to solve 1 and 2 first though.

@romgrk
Copy link
Contributor

romgrk commented Aug 28, 2023

  1. Wdym by input lag? I didn't notice it.
  2. It's going to require more work on the filtering, we need to decouple things a bit before we can get there.
  3. We set it at 150ms because of 2, until we solve that it's preferable to have it at a speed where it's less likely to trigger between typing 2 characters.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 28, 2023

  1. Wdym by input lag? I didn't notice it.

@romgrk Two demos: see how "q" takes a long time to be updated in the <input>

Screen.Recording.2023-08-29.at.01.14.58.mov

and see how select all + delete doesn't work:

Screen.Recording.2023-08-29.at.01.13.57.mov

@romgrk
Copy link
Contributor

romgrk commented Aug 28, 2023

Yes so it feels like all of those issues are caused by the filter being triggered in-between the keypresses. The immediate solution to that would be to... increase the filter debounce again 😅

With the 150ms delay, the UX for grids with small datasets is better, but for large datasets it's worst. I have mentioned somewhere else that an adaptive debouce delay could be interesting (small dataset = small delay, large dataset = large delay).

And of course the ideal solution would be interruptable filtering, but that requires quite a bit of work and it's not on the roadmap at the moment afaik. We may want to discuss how to prioritize that.

@cherniavskii
Copy link
Member

Yes so it feels like all of those issues are caused by the filter being triggered in-between the keypresses. The immediate solution to that would be to... increase the filter debounce again 😅

Is it the same issue we saw earlier in #9712 (comment)?
If so, we might be able to fix it without changing the debounce time, at least we managed to do this for quick filtering in #9630

@romgrk
Copy link
Contributor

romgrk commented Sep 12, 2023

It doesn't feel like the same issue, it's not a logical bug, it's caused by the CPU doing filtering while the user is typing.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Mar 12, 2024

I'm coming here from https://twitter.com/addyosmani/status/1767540608779083972, INP is now part of SEO ranking.

Testing https://next.mui.com/x/react-data-grid/filtering/quick-filter/ using https://web.dev/articles/debug-cwvs-with-web-vitals-extension (Chrome extension) illustrates the problems well:

Screen.Recording.2024-03-13.at.00.06.20.mov

This video is perfect https://youtu.be/KZ1kxzsJZ5g?si=DmhtZ8DZ0rbxs35-&t=223 and bring back the solution raised before. We need to make two changes/two PRs:

  • 1. use useTransition() to be responsive to user inputs
  • 2. filter the rows in chunks to not block the main thread

@oliviertassinari oliviertassinari changed the title [data grid] Reduce filter debounce from 500ms to 50ms [data grid] Improve quick filter performance (INP) Mar 12, 2024
@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid Jul 3, 2024
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Nov 9, 2024

One day, we could use scheduler.yield() https://web.dev/articles/top-cwv#in-long-tasks instead of:

import { getEmployeeColumns } from '../columns/employees.columns';

that has a "background" task priority inherited from requestIdleCallback() but we want "user-visible", no? https://developer.mozilla.org/en-US/docs/Web/API/Prioritized_Task_Scheduling_API#task_priorities

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer enhancement This is not a bug, nor a new feature feature: Filtering Related to the data grid Filtering feature performance
Projects
Status: 🆕 Needs refinement
Development

Successfully merging a pull request may close this issue.

4 participants