Skip to content

Commit

Permalink
clear controller state on final unmount
Browse files Browse the repository at this point in the history
Summary:
The most recent run of the nested tree experiment was increasing OOMs [[scuba](https://fburl.com/scuba/errorreporting_facebook_android_crashes/f74yx517)].

 {F1471144788}

On checking heap dumps from test groups [[query](https://fburl.com/scuba/omura_facebook_hprof_files/dlhejwgk)], we saw a number of `TreeState` objects being kept around in memory.

{F1471146040}

The inference is that the controller never clears the latest committed state when the nested tree is unmounted. This leads to more `TreeState` objects being accumulated in memory as more nested trees are rendered until the app OOMs. This also explains why the OOM was more prevalent on surfaces like Newsfeed and the Goodwill throwback screen. They tend to have a significant number of NT and Bloks units and can regularly unmount and remount new generations by way of refreshing.

This diff attaches a release listener to the controller and clears the committed tree state as soon as the nested tree is released

Reviewed By: adityasharat

Differential Revision: D55143679

fbshipit-source-id: 294fb9e713ba928ca92228e355e037ee5a5a9de4
  • Loading branch information
Daniel Famakin authored and facebook-github-bot committed Mar 20, 2024
1 parent c2cc775 commit 8890b88
Showing 1 changed file with 2 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ class NestedLithoTreeLifecycleProvider : LithoTreeLifecycleProvider {

private val listeners: MutableList<LithoTreeLifecycleProvider.OnReleaseListener> = mutableListOf()

private var _isReleased: Boolean = false
@Volatile private var _isReleased: Boolean = false

override val isReleased: Boolean
get() = _isReleased
Expand All @@ -344,6 +344,7 @@ class NestedLithoTreeLifecycleProvider : LithoTreeLifecycleProvider {
assertMainThread()
_isReleased = true
listeners.forEach { it.onReleased() }
listeners.clear()
}
}
// endregion
Expand Down

0 comments on commit 8890b88

Please sign in to comment.