razeware / emitron-Android

Android version of emitron
Apache License 2.0
55 stars 30 forks source link

Ema-289 library screen performance - DO NOT MERGE #292

Closed filbabic closed 3 years ago

filbabic commented 3 years ago

This is a placeholder PR, to show some progress on the library screen performance issue.

@orionthewake I've done some research and some progress on the #289.

The current data flow implementation is extremely complicated and a bit too tightly coupled. Instead of having reusable and clean way to differentiate different types of data fetching, the paging library is just deeply rooted at the data fetching, meaning it's not just easily refactored and changed to be database-first/offline-first.

I think, if we want to support full-on offline mode, we'd have to refactor a good amount of the application and the data flow, to somehow integrate database data instead of using the remote API as its primary source of information.

Also, all the adapters are using the same ContentAdapter, meaning that we could technically use this heavy coupling & system of data loading, and somehow try to switch from the remote source to a local source.

That being said, I think a good estimate on this could be to spend a few weeks, instead of a week, to fully refactor all parts of the app to try to cache data locally before showing it.

I feel like I'd also like to refactor the UI parts and how the uiState handling is done, as that part seems also very convoluted and hard to work around/with.

I've attached a few screenshots of the behavior on this branch. The process is the following:

The issues with current data loading is the following:

The problem here is that we store all the content in the same way. Well it's all content, and all within the Contents table. So things like bookmarks, progressions, collections, etc. are all in the same table.

That being said, in case we want to switch to an offline-first model, we'd have to decide how this is going to behave. How many items will we store, what happens when the user connects to the app - do we clear the DB and start loading and storing new items etc.

There are many use cases we'd have to think about and develop/test because small things can mess up the paging setup and the DB contents.

In an ideal (and much simplified use case), we'd just connect our data to a LiveData from the DB and then as we scroll we'd load more and more items from the network. That way the data would just update automatically.

But as it sits, the data comes from a paging source and we'd have to refactor a good amount of code that's used all over the place.

We can discuss this more in our meeting but I feel like we'd have to do a bigger overhaul of the app soon to make things easier in the future!

Screenshot_20201019-122039 Screenshot_20201019-121920 Screenshot_20201019-122048 Screenshot_20201019-122000 Screenshot_20201019-122008

These images show how the offline first behavior currently works. There is some data cached, but not all of it. I'd love to have this one of the bigger features we build in the next few months or something!