Skip to content
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

Improve load-time performance of widgets #6008

Open
Haroenv opened this issue Jan 17, 2024 · 3 comments
Open

Improve load-time performance of widgets #6008

Haroenv opened this issue Jan 17, 2024 · 3 comments

Comments

@Haroenv
Copy link
Contributor

Haroenv commented Jan 17, 2024

When mounting a lot of widgets, the time to mount can add up quite a bit. This issue highlights a couple ways to deal with it. These PRs are ordered in the ordering they should end up in master.

Times are for an extreme app which mounts hundreds of widgets using DynamicWidgets.

The applied changed in 🟢 are available in InstantSearch.js 4.64.1, React InstantSearch 7.5.3, Vue InstantSearch 4.13.5

swap clearRefinements for individual refinement clearing methods which are more efficient 🟢

Fix sort of facet values in requestBuilder 🟢

avoid a copy of parameters to set in setQueryParameters and new SearchParameters 🟢

Don't recompute search parameters for every widget 🔴

  • privateHelperSetState function needs to loop through every already added widget, and this is called once per addWidgets. In react there's one addWidgets for every widget, instead of being able to call addWidgets once for all widgets at once
  • impact: 120ms -> ~70ms(?) (only an estimate, this was completely removing the code completely)
  • relevant code:
    privateHelperSetState(helper!, {
    state: getLocalWidgetsSearchParameters(localWidgets, {
    uiState: localUiState,
    initialSearchParameters: helper!.state,
    }),
    _uiState: localUiState,
    });

Don't clean up "unused" UiState and parameters 🔴

  • For the moment this is too much code to change to be able to measure, but we have a decent amount of impact cleaning up the impact of a widget no longer being mounted in getWidgetSearchParameters and getWidgetUiState. This would be more efficient if we left the empty values in place instead of looping and checking if every value is empty.
  • This isn't straightforward to change, as UiState is passed to routing for example where we need to be sure empty values are removed (they aren't today), as well as for inheriting parameters with multi-index, where an empty values implies a child index takes preference over a parent index, even if it has no refinements.
@redox
Copy link
Contributor

redox commented Jan 17, 2024

Exciting @Haroenv - I think https://sorare.com/football/market/new-signings could be a great production use-case to confirm those numbers ; I can give this a try next week

@redox
Copy link
Contributor

redox commented Jan 24, 2024

@Haroenv I'm a bit confused, is instantsearch.js 4.64.1 including those?

@dhayab
Copy link
Member

dhayab commented Jan 24, 2024

Hi @redox, Haroen is off so I'll answer in his place. Today's release of instantsearch.js@4.64.1, react-instantsearch@7.5.3 and vue-instantsearch@4.13.5 include the PRs mentioned in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants