[icons] Improve icon search performance

[oliviertassinari]

Problem

This is a continuation of #44001. I'm not sure that the change had any positive impact. I suspect that that PR only surfaced a Chrome DevTools bug. I'm saying this because there is what we see now: https://pagespeed.web.dev/analysis/https-mui-com-material-ui-material-icons/cvfn967l26?form_factor=desktop. That INP is "poor":

It's a problem because this page is one of the pages that has the most traffic to our docs. We have to have a nice UX.

Looking at the page load timeline, I see we spend +500ms creating the search index, the page is hydrated after 1.40s:

Solution

1. Index async, moving [docs] Try out further icons optimization (ignore) #44025 to the finish line.
2. Use a faster search enginee. I was curious to check how other icon libraries handle this:

• https://react-icons.github.io/react-icons/: INP: 128ms, https://github.com/react-icons/react-icons/blob/master/packages/preview-astro/src/components/search/search-iconset.tsx#L38. No synonyms, no sorting, no fuzzing. Pretty raw.
• https://www.radix-ui.com/icons. INP: 88ms, https://github.com/radix-ui/icons/blob/bde33b13aa5848555f5512ac12155930fb4beb7d/packages/website/components/SearchResults.tsx#L57 similar to react icons.
• https://ionic.io/ionicons, INP 115ms.
• https://lucide.dev/icons/, INP 20ms, using https://www.fusejs.io/ under the hood.

In our case, minisearch takes about 250ms to index, flexsearch takes about 500ms, and fuse.js 50ms. The problem is that fuse trades too much indexation speed for search slowness, so minisearch seems best.

3. Avoid rerendering of the <Icons> component by using useEventCallback. Actually, many more parts of the docs are impacted by this problem, I have left a comment in the source for the root solution useRouterQueryParam() instead of useRouter() for performance vercel/next.js#45969. We could already start to fix this performance issue by having wrappers, matching the new App Router API. The issue can be observed using https://github.com/aidenybai/react-scan/blob/main/CHROME_EXTENSION_GUIDE.md.
4. I used the opportunity to migrate Grid -> Grid2. Fix some design issues. Remove DOM nodes where possible.
5. During the indexation, we can avoid the creation of a regex 10,000 times.
6. I have memo the filter component as well, as it seems to be wasteful.
7. Use https://mui.com/material-ui/react-progress/#high-cpu-load to reduce CPU load when showing the progress, also make it more likely to be seen.

Preview: https://deploy-preview-44450--material-ui.netlify.app/material-ui/material-icons/

Our search is so much faster than on https://fontawesome.com/search 🥹. So, will this solve the 533ms INP field data? I hope so. Two hypotheses as to why we have this data now:

1. It's a regression from [icons][docs] Index search synchronously #44001. I shouldn't have moved to as sync indexation of the records.
2. It's about a bug with devtools + dev extensions. Right now, in Chrome v131, anytime I have the dev tools open and one of those extensions enabled (Grammarly, Bitwarden, uBlock Origin), the search becomes so much slower. It doesn't happen n Chrome Canary v133. So I hope it's only temporary

[mui-bot]

Netlify deploy preview

https://deploy-preview-44450--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against d92816d

[Janpot]

I'd also be interested in whether creating and serializing the index on the server and just deserializing it with loadJson on the client would be faster.

[oliviertassinari]

@Janpot Oh, I didn't think about this, smart. I guess the index could be created with a macro, during the bundle phase, like we do for parsing the markdown.

Maybe https://github.com/devongovett/unplugin-parcel-macros would do it. For example, I have seen @PookMook use it in https://github.com/PookMook/boomer-css/blob/fc051df3a67268b685e20765c3eb7a6a7c01686b/src/components/hero-section.tsx#L2. Edit: hum, looking at the issues with unplugin-parcel-macros, it might not fly. Maybe https://github.com/kentcdodds/babel-plugin-macros is still our best option 🤔.

[PookMook]

What are the issues that are concerning to you with parcel macros?
I now have a little bit more experience playing on and off with it for a few months maybe I can tell you if it's a no-go!

[oliviertassinari]

@PookMook Two concerns: 1. Turbopack support (thought to be fair, Turbopack starts a bit to feel like a failed project, compared to Rspack or Rolldown, but as long as we are within the Next.js ecosystem, it's important), 2. low npm download count, Devon is already spread thin with Lightingcss, Parcel, React Aria.

[PookMook]

In those two cases, very valid concerns.

1. I managed to get some macros working with turbopack by massaging the webpack plugin, but it's very unstable and never works when manipulating CSS (which is a no-go for me)
2. I consider the plugin as-is, because there's no indication that Devon is going to maintain the project at all. Best bet is either to contribute to unplugin/next to get it sorted, or full fork*.

Probably not acceptable state for a project like MUI indeed.

I'm exploring those two options in the very distant background as time is short recently and I personally have very little incentive to do so (we're using vite on our prod projects + bundle our design system on it's own and use it like CSS modules)

[Janpot]

I was more thinking about using getStaticProps for this. but my main worry would be that it would blow up the html again.

in general I'm not in favor of using a macro system as it tends to make code non standard, hard to reason about and locks you into a single compiler implementation. seems that parcel macros just execute at build time and don't alter JavaScript semantics? perhaps it could be acceptable? it would still blow up bundle size though. edit: 🤔 perhaps can be circumvented by loading it with a dynamic import?

a third option I'm pondering about is to pregenerate a static json file at build time which we statically host and fetch on page load. it would have the added benefit that json parsing happens on a background thread automatically. drawback is that we have to add some locking behavior to make sure it's loaded before we start filtering and have to solve SSR.

[oliviertassinari]

json parsing happens on a background thread automatically

@Janpot Unclear: https://news.ycombinator.com/item?id=21013215, https://www.reddit.com/r/javascript/comments/r8pwoz/comment/hn86ge6/.

a third option

Option 4 is to be lazy, do nothing 😄. I would be curious to see if this help, but I don't know if after this PR, it's still needed.

[Janpot]

Option 4 is to be lazy, do nothing 😄. I would be curious to see if this help, but I don't know if after this PR, it's still needed.

Did we ever make a version of this without a full text search engine? Intuitively I'd expect filtering a 6k list of relatively short strings to be not a super heavy operation. The best part is no part they say 🙂

[oliviertassinari]

@Janpot Oh I didn't know that https://v8.dev/blog/cost-of-javascript-2019#json. The file is pretty big https://github.com/mui/material-ui/blob/master/docs/data/material/components/material-icons/synonyms.js. Maybe it could save time.

make a version of this without a full text search engine

I can't remember, I might have added this in the very first version as I was using Font Awesome as reference. I struggle to envision the search to be great without synonymes.

Right now, the search takes a few ms and the indexation never blocks the main thread, maybe it's OK. I'm unclear on what's the main bottleneck now and either it's fast enough or not.

[Janpot]

Suggested change

	miniSearch.addAll([allIcons[tasks.current - 1]]);
	miniSearch.add(allIcons[tasks.current - 1]);

[Janpot]

It's kind of a weird method signature. Wouldn't it make more sense as

async function asyncWorker<T>(tasks: T[], doWork: (task: T) => void): Promise<void>

?

[Janpot]

Could this be simplified with scheduler.yield?

edit Interesting exploration on the topic

[Janpot]

I struggle to envision the search to be great without synonymes.

I don't think it would be that hard to make this filter with synonyms but we can try that out later. left a few suggestions, but nothing blocking

[Janpot]

@oliviertassinari Just did a quick test with the following snippet and it seems to consistently run in the 1ms range or lower on my machine

  const filteredIcons = React.useMemo(() => {
    console.time('filteredIcons');
    const result = allIcons.filter((icon) => {
      if (theme !== icon.theme) {
        return false;
      }

      if (query.length === 0) {
        return true;
      }

      // TODO: normalize strings
      if (icon.name.includes(query) || synonyms[icon.name]?.includes(query)) {
        return true;
      }

      return false;
    });
    console.timeEnd('filteredIcons');
    return result;
  }, [query, theme]);

  // The results slightly differ
  console.log(filteredIcons.length, icons.length);

I tried to time the indexed search and interestingly get results in the same ballpark for 1 or 2 character queries. For longer strings the indexed search is indeed an order of magnitude faster.

  const icons = React.useMemo(() => {
    console.time('indexedIcons');
    const keys = query === '' ? null : searchIndex.search(query, { limit: 3000 });
    const result = (
      keys === null ? allIcons : keys.map((key) => allIconsMap[key])
    ).filter((icon) => theme === icon.theme);
    console.timeEnd('indexedIcons');
    return result;
  }, [query, theme]);

I assume the index defaults to a simple filter for short queries, which would be what I'd expect with my limited knowledge of how these FT indices work 🙂.