algolia / instantsearch-ios

⚡️ A library of widgets and helpers to build instant-search applications on iOS.
https://www.algolia.com/doc/guides/building-search-ui/what-is-instantsearch/ios/
Apache License 2.0
592 stars 55 forks source link

HitsPerPage is not working , It causes infinitive api calls #207

Closed vishalgspaceo closed 2 years ago

vishalgspaceo commented 2 years ago

Describe the bug 🐛 I downloaded https://github.com/algolia/instantsearch-ios-examples this repo. I am using algolia in my project and found lots of calls happens . and found out that Multi Index search has pagination issue . hitsPerPage is not working properly. I used flex to check network history for api calls. https://github.com/FLEXTool/FLEX.

To Reproduce 🔍 Steps to reproduce the behavior: I tried two cases here

 let indexModules: [MultiIndexHitsConnector.IndexModule] = [
      .init(indexName: MultiIndexDemoSection.movies.indexName, hitsInteractor: HitsInteractor<Movie>(infiniteScrolling: .on(withOffset: **40**), showItemsOnEmptyQuery: true)),
      .init(indexName: MultiIndexDemoSection.actors.indexName, hitsInteractor: HitsInteractor<Hit<Actor>>(infiniteScrolling: .on(withOffset: 10), showItemsOnEmptyQuery: true)),
    ]
 func setup() {
    **multiIndexHitsConnector.searcher.indexQueryStates[0].query.hitsPerPage = 10**
    multiIndexHitsConnector.searcher.search()
    addChild(hitsViewController)
    hitsViewController.didMove(toParent: self)
  }

Expected behavior 💭 Found that It reaches to infinite api calles , I checked in flex which has more than 12000+ api calls . If I scroll the collection view in demo InstantSearch -> Paging(Section) -> Paging Multiple Index. It would have crash too

Screenshots 🖥 Simulator Screen Shot - iPhone 12 Pro Max - 2021-10-27 at 16 54 56

https://drive.google.com/file/d/1eMKJ46lVlm4DisonZWp2gxMBS0ioOb1I/view?usp=sharing

Environment:

Additional context Let me know if you need more help here

VladislavFitz commented 2 years ago

Hi @vishalgspaceo, Thank you for reporting this issue. It is prioritized in our roadmap, I'll notify you when the patch is ready.

VladislavFitz commented 2 years ago

Hi @vishalgspaceo,

I carefully examined the issue reproduction steps you provided. Actually this behaviour caused by an intended memory optimization. To avoid running out of device memory during the aggressive hits scrolling, the pagination engine keeps in memory 3 pages of hits before the first visible result and 3 pages of hits after last visible result. The value 3 is a hardcoded empirically derived value.

Let's examine the cases you reported one by one. First of all, these issues are not related to multi-index search. You can reproduce them in the single index search experience as well by setting the same parameters values.

Given:

Search result screen presenting 3 result hits at once (the case of multi-index demo).

1. First case

Query.hitsPerPage = 1 means that 1 page of results is equivalent of 1 hit HitsInteractor(infiniteScrolling(.on(withOffset: 10))) means that the pagination engine might ensure the presence of 10 hits before the first visible hit and 10 hits after the latest visible hit and trigger the corresponding network request if needed. pageCleanUpOffset = 3 means that the pagination engine when the page with index n is loaded, removes the pages with indices less than n-3 and more than n+3.

2. Second case

Query.hitsPerPage = 10 HitsInteractor.infiniteScrolling(.on(withOffset: 40) pageCleanUpOffset = 3

Conclusion

Since from the InstantSearch side we cannot control or predict the number of visible hits due to different rows/cell sizes or even dynamic autolayout sizing, we decided to keep the pagination parameters configurable. The default configuration:

is reasonable for most mobile search experiences to ensure a decent performance. You can fine tune these parameters to ensure the best performance in your application. The fact that they are exposed doesn't mean that everything might work flawlessly with any set of values of these parameters. Briefly:

We expect that if a developer alters the default values of these parameters, he knows what he is doing.

The only problem I see here is the private and hardcoded pageCleanUpOffset parameter and the missing warning in the documentation to be careful with pagination parameters 🙂

If you have a concrete infinite scrolling/pagination issue with your implementation where such specific pagination parameters make sense, I would be glad to discuss it and to find an optimal solution.