Performance regression from dom to hooks - hits hooks and components have costly `escapeHits`

[joepio]

🐛 Bug description

I noticed that performance of updating the search query became quite a bit worse when I changed from -dom to -web-hooks.

Some functions that seem responsible for pretty much all of the wait time:

• escapeHits
• recursiveEscape
• getWidgetRenderState

🔍 Bug reproduction

Steps to reproduce the behavior:

1. Set up a basic React-Instantsearch with Hits and SearchBox with react-instantsearch-dom
2. Start typing, note the quick rendering
3. Switch to react-instantsearch-dom
4. Start typing, note the slower rendering

Sorry for the lack of demo repo, my project is currently closed source. I'll try to dive in deeper, I'll update this issue

Some findings / thoughts:

• The issue scales with the amount of HitsPerPage
• Even if my custom Hit component returns null, the issue is bad, so it's not really the rendering part, I think.
• It may have something to do with my relatively large documents, which contains many keys (about 100 KV combos)

Live reproduction:

https://codesandbox.io/s/github/algolia/react-instantsearch/tree/master/examples/hooks

Environment

• React InstantSearch Hooks version: 6.38.0 (also tried 6.40.0)
• React version: 18.2.0
• Browser: Firefox 108
• OS: MacOS

POSSIBLE FIX:

• Set escapeHTML to false in all InfiniteHits / Hits components and useHits , useInfiniteHits hooks.
• Use production build

[Haroenv]

Can you reproduce this as well in production mode? We notice the difference between different implementations is almost not visible when not in dev mode.

For individual improvements, you can put escapeHTML to false and pass the highlightPre/PostTag manually avoiding the escape

[joepio]

Thanks for the fast reply! I haven't yet tried production mode.

[vdumitraskovic]

@joepio Can you please check if you have the same issue with version 6.36? I'm investigating a performance issue in the private repo and found a problem in newer lib versions.

[joepio]

Hi @vdumitraskovic!

I've tried switching between 6.36.0 and 6.38.1, but there is not discernable difference here. The escapeHTML calls for both take up about 25% of time per query.

For me the real difference was between hooks and DOM

[Haroenv]

escapeHTML can be avoided in React, because you don't render dangerouslySetInnerHTML, therefore it can be faster if you do this:

https://codesandbox.io/s/silly-fermi-qj4dbn?file=/App.tsx:4270-4410

I'll think about how this can be improved and leads to less wasted cycles by default too

[joepio]

Hi @Haroenv, thanks for the help.

I setescapeHTML to false, but if I look at the profiler, I can still see that escapeHits is taking up a big chunk of the CPU cycles in 6.38.0. So it's still running with escapeHTML set to off, not sure what it's doing.

Note that this is in dev mode:

[Haroenv]

Do you possibly have multiple hits/infiniteHits/autocomplete widgets that could also be setting escapeHTML? I could see a difference profiling the example I posted

[joepio]

@Haroenv

No multiple components, but I do have a useHits somewhere else to render the count.

Let me try this:

  let {
    results: { nbHits },
    hits,
  } = useHits({
    escapeHTML: false,
  });

EDIT: it works! Updated post

[AntoineDuComptoirDesPharmacies]

@joepio @Haroenv

We clearly see a big performance dump when repeating the useHits hook a large amount of time. In our use case, we were putting useHits in every Hit sub-component so it have easy access to the sendEvent method. However, having like 50 or 100 useHits make the page terribly slow.

Easy workaround is to pass the sendEvent method from parent to children but as our JSX tree is very deep, it create a lot of "useless" boilerplate.

[Haroenv]

Seems like we could fix your issue specifically by passing sendEvent to hitComponent on the Hits widget, right @AntoineDuComptoirDesPharmacies?

[AntoineDuComptoirDesPharmacies]

@Haroenv that would help people using the Hits UI component, for sure !

On our side we have our own UI component but we could do the same as you propose, pass the sendEvent to children components. The only concern is that we have a deep JSX tree before reaching the button that should trigger a Conversion event. Maybe a good workaround in this case is to put this sendEvent method in a homemade context.

I am wondering if it could be interesting to provide this "Insights context" directly from react-instantsearch-hook-web package ?

[Haroenv]

I'd say it's probably best if you keep track of this in your own context, as for our use cases that wouldn't be needed, the component rendering has the same access as the place sendEvent is accessible from.

If you have some kind of indicator of scale and what makes the tree so complex that you can't use Hits, that would be interesting, as we'd like to encourage people to use the default components in most cases.

[Haroenv]

@AntoineDuComptoirDesPharmacies, Hits already exposes sendEvent to hitComponent 👍

[AntoineDuComptoirDesPharmacies]

In fact we do not use Hits because we have a custom UI for the listing of Hits, we do not want to use ol / li DOM nodes but instead we can use Table or Grid, depending the context.

As for passing the sendEvent across the tree, here is a shortlist of our tree component layers :

• Grid component
• Card
• Modal
• AddToCart
• Button with onClick => sendEvent

[joepio]

@Haroenv: I've set escapeHTML to false in the useInfiniteHits hook (only one instance) and in the InfiniteHits component, but I can still see escapeHits being called (and causing about 80% of all cycles in the JS), also in the production bundle. I think the escapeHits=false options are not working properly in either the useInfiniteHits hook or in the InfiniteHits components.

EDIT: Whoops... I think I missed my useHits hook. Everything seems fine now!