bumptech / glide

An image loading and caching library for Android focused on smooth scrolling
https://bumptech.github.io/glide/
Other
34.67k stars 6.12k forks source link

Potential memory leak with Fragment RecyclerView and Glide #3075

Closed mazzouzi closed 6 years ago

mazzouzi commented 6 years ago

Glide Version: 4.6.1 Integration libraries: none Device/Android Version: All devices

Issue details / Repro steps / Use case background: The scenario is as follows: when I open my app, I'm showing Fragment A in a bottomNavigationView. At this point the Java heap equals 10.2MB. When I select Fragment B, I'm showing a list of videos with thumbnails loaded by Glide. In this fragment I scroll down to paginate and display all videos -> Java heap equals 20.9MB. Then I press Back so Fragment B is popped off backstack and Fragment A is displayed. I'm monitoring everything with Android Profiler so I can force a garbage collection.

Expected behavior: Java heap memory should go back to 10.2MB or something close. Actual behavior: After pressing back and forcing a garbage collection java heap memory goes down to 18.3 MB only. I'm leaking 8 MB on this simple scenario.

backkeyonfragmentbtofragmentascenario

A few points: 1) Fragment B and corresponding adapter are coded in Kotlin. Viewholder in Java. 2) In Android Profiler I can see that Fragment B, Adapter and Viewholder have been garbage collected successfully. The only remaining objects are Fragment and Adapter's companion objects which is normal. 3) In Android Profiler, in Bitmap Object, I can see that almost all thumbnails loaded in Fragment B are still there and have not been freed (7.6 MB). This is the issue to me. 4) I'm using Glide this way in my Viewholder: Glide.with(itemView.getContext()) .load(currentFavoriteVideo.getThumbnail()) .transition(withCrossFade()) .into(thumbnailImageView); 5) Fragment B's layout is the following :

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:gravity="center"
    android:orientation="vertical">

    <ViewSwitcher
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        android:id="@+id/viewSwitcher">

        <include layout="@layout/loader"/>

        <android.support.v7.widget.RecyclerView
            android:id="@+id/favorite_recycler_view"
            android:layout_width="match_parent"
            android:layout_height="match_parent"
            tools:listitem="@layout/fragment_allplaylists_viewholder" />
    </ViewSwitcher>
</LinearLayout>

What I do not understand is that if I put the ViewSwitcher inside a NestedScrollView then IT WORKS FINE and there is no more memory leak :

<android.support.v4.widget.NestedScrollView
        android:layout_width="match_parent"
        android:layout_height="match_parent">

        <ViewSwitcher
            android:layout_width="match_parent"
            android:layout_height="match_parent"
            android:id="@+id/viewSwitcher">

            <include layout="@layout/loader"/>

            <android.support.v7.widget.RecyclerView
                android:id="@+id/favorite_recycler_view"
                android:layout_width="match_parent"
                android:layout_height="match_parent"
                tools:listitem="@layout/fragment_allplaylists_viewholder" />
        </ViewSwitcher>
    </android.support.v4.widget.NestedScrollView>
</LinearLayout>

No more memory leak issue with above layout... Any idea ? Fragment/Adapter/Viewholder have been garbage collected successfully so why Glide is keeping those thumbnails in memory ?

Thanks in advance for you help.

TJBlack31 commented 6 years ago

@mazzouzi have you tried putting the ViewSwitcher in a FrameLayout instead of directly in a LinearLayout?

mazzouzi commented 6 years ago

Hi @TJBlack31 thanks for your help. I just tried following layouts but still facing same issue :

<LinearLayout 
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:gravity="center"
    android:orientation="vertical">

    <FrameLayout
        android:layout_width="match_parent"
        android:layout_height="match_parent">

        <ViewSwitcher
            android:layout_width="match_parent"
            android:layout_height="match_parent"
            android:id="@+id/viewSwitcher">

            <include layout="@layout/loader"/>

            <android.support.v7.widget.RecyclerView
                android:id="@+id/favorite_recycler_view"
                android:layout_width="match_parent"
                android:layout_height="match_parent"/>
        </ViewSwitcher>
    </FrameLayout>
</LinearLayout>

<FrameLayout
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:orientation="vertical">

    <ViewSwitcher
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        android:id="@+id/viewSwitcher">

        <include layout="@layout/loader"/>

        <android.support.v7.widget.RecyclerView
            android:id="@+id/favorite_recycler_view"
            android:layout_width="match_parent"
            android:layout_height="match_parent"/>
    </ViewSwitcher>
</FrameLayout>

<FrameLayout 
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:orientation="vertical">

    <android.support.v7.widget.RecyclerView
        android:id="@+id/favorite_recycler_view"
        android:layout_width="match_parent"
        android:layout_height="match_parent"/>
</FrameLayout>
TJBlack31 commented 6 years ago

@mazzouzi

hmm. It might help to manually clear the media using Glide.clear(View)

according to this, it's not recommended usually, but in your case it might be worth a shot.

You basically override onViewRecycled from the RecyclerView.Adapter class and call Glide.clear(View). It could free up some memory in your case.

Please let us know what you find!

-TJB

mazzouzi commented 6 years ago

I'm running Glide V4 which does not have clear() method. Is there something similar in V4 ?

TJBlack31 commented 6 years ago

@mazzouzi

I found a clear() in the RequestManager class.

If that doesn't work, maybe try something like this:

Glide.with(DemoActivity.this)
.load(Uri.parse("file://" + imagePath))
.diskCacheStrategy(DiskCacheStrategy.NONE)
.skipMemoryCache(true)
.into(mImage);

from here

TJBlack31 commented 6 years ago

@mazzouzi

Also, you can refer to this and experiment with Glide.get(context).clearDiskCache() and clearMemory() methods.

mazzouzi commented 6 years ago

I tried Glide.with(this).clear(itemview) in my adapter but it has no effect unfortunately.

If I use skipMemoryCache(true) then no more memory leak so it confirms that it's all related to those bitmaps cached in memory but not released when fragment is destroyed. I also tried to code my viewholder/adapter in a couple of different ways to see if it changes something but no. I even took "list" template in Android Studio to do some test but same result, after back key press some of the bitmaps from Fragment B are not released...

Any other idea ? Could someone investigate or at least point me to a project where no memory leaks occur after second fragment is destroyed ?

Thanks

sjudd commented 6 years ago

Glide uses a size limited LRU memory cache. Whenever an image is cleared (either explicitly or because the context is destroyed), the image gets moved into the LRU cache. When the LRU cache is full, it evicts the least recently used items.

If you want to avoid putting items into the memory cache, you can use skipMemoryCache as you've noticed. Otherwise this sounds like it's expected and not a memory leak.

mazzouzi commented 6 years ago

Thanks @sjudd. Just to confirm, are you saying that it's totally expected at this point that the thumbnails loaded in Fragment B are not cleared when fragment B gets destroyed ? The LRU cache should not clear them automatically ? I had in mind the fact that Glide was capable of following fragment's lifecycle...

Any idea about NestedScrollView mentioned above which seem to make the memory cache release thumbnails automatically ?

Thanks a lot

sjudd commented 6 years ago

It's expected that those items go into the LRU cache, yes. Glide does follow the fragments lifecycle to cancel in progress requests and return Bitmaps to the cache and then the Bitmap pool where they can be re-used.

In general it's common for people to switch back and forth between screens. Removing all Bitmaps and not caching them each time the current fragment changes would lead to janky experiences where users go back and forth between the same two screens.

There's no difference with a nested scroll view with regards to the caching behavior, at least not in Glide's logic.

mazzouzi commented 6 years ago

Thanks for your clarification !

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.