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

[charts] Slow Sparkline on /material-ui/getting-started/templates/dashboard/ #16018

Open
oliviertassinari opened this issue Dec 28, 2024 · 3 comments
Labels
bug 🐛 Something doesn't work component: charts This is the name of the generic UI component, not the React module! performance

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 28, 2024

Steps to reproduce

Steps:

  1. Open https://mui.com/material-ui/getting-started/templates/dashboard/
  2. Use the sorting feature, or pagination feature
  3. See how the INP is "poor":
SCR-20241228-plcd
  1. See how I'm not the only one: https://pagespeed.web.dev/analysis/https-mui-com-material-ui-getting-started-templates-dashboard/56m6qhi0mo?form_factor=desktop
SCR-20241228-plhy

Context

In my view, this is super important, a demo that doesn't pass webvitals is a demo that doesn't exist. I mean, it's not usable in production.

At 100ms, this starts to be great, e.g. https://dashboard.tremor.so/details

SCR-20241228-pmhh

Root cause

The root of the issue seems to be with this column:

SCR-20241228-porc

I can measure that 90% of the time is spent rendering this.

The root cause might be #9799. Now, it's really weird, I can reproduce this https://stackoverflow.com/questions/52991688/significantly-higher-performance-in-react-whilst-profiling-with-chrome

When I enable profiling mode, it's so much faster 😆, so no longer a true issue.

So maybe the INP reported by https://pagespeed.web.dev/ is actually the theme toggle:

SCR-20241228-slty
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work priority: important This change can make a difference performance component: data grid This is the name of the generic UI component, not the React module! status: waiting for maintainer These issues haven't been looked at yet by a maintainer component: charts This is the name of the generic UI component, not the React module! labels Dec 28, 2024
@oliviertassinari oliviertassinari changed the title [data grid] Demo in https://mui.com/material-ui/getting-started/templates/dashboard/ is not usable [data grid][charts] Demo in /material-ui/getting-started/templates/dashboard/ is not usable Dec 28, 2024
@oliviertassinari oliviertassinari changed the title [data grid][charts] Demo in /material-ui/getting-started/templates/dashboard/ is not usable [charts] Demo in /material-ui/getting-started/templates/dashboard/ is not usable Dec 28, 2024
@oliviertassinari oliviertassinari changed the title [charts] Demo in /material-ui/getting-started/templates/dashboard/ is not usable [charts] Show Sparkline on /material-ui/getting-started/templates/dashboard/ Dec 28, 2024
@oliviertassinari oliviertassinari removed priority: important This change can make a difference component: data grid This is the name of the generic UI component, not the React module! labels Dec 28, 2024
@oliviertassinari oliviertassinari changed the title [charts] Show Sparkline on /material-ui/getting-started/templates/dashboard/ [charts] Slow Sparkline on /material-ui/getting-started/templates/dashboard/ Dec 28, 2024
@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 2, 2025
@romgrk
Copy link
Contributor

romgrk commented Jan 3, 2025

The root of the issue seems to be with this column:

I had similar results when I benchmarked the performance of that template a few months ago. By personal experience, I don't think we'll get that column to an acceptable performance by tweaking charts code. React+svg+charts is a bad combo for performance, we need to replace at least one of those aspects to offer good performance.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jan 3, 2025

@zanivan @alexfauquette what if we were to render something else on that data grid? I'm afraid it's too slow, discrediting the whole page, when it might be a Sparkline only issue.

@romgrk I dove a bit deeper to understand why the Spackline is slow with #9799 (comment). It felt like the current bottleneck is emotion. Responsible for a +50% degradation of performance.

I can imagine a canvas would be +90% faster, thought, it's not clear if most developers would want us to make this tradeoff.

@JCQuintas
Copy link
Member

Right now the Sparkline shares a lot of logic with the full bar/line charts. So there is definitely parts we can chunk off, like the animation "transition" you found, even though animation is disabled in sparkline, hence #9799

How high of a priority do you think this should be in broad terms?

I ask because right now we see it as an improvement, but not high on the list due to the amount of necessary work vs perceived return compared to the other topics in our list. Eg: V8 Breaking changes and new pro features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: charts This is the name of the generic UI component, not the React module! performance
Projects
None yet
Development

No branches or pull requests

4 participants