-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[DataGrid] Avoid allocations in hydrateRowsMeta
#9121
Conversation
Netlify deploy previewNetlify deploy preview: https://deploy-preview-9121--material-ui-x.netlify.app/ Updated pagesNo updates. These are the results for the performance tests:
|
const existingBaseSizes = Object.entries(sizes).reduce<Record<string, number>>( | ||
(acc, [key, size]) => { | ||
if (/^base[A-Z]/.test(key)) { | ||
acc[key] = size; | ||
} | ||
return acc; | ||
}, | ||
{}, | ||
); | ||
|
||
// We use an object to make simple to check if a height is already added or not | ||
const initialHeights: Record<string, number> = { | ||
...existingBaseSizes, | ||
baseCenter: baseRowHeight, | ||
}; | ||
const initialHeights = {} as Record<string, number>; | ||
/* eslint-disable-next-line no-restricted-syntax */ | ||
for (const key in sizes) { | ||
if (/^base[A-Z]/.test(key)) { | ||
initialHeights[key] = sizes[key]; | ||
} | ||
} | ||
initialHeights.baseCenter = baseRowHeight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love functionnal programming, but FP is always slower in JS :|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement, it surely saves some ticks 👍
I agree mutation could be really useful for performance bumps.
This PR avoids object allocations in
hydrateRowsMeta
. For large datasets, the costs are quite high. This is compounded by the fact that we create our objects with{ ...spread }
, and_objectSpread2
is very expensive.The results below are obtained by clearing the filters in the same example as in #9120