From ae66840e327c2ea4ab1e15e84468be93e51bf696 Mon Sep 17 00:00:00 2001 From: Emmanuel Boudrant Date: Tue, 1 Dec 2020 16:13:56 -0800 Subject: [PATCH] proposal - instead of linking the view pool to activities, could we link to a lifecycle owner? The problem is with Hilt, there is a memory leak. The memory leak is because Epoxy link the view pool to the activity however, when a view get inflated in a Hilt fragment, the context is a wrapper and hold a reference on the fragment. The fragment leak. So why Epoxy link the view pool arbitrary to the `Activity`. Instead, could we use the view's context and find the closest `LifecycleOwner`. Use the `LifecycleOwner`'s lifecycle ti decide when to clear the pool. However this change won't be enough to fix the memory leak we have with Hilt. We will also need this change in Hilt : --- .../com/airbnb/epoxy/EpoxyRecyclerView.kt | 10 +++--- ...rPool.kt => LifecycleOwnerRecyclerPool.kt} | 36 ++++++++----------- 2 files changed, 19 insertions(+), 27 deletions(-) rename epoxy-adapter/src/main/java/com/airbnb/epoxy/{ActivityRecyclerPool.kt => LifecycleOwnerRecyclerPool.kt} (69%) diff --git a/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyRecyclerView.kt b/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyRecyclerView.kt index 74ec78d491..4d4741871d 100644 --- a/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyRecyclerView.kt +++ b/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyRecyclerView.kt @@ -248,7 +248,7 @@ open class EpoxyRecyclerView @JvmOverloads constructor( } setRecycledViewPool( - ACTIVITY_RECYCLER_POOL.getPool( + LIFECYCLE_OWNER_RECYCLER_POOL.getPool( context ) { createViewPool() }.viewPool ) @@ -627,7 +627,7 @@ open class EpoxyRecyclerView @JvmOverloads constructor( // Views in the pool hold context references which can keep the activity from being GC'd, // plus they can hold significant memory resources. We should clear it asap after the pool // is no longer needed - the main signal we use for this is that the activity is destroyed. - if (context.isActivityDestroyed()) { + if (context.isLifecycleDestroyed()) { recycledViewPool.clear() } } @@ -636,9 +636,9 @@ open class EpoxyRecyclerView @JvmOverloads constructor( private const val DEFAULT_ADAPTER_REMOVAL_DELAY_MS = 2000 /** - * Store one unique pool per activity. They are cleared out when activities are destroyed, so this - * only needs to hold pools for active activities. + * Store one unique pool per [androidx.lifecycle.LifecycleOwner]. They are cleared out when + * lifecycles is destroyed, so this only needs to hold pools for active lifecycles. */ - private val ACTIVITY_RECYCLER_POOL = ActivityRecyclerPool() + private val LIFECYCLE_OWNER_RECYCLER_POOL = LifecycleOwnerRecyclerPool() } } diff --git a/epoxy-adapter/src/main/java/com/airbnb/epoxy/ActivityRecyclerPool.kt b/epoxy-adapter/src/main/java/com/airbnb/epoxy/LifecycleOwnerRecyclerPool.kt similarity index 69% rename from epoxy-adapter/src/main/java/com/airbnb/epoxy/ActivityRecyclerPool.kt rename to epoxy-adapter/src/main/java/com/airbnb/epoxy/LifecycleOwnerRecyclerPool.kt index 1df3522178..015d3d331c 100644 --- a/epoxy-adapter/src/main/java/com/airbnb/epoxy/ActivityRecyclerPool.kt +++ b/epoxy-adapter/src/main/java/com/airbnb/epoxy/LifecycleOwnerRecyclerPool.kt @@ -1,10 +1,7 @@ package com.airbnb.epoxy -import android.app.Activity import android.content.Context import android.content.ContextWrapper -import android.os.Build -import androidx.core.view.ViewCompat import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleObserver import androidx.lifecycle.LifecycleOwner @@ -13,11 +10,11 @@ import androidx.recyclerview.widget.RecyclerView import java.lang.ref.WeakReference import java.util.ArrayList -internal class ActivityRecyclerPool { +internal class LifecycleOwnerRecyclerPool { /** - * Store one unique pool per activity. They are cleared out when activities are destroyed, so this - * only needs to hold pools for active activities. + * Store one unique pool per lifecycle owner. They are cleared out when lifecycles are destroyed, + * so this only needs to hold pools for active lifecycles. */ private val pools = ArrayList(5) @@ -40,9 +37,9 @@ internal class ActivityRecyclerPool { poolToUse = poolReference // finish iterating to remove any old contexts } - poolReference.context.isActivityDestroyed() -> { - // A pool from a different activity that was destroyed. - // Clear the pool references to allow the activity to be GC'd + poolReference.context.isLifecycleDestroyed() -> { + // A pool from a different lifecycle owner that was destroyed. + // Clear the pool references to allow the lifecycle owner to be GC'd poolReference.viewPool.clear() iterator.remove() } @@ -59,7 +56,7 @@ internal class ActivityRecyclerPool { } fun clearIfDestroyed(pool: PoolReference) { - if (pool.context.isActivityDestroyed()) { + if (pool.context.isLifecycleDestroyed()) { pool.viewPool.clear() pools.remove(pool) } @@ -81,7 +78,7 @@ internal class ActivityRecyclerPool { internal class PoolReference( context: Context, val viewPool: RecyclerView.RecycledViewPool, - private val parent: ActivityRecyclerPool + private val parent: LifecycleOwnerRecyclerPool ) : LifecycleObserver { private val contextReference: WeakReference = WeakReference(context) @@ -92,28 +89,23 @@ internal class PoolReference( } @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY) - fun onContextDestroyed() { + fun onLifecycleDestroyed() { clearIfDestroyed() } } -internal fun Context?.isActivityDestroyed(): Boolean { +internal fun Context?.isLifecycleDestroyed(): Boolean { if (this == null) { return true } - if (this !is Activity) { - return (this as? ContextWrapper)?.baseContext?.isActivityDestroyed() ?: false + if (this !is LifecycleOwner) { + return (this as? ContextWrapper)?.baseContext?.isLifecycleDestroyed() ?: false } - if (isFinishing) { + if (lifecycle.currentState >= Lifecycle.State.DESTROYED) { return true } - return if (Build.VERSION.SDK_INT >= 17) { - isDestroyed - } else { - // Use this as a proxy for being destroyed on older devices - !ViewCompat.isAttachedToWindow(window.decorView) - } + return false }