Skip to content

Commit

Permalink
Desktop: fix memory leak after recomposition, when composition is dis…
Browse files Browse the repository at this point in the history
…posed.

Fixes JetBrains/compose-multiplatform#538 and https://partnerissuetracker.corp.google.com/issues/170869626#comment4

As far as I figure out how observation works, it is enough to call `clearInvalidObservations` in the end of the every frame.
Because all changes will be commited before (on recomposition/layout/draw).
Similar code is in https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt;l=378;drc=f3b4f4f28cf61d4434af59d9247a9d2f6d5d70e7.

Bug: 183493610
Test: JetBrains/compose-multiplatform#538 (comment), click "Next" multiple times, run VisualVM, run GC, dump heap, find "Presenter"
Test: ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true
Change-Id:  I864f4e7aeca28547465ea468229e19426d7b2827
  • Loading branch information
igordmn committed Mar 30, 2021
1 parent f0685ad commit 94dce3a
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package androidx.compose.ui.platform

import androidx.compose.runtime.getValue
import androidx.compose.runtime.key
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.ExperimentalComposeUiApi
Expand Down Expand Up @@ -191,6 +190,7 @@ internal class DesktopOwner(
override fun onDetach(node: LayoutNode) {
measureAndLayoutDelegate.onNodeDetached(node)
snapshotObserver.clear(node)
needClearObservations = true
}

override val measureIteration: Long get() = measureAndLayoutDelegate.measureIteration
Expand All @@ -207,6 +207,16 @@ internal class DesktopOwner(
measureAndLayout()
needsDraw = false
draw(canvas)
clearInvalidObservations()
}

private var needClearObservations = false

private fun clearInvalidObservations() {
if (needClearObservations) {
snapshotObserver.clearInvalidObservations()
needClearObservations = false
}
}

private fun requestLayout() {
Expand Down Expand Up @@ -248,7 +258,8 @@ internal class DesktopOwner(
invalidateParentLayer()
requestDraw()
},
drawBlock = drawBlock
drawBlock = drawBlock,
onDestroy = { needClearObservations = true }
)

override fun onSemanticsChange() = Unit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ import org.jetbrains.skija.ShadowUtils
internal class SkijaLayer(
private val getDensity: () -> Density,
private val invalidateParentLayer: () -> Unit,
private val drawBlock: (Canvas) -> Unit
private val drawBlock: (Canvas) -> Unit,
private val onDestroy: () -> Unit = {}
) : OwnedLayer {
private var size = IntSize.Zero
private var position = IntOffset.Zero
Expand Down Expand Up @@ -76,6 +77,7 @@ internal class SkijaLayer(
picture?.close()
pictureRecorder.close()
isDestroyed = true
onDestroy()
}

override fun resize(size: IntSize) {
Expand Down

0 comments on commit 94dce3a

Please sign in to comment.