mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.82k stars 1.14k forks source link

[Datagrid] Poor mount performance on Safari #12908

Open lauri865 opened 3 weeks ago

lauri865 commented 3 weeks ago

Steps to reproduce

There's been amazing performance improvements in rendering the V7 of Datagrid, huge kudos to @romgrk et al. One area that could still be optimised is initial rendering/mounting performance, which seems especially poor in Safari (unfortunately we have to support that).

Link to live example: (required)

Steps:

  1. Open e.g. this page in Safari: https://mui.com/x/react-data-grid/editing/
  2. Open Devtools -> timeline
  3. Referesh the page

I'm seeing 85%+, sometimes 100%+ CPU usage on page load. As a benchmark, the demo site of AG-grid only peaks at around 30% for a short time. https://ag-grid.com/example/

Current behavior

CPU gets choked by (most likely) grid measurements.

Example output – not sure if it's the culprit or just a result of choked CPU, but seeing measureScrollbarSize called many times on mount:

Screenshot 2024-04-25 at 16 46 32 Screenshot 2024-04-25 at 16 56 34

Gets especially bad when a page has multiple datagrids. We have a page with 5 grids, and while in Chrome the performance is completely acceptable, in Safari, there's a visual delay between page navigations. Removing the grids makes everything super snappy.

Expected behavior

CPU not getting choked on mounting.

Context

No response

Your environment

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: safari, performance,slow

romgrk commented 3 weeks ago

That function is only called at a single site, seems like it could be an excessive number of updateDimensions being triggered during mount.

I'd take it but I'm on linux, maybe @cherniavskii or @MBilalShafi?

romgrk commented 3 weeks ago

If you copy the measureScrollbarSize function and set the prop scrollbarSize it could maybe improve performance easily on your side:

// calculate once
const scrollbarSize = measureScrollbarSize(document.body, -1, undefined)

function Component() {
  return <DataGrid scrollbarSize={scrollbarSize} />
}

The reason why measureScrollbarSize is not cached is because we can't know if the user is setting their grid scrollbars to different sizes, but you should be able to make that assumption.

We're doing a DOM write+read in the measure function which is quite expensive, it would be nice to find a solution.

lauri865 commented 3 weeks ago

Amazing, thanks a lot @romgrk. Definitely visibly better.

There's still somewhat of a performance gap with Safari (vs. Chrome) though, but at least it doesn't look like our client-rendered pages are fetched from the server anymore ;). I think Chrome is better at optimizing layout measurements/trashing, so you can get away with less optimised code / calling offsetWidth, etc. more frequently without incurring any performance penalty, so maybe there's more room to optimise around updateDimensions.