Ahmad-Hamwi / lazy-pagination-compose

An intuitive and customizable Kotlin/Compose Multiplatform pagination composables that are built on top of lazy scrollables. Available on Android, iOS, MacOS, Linux, and Web.
MIT License
95 stars 6 forks source link

The list grows infinitely until the app crashes with OOM #4

Open Nek-12 opened 3 months ago

Nek-12 commented 3 months ago

With current implementation, after scrolling through several thousand items, the app crashes with OOM because it fails to allocate a huge list of items. Before that, the app slows down to a halt due to GC pressure.

Ahmad-Hamwi commented 3 months ago

Thank you for your observation, may I ask you to test this on a normal list? Not a paginated one. I need first to know whether this is a compose problem or the library's problem.

Also let me know what platform you're on.

Nek-12 commented 3 months ago

The same list if not paginated will also crash the app. Android.

Ahmad-Hamwi commented 3 months ago

This means this isn't this library's bug, it is probably a misuse of the lazy list or an actual bug on compose's side.

Are you assigning keys to each item in the list? If not, do it and let me know if it helped. The key should be unique across all items.

Nek-12 commented 3 months ago

This is a library bug on this line https://github.com/Ahmad-Hamwi/lazy-pagination-compose/blob/104dbe2fd84312f87e85e8e3b5c18c845bb4aead/lazy-pagination-compose/src/commonMain/kotlin/io/github/ahmad_hamwi/compose/pagination/PaginationState.kt#L49

You grow the list infinitely and never let go of the references

Ahmad-Hamwi commented 3 months ago

True, otherwise your data will be lost, you won't be able to go back to your previously scrolled data, this isn't the behavior I expect when scrolling in any feed app for example :)

If you suggest any better way of holding the data without a loss, let me know.

Nek-12 commented 3 months ago

Jetpack paging evicts items from the cache on LRU basis and reloads them on scroll back.

Ahmad-Hamwi commented 3 months ago

You're talking about disk caching, now it makes sense, it is not supported.

I think supporting such optimization will take some time, especially on each platform, I may start with Android.

Will consider such change and I'm open to any contributions.

Nek-12 commented 3 months ago

No I'm not talking about disk caching, I meant an in-memory object pool based on distance to current index.

Ahmad-Hamwi commented 3 months ago

Can you provide me with resources on such in-memory object pool? A quick search showed me that it can be the same as an in-memory DB, but I'm not sure how it differs from using heap directly (current implemention) in terms of optimization.

Ahmad-Hamwi commented 3 months ago

I would also appreciate a minimal sample project that has produces the crash.

Nek-12 commented 3 months ago

Can you provide me with resources on such in-memory object pool? A quick search showed me that it can be the same as an in-memory DB, but I'm not sure how it differs from using heap directly (current implemention) in terms of optimization.

https://android.googlesource.com/platform/frameworks/support/+/androidx-paging-release/paging/paging-common/src/commonMain/kotlin/androidx/paging/ItemSnapshotList.kt

https://android.googlesource.com/platform/frameworks/support/+/androidx-paging-release/paging/paging-common/src/commonMain/kotlin/androidx/paging/PageStore.kt

terrakok commented 3 months ago

@Nek-12 instead of the list of objects you can keep paged list of IDs. And every item will load its data by its own. (and will free the memory when the view leave the composition)