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

[pickers] Improve open action performance #4602

Open
oliviertassinari opened this issue Apr 17, 2020 · 5 comments
Open

[pickers] Improve open action performance #4602

oliviertassinari opened this issue Apr 17, 2020 · 5 comments
Labels
component: DatePicker The React component. component: pickers 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 performance

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 17, 2020

Why it matters

Opening the date picker on desktop and mobile have a perceptible delay, it's even more noticeable with the range picker. As a UI library, we have a 100ms delay budget before the users start to notice.

0 to 100ms | Respond to user actions within this time window and users feel like the result is immediate. Any longer and the connection between action and reaction is broken.

0.1 second is about the limit for having the user feel that the system is reacting instantaneously, meaning that no special feedback is necessary except to display the result.

The problem

On this demo https://mui.com/x/react-date-pickers/date-picker/#basic-usage, the pickers take about 227ms to show with an x4 throttle (to simulate a regular Windows laptop). The lag can be felt.

Screenshot 2023-08-11 at 23 18 19

Compared with these, they two also have the advantage of triggering on mouse-down which saves ~80ms

Screenshot 2023-08-11 at 23 13 08 Screenshot 2023-08-11 at 23 19 33 Screenshot 2023-08-11 at 23 16 37 Screenshot 2023-08-11 at 23 14 43

I suspect that part of the root cause is that we use too many Material UI primitives (not fast enough), we could instead style from the theme directly.

2020 exploration on the problem

Regarding delivering on better runtime performance

  1. The first thing I would look at is the usage of the ButtonBase for the days, I suspect an easy win by replacing them with a simple <button>, it seems to be an obvious win, we don't need the ripple there.

For instance, in dev mode, we get from 400ms to 250ms with this diff:

diff --git a/lib/src/DateRangePicker/DateRangePickerDay.tsx b/lib/src/DateRangePicker/DateRangePickerDay.tsx
index 9f54cb31..05ea0182 100644
--- a/lib/src/DateRangePicker/DateRangePickerDay.tsx
+++ b/lib/src/DateRangePicker/DateRangePickerDay.tsx
@@ -135,13 +135,7 @@ export const PureDateRangeDay = ({
           [classes.rangeIntervalDayPreviewStart]: isStartOfPreviewing || isStartOfMonth,
         })}
       >
-        <Day
-          allowKeyboardControl={false}
-          disableMargin
-          {...other}
-          day={day}
-          selected={selected}
-          inCurrentMonth={inCurrentMonth}
+        <button
           data-mui-test="DateRangeDay"
           className={clsx(
             classes.day,
@@ -152,7 +146,9 @@ export const PureDateRangeDay = ({
             },
             className
           )}
-        />
+        >
+          1
+        </button>
       </div>
     </div>
   );
  1. The second thing I would look at is how "loaded" the DOM is. For instance, react-day-picker has a single DOM node per cell vs x for us. replacing div > div > button with button we reduce the runtime from 250ms to 220ms.

I believe we could do the same with the year picker. div > h6 seems wasteful (as well as breaking the accessibility rule of headers continuity. It should be div > div at the very least, and ideally only div.

  1. Removing the Grow animation gets us down from 220ms to 130ms, still in dev mode. Something seems wrong with the implementation, it double renders. In this condition, I think there we are better off without. We would need to investigate, on what's wrong. We might only need a memo on the component just below Grow. Hopefully, we will be able to make the other components benefit from it.
  2. I haven't noticed another opportunity but it's probably not necessary to look deeper. from 400ms to 130ms in dev mode, if we extrapolate to the 190ms baseline, it means 62ms in production. This is perfect.

Environment

v6.11.1

@oliviertassinari

This comment was marked as outdated.

@oliviertassinari oliviertassinari changed the title Improve runtime [DatePicker] Improve runtime Nov 15, 2020
@oliviertassinari oliviertassinari transferred this issue from mui/material-ui-pickers Nov 15, 2020
@eps1lon
Copy link
Member

eps1lon commented Jan 14, 2021

I haven't noticed another opportunity but it's probably not necessary to look deeper. from 400ms to 130ms in dev mode, if we extrapolate to the 190ms baseline, it means 62ms in production.

Just in case others are reading this: Do not extrapolate from dev to prod. And in general: Do not meassure in development.

On my high-end $3,000 MacBook,

These are not expensive because of their performance.

Current results for opening a picker by keypress:
Datepicker: 50ms
Timepicker: 30ms
DateRangePicker: 70ms

We can definitely improve the performance and there are some "easy" wins within reach. But let's be accurate in our measurements so that we can make lasting changes. Not measuring did lead to some unnecessary work.

@flaviendelangle flaviendelangle added the component: pickers This is the name of the generic UI component, not the React module! label Apr 21, 2022
@flaviendelangle flaviendelangle transferred this issue from mui/material-ui Apr 21, 2022
@flaviendelangle flaviendelangle changed the title [DatePicker] Improve runtime [DatePicker] Improve runtime performances Sep 15, 2022
@flaviendelangle
Copy link
Member

This issue is very old and did not get a lot of feedback.
I think we can start by doing a small performance analysis of the pickers in there current shape to see if we find bottlenecks.

@flaviendelangle flaviendelangle changed the title [DatePicker] Improve runtime performances [pickers] Improve runtime performances Sep 15, 2022
@LukasTy
Copy link
Member

LukasTy commented May 2, 2023

Need to refresh the current condition of pickers' performance before identifying if any work is still needed.

@oliviertassinari
Copy link
Member Author

I have refreshed the content, extracting what I had identified in #7440. Part of the frustration is around the animation, which is a bit slow. I touched a bit on it in #10006.

@oliviertassinari oliviertassinari changed the title [pickers] Improve runtime performances [pickers] Improve open action performance Aug 11, 2023
@oliviertassinari oliviertassinari added design This is about UI or UX design, please involve a designer and removed design: ux labels Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: DatePicker The React component. component: pickers 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 performance
Projects
None yet
Development

No branches or pull requests

4 participants