[icons][docs] Index search synchronously #44001
Conversation
Netlify deploy previewhttps://deploy-preview-44001--material-ui.netlify.app/ Bundle size report |
oliviertassinari
force-pushed
the
search-speed
branch
from
640fff2 to
4123bbd
Compare
oliviertassinari
changed the title
oliviertassinari
force-pushed
the
search-speed
branch
from
4123bbd to
5637c55
Compare
| searchIndex.addAsync(importName, searchable); | ||
| searchIndex.add(importName, searchable); |
There was a problem hiding this comment.
The change that yields this outcome. Raised in nextapps-de/flexsearch#451.
Now, this is strange, what the async call seems to do is only https://github.com/nextapps-de/flexsearch/blob/961c3ae84a87fb3af2a52047fd7c8adc8949b86d/src/async.js#L32. So, I don't really see why 6,000 calls to setTimeout is slower. cc @romgrk if you ever seen stuff like this in the past.
There was a problem hiding this comment.
That whole initialization block becomes about 3 times slower for me now. It's indexing everything synchronously during module evaluation. Perhaps we can defer it as a whole? Trying a few things in #44025.
There was a problem hiding this comment.
That whole initialization block becomes about 3 times slower for me now. It's indexing everything synchronously during module evaluation.
@Janpot Ah, ok, this PR description wasn't super clear, I have updated it with what I was measuring. My goal was to show how time to interactive went from 6.2s to 1.9s.
It's indexing everything synchronously during module evaluation. Perhaps we can defer it as a whole? Trying a few things in #44025.
Yeah, this could be even better 👍. Overall, I find it strange that we need 700ms to index 6,000 records. This feels broken. Maybe we need a different search engine too.
Now, it's unclear to me if we need to go as far. I would be curious to merge this, wait for 30 days to see how the field vitals are and see if we need to allocate more time on this problem.
There was a problem hiding this comment.
Ah, ok, this PR description wasn't super clear, I have updated it with what I was measuring. My goal was to show how time to interactive went from 6.2s to 1.9s.
It was clear, I assume it's caused by the event loop being saturated. I just meant that the module evaluation was blocked for longer. It impacts hydration but the page should be visible because SSR.
Overall, I find it strange that we need 700ms to index 6,000 records. This feels broken. Maybe we need a different search engine too.
👍
oliviertassinari
changed the title
I initially worked on this because it seemed to be a quick win. I couldn't believe the need in pagespeed.web.dev for the 11s of blocking time. Eleven seconds, it sounded crazy. In the timeline, there was a strange and long segment of setTimeout.
See the time it takes to see the input updated with the search term "a". I have used this as a way to measure Time To Interactive.
Before: 6.2s https://deploy-preview-44000--material-ui.netlify.app/material-ui/material-icons/?query=a
After: 1.9s https://deploy-preview-44001--material-ui.netlify.app/material-ui/material-icons/?query=a
Help with #41126
With all the changes that we did, we are closer to the kind of performance that we had in https://v4.mui.com/components/material-icons/. We are not at Material UI v4 level yet (1.4s), but at least, with the changes that we did lately, it's not mediocre UX for developers like it was before (I'm not sure where these regressions on the icons page came from in the last 3 years, it would be interesting to debug this, we want changes to help move forward).