mikepenz/FastAdapter

For discussion - Performance issues on selectExtension.deselect()

RobbWatershed opened this issue · 5 comments

RobbWatershed commented

About this issue

  • Briefly describe the issue

Performance of selectExtension.deselect() is atrocious : users with large collections of items report the app hanging during 20-30s during that sole step of deselecting data.

When calling selectExtension.deselect(selectExtension.getSelections()) instead, FastAdapter only processes actually selected items, and the freeze doesn't happen anymore.

=> Are you certain the use of fastAdapter.recursive when calling deselect() is the best way to go ?
Isn't there some kind of optimization that can be done there ?

  • How can the issue be reproduced / sample code

Details

  • Used library version : 5.4.1
  •  Used support library version : not sure what you mean
  •  Used gradle build tools version : 4.2.0
  •  Used tooling / Android Studio version : 4.2
  •  Other used libraries, potential conflicting libraries : Not that I know of
  • The list where it happens uses the paging extension and is fed by livedata

Checklist

mikepenz commented

@RobbWatershed thank you so much for the issue (and the PR) I hope to be able to come back to it soon. the next 2-3 weeks may be super busy though, so I am sorry in case it's taking me a bit longer to come back to these

RobbWatershed commented

No problem man, take your time, it's not blocking me in any way 😉

mikepenz commented

@RobbWatershed one of the major reasons for the way deselect() works is to really make sure nothing is selected anymore.
It does not make assumptions on implementation details (e.g. it will deselect all items) -> for example if sub items can't be selected.

if your usecase does not allow selections in subitems then the second approach would for sure much more fit the requirements.

in the past we had a list keeping track of selections, but ultimately this served to be problematic as it may get out of sync, and ultimately the adapter itself should not necessarily maintain this.

Not sure if this already answers your concern/question?

mikepenz commented

Also congrats to opening the 1000th ticket :D

  • 😄1
RobbWatershed commented

if your usecase does not allow selections in subitems then the second approach would for sure much more fit the requirements.

in the past we had a list keeping track of selections, but ultimately this served to be problematic as it may get out of sync, and ultimately the adapter itself should not necessarily maintain this.

That's a perfectly valid answer to me, thanks !

I didn't even think about subitems when considering creating that issue, tbh, as my app doesn't use them.

I'll keep using selectExtension.deselect(selectExtension.getSelections()) for my own needs