Improved bookmarks screen performace.

[anikiki]

Task/Issue URL: https://app.asana.com/0/414730916066338/1202088634690441/f

Description

Changes:

1/ Updated to insert all bookmarks in one go and all favorites in one go. 2/ Removed the ListAdapter along with the DiffCallback from BookmarksAdapter / FavoritesAdapter. This was incredibly slow, taking more than 20 seconds to display a list, unlike a regular implementation of RecyclerView.Adapter. 3/ Removed the usage of findViewById() when updating the viewholder in both BookmarksAdapter and FavoritesAdapter. Updated the TwoLineListItem custom view to give access to the image. 4/ Updated the loadFavicon in both BookmarksAdapter and FavoritesAdapter to do all operations needed to find the favicon using the IO dispatcher. Updated the actual Glide call which loads the image to use the Main dispatcher and also forced cancelling image requests that are not needed (i.e the view is no longer on screen). 5/ Used StrictMode before and after the changes to prove the main thread is now free of unnecessary operations.

Steps to test this PR

To see the issue:

• fresh install from develop
• import a large number of bookmarks / favorites. Example files are available in the asana task.
• while importing try to scroll up / down
• notice the app crashes

To test the fix:

• fresh install from this branch
• import a large number of bookmarks / favorites. Example files are available in the asana task.
• while importing try to scroll up / down
• the app should not crash
• additional tests: do a search in bookmarks

NO UI changes

[AndroidPoet]

if I may I would like to suggest you please remove "GlobalScope.launch(Dispatchers. Main)" and replace it with lifecycle aware coroutine scope, calling GlobalScope inside the adapter will launch a top-level coroutine and they are unbound and keeps running until finished or canceled. it might create a memory leak. GlobalScope is ok if we use it for the single task but using an inside adapter where the user will scroll a list and glide will load images it might create memory leak.

Extesntion function

using above extesntion function inside suspand function

calling everything inside lifecycle aware coroutine

i guess we are creating a nesting of coroutines.

[AndroidPoet]

Hi @anikiki I have created issues for memory leak https://github.com/duckduckgo/Android/issues/1990 This is the report from "TabSwitcherActivity", I have attached a screenshot for your reference, only "Favicon" is retaining 1.5 Mb this might be the reason for the slow performance of the bookmarks screen. I hope this will help. 0

[anikiki]

Hi @anikiki I have created issues for memory leak #199 This is the report from "TabSwitcherActivity", I have attached a screenshot for your reference, only "Favicon" is retaining 1.5 Mb this might be the reason for the slow performance of the bookmarks screen. I hope this will help. 0

Hey @AndroidPoet! Thanks for the input on these changes. At the moment this PR is still in progress (converted to draft) as other things need to be addressed, including what you mentioned in your first comment. I'm not actively working on it at the moment, however I'll come back to it as soon as I can.

[AndroidPoet]

in my opinion there is no need to optimize this screen with coroutines, The main reason behind the slow performance is the Memory leak. I was testing the app the few main screens performed slowly due to deeply coupling of the objects. When those memory leaks will be fixed these screens will work perfectly. I am sharing these details so this might help in other areas where the actual reason might be related to a memory leak.