mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[data grid] Poor mount performance on Safari

Open lauri865 opened this issue 9 months ago • 8 comments

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

lauri865 avatar Apr 25 '24 15:04 lauri865

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 avatar Apr 25 '24 16:04 romgrk

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.

romgrk avatar Apr 25 '24 17:04 romgrk

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.

lauri865 avatar Apr 25 '24 18:04 lauri865