cashapp / paparazzi

Render your Android screens without a physical device or emulator
https://cashapp.github.io/paparazzi/
Apache License 2.0
2.29k stars 213 forks source link

Getting OutOfMemoryErrors after ~500 screenshots #915

Closed jmartinesp closed 1 year ago

jmartinesp commented 1 year ago

Description

In our project we're using a combination of Showkase and Paparazzi to automatically generate screenshot tests for our components. Everything was working fine until quite recently, when we noticed at around 500 screenshots the record and verification tests were failing with OOMs.

Steps to Reproduce You can use this branch in our repo experiment/screenshots-oom, then run either recordPaparazziDebug or verifyPaparazziDebug.

If you run it with logs, you'll be able to see some logs like:

io.element.android.tests.uitests.ScreenshotTest > preview_tests[io.element.android.libraries.designsystem.theme.components_null_TextFields_TextFieldDarkPreview_0_null,NEXUS_5,1.0,en] STANDARD_OUT
    maxMemory  : 536870912
    totalMemory: 330301440

...

io.element.android.tests.uitests.ScreenshotTest > preview_tests_screens_light[preview] STANDARD_OUT
    maxMemory  : 536870912
    totalMemory: 536870912
io.element.android.tests.uitests.ScreenshotTest > preview_tests_screens_light[preview] STANDARD_ERROR
    Jun 01, 2023 9:48:06 AM app.cash.paparazzi.internal.PaparazziLogger logAndroidFramework
    INFO: Settings [3]: Can't get key animator_duration_scale from content://settings/global
    Jun 01, 2023 9:48:07 AM app.cash.paparazzi.internal.PaparazziLogger error
    SEVERE: broken: Failed executing Choreographer#doFrame
    java.lang.OutOfMemoryError: Java heap space
        at java.desktop/java.awt.image.DataBufferInt.<init>(DataBufferInt.java:75)
        at java.desktop/java.awt.image.Raster.createPackedRaster(Raster.java:539)
        at java.desktop/java.awt.image.DirectColorModel.createCompatibleWritableRaster(DirectColorModel.java:1032)
        at java.desktop/java.awt.image.BufferedImage.<init>(BufferedImage.java:333)
        at app.cash.paparazzi.internal.ImageUtils.scale(ImageUtils.kt:238)
        at app.cash.paparazzi.Paparazzi.scaleImage(Paparazzi.kt:412)
        at app.cash.paparazzi.Paparazzi.access$scaleImage(Paparazzi.kt:83)
        at app.cash.paparazzi.Paparazzi$takeSnapshots$1$2.invoke(Paparazzi.kt:325)
        at app.cash.paparazzi.Paparazzi$takeSnapshots$1$2.invoke(Paparazzi.kt:312)
        at app.cash.paparazzi.Paparazzi.withTime(Paparazzi.kt:360)
        at app.cash.paparazzi.Paparazzi.takeSnapshots(Paparazzi.kt:312)
        at app.cash.paparazzi.Paparazzi.snapshot(Paparazzi.kt:206)
        at app.cash.paparazzi.Paparazzi.snapshot$default(Paparazzi.kt:205)
        at app.cash.paparazzi.Paparazzi.snapshot(Paparazzi.kt:201)
        at app.cash.paparazzi.Paparazzi.snapshot$default(Paparazzi.kt:197)
        at io.element.android.tests.uitests.ScreenshotTestKt.screenshotTest(ScreenshotTest.kt:176)
        at io.element.android.tests.uitests.ScreenshotTestKt.access$screenshotTest(ScreenshotTest.kt:1)
        at io.element.android.tests.uitests.ScreenshotTest.preview_tests_screens_light(ScreenshotTest.kt:121)
        at jdk.internal.reflect.GeneratedMethodAccessor30.invoke(Unknown Source)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
        at app.cash.paparazzi.Paparazzi$apply$statement$1.evaluate(Paparazzi.kt:125)
        at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
        at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)

   io.element.android.tests.uitests.ScreenshotTest > preview_tests_screens_light[preview] FAILED
    java.lang.OutOfMemoryError: Java heap space: failed reallocation of scalar replaced objects

And every test adds a couple of MBs to the totalMemory value until the maxMemory is reached.

We tried removing everything that's not 'standard' from the tests, even tried moving from TestParameterInjector to Parameterized runners in case this was the source of the issue, but the issue persisted.

I'd upload a heap dump, but that's quite big, so it's better to just run the tests to generate one locally by running either paparazzi task.

Expected behavior Tests should run with no memory being retained in each run.

Additional information:

jrodbx commented 1 year ago

At first, I was suspecting that this was due to Electric Eel's Paparazzi, since Flamingo's recently introduced an Application Context, to prevent memory leaks with layoutlib's BridgeContext during Compose animations, but I just re-read and see you're using 1.3.0....will keep digging.

kevinzheng-ap commented 1 year ago

@jmartinesp After conducting some investigations, I identified it is related to the memory leak of context in BridgeContentResolver from animationScale map in WindowRecomposer.android.kt.

In your instance where lots of test cases are executed within same Paparazzi instance, an excessive accumulation of AndroidComposeView occurs, leading to the occurrence of memory leak.

An issue has been filed to the layoutlib team.

cc @jrodbx

jrodbx commented 1 year ago

Closing as "fixed", see https://issuetracker.google.com/issues/290990640 for current status.

agrosner commented 1 year ago

what would be an intermediate fix, don't cache the paparazzi instance in our tests and recreate it per test run? we're also seeing it now. Do we have to wait until 1.4?

davidvavra commented 1 year ago

Hello, we are also experiencing this problem and we had to workarounds like splitting screenshot testing on CI into multiple jobs (light mode separately from dark mode etc.)

It was great to meet @msya on Devfest Bootcamp event. We have some capacity in our company for helping open-source tools we use. Let us know if we could help with some investigation/pull request.

TWiStErRob commented 1 year ago

@davidvavra We have to wait until Iguana is stable and Paparazzi can reference its layoutlib. Until then maybe there's a hack as listed in the Google issue, I'm sure if you make that work somehow, and make it copy-paste-able or pull it, people will be grateful :)

agrosner commented 1 year ago

This fix works around the memory leak issue, tested on kotlin 1.9.10 This involves a couple of steps:

  1. Wrap Paparazzi in your own rule, and use a RunRules chain to run PaparazziCleanupRule at the end.
  2. Add PaparazziCleanupRule using a reflection hack to retrieve the private bridgeRenderSession object.
  3. Add the extension function that hacks around the RenderSession object.

TestRule wrapper

class PaparazziRule internal constructor(
    private val testDispatcher: TestDispatcher = UnconfinedTestDispatcher(),
    val paparazzi: Paparazzi,
) : TestRule {

    override fun apply(base: Statement, description: Description): Statement =
 RunRules(
            base,
            listOf(MainTestWatcher(testDispatcher), PaparazziCleanupRule(paparazzi), paparazzi),
            description,
        )
 // ** wrapper test rule code including snapshot functions calling into nested paparazzi
}

## Parazzi Reflection Hack
private class PaparazziCleanupRule(
        private val paparazzi: Paparazzi,
    ) : TestWatcher() {

        override fun finished(description: Description?) {
            super.finished(description)
            @Suppress("UNCHECKED_CAST")
            val renderSession: RenderSession = (
                paparazzi::class.memberProperties
                    .first { it.name == "bridgeRenderSession" } as KProperty1<Paparazzi, RenderSession>
                )
                .apply { isAccessible = true }
                .invoke(paparazzi)
            renderSession.disposeHack()
        }
    }

Rendersession Hack

package **

import androidx.compose.ui.platform.AndroidUiDispatcher
import androidx.compose.ui.platform.ComposeView
import app.cash.paparazzi.internal.ComposeViewAdapter
import com.android.ide.common.rendering.api.RenderSession
import com.android.ide.common.rendering.api.ViewInfo
import **.Logger
import java.lang.ref.WeakReference
import java.lang.reflect.InvocationTargetException
import java.util.concurrent.atomic.AtomicReference

private const val WINDOW_RECOMPOSER_ANDROID_KT_FQN =
    "androidx.compose.ui.platform.WindowRecomposer_androidKt"
private const val COMBINED_CONTEXT_FQN = "kotlin.coroutines.CombinedContext"
private const val TAG = "DisposeRenderSession"
private const val SNAPSHOT_KT_FQN = "androidx.compose.runtime.snapshots.SnapshotKt"

/**
 * Initiates a custom [RenderSession] disposal, involving clearing several static collections
 * including some Compose-related objects as well as executing default [RenderSession.dispose].
 */
fun RenderSession.disposeHack() {
    val applyObserversRef = AtomicReference<WeakReference<MutableCollection<*>?>?>(null)
    val toRunTrampolinedRef = AtomicReference<WeakReference<MutableCollection<*>?>?>(null)

    try {
        val windowRecomposer: Class<*> = Class.forName(WINDOW_RECOMPOSER_ANDROID_KT_FQN)
        val animationScaleField = windowRecomposer.getDeclaredField("animationScale")
        animationScaleField.isAccessible = true
        val animationScale = animationScaleField[windowRecomposer]
        if (animationScale is Map<*, *>) {
            (animationScale as MutableMap<*, *>).clear()
        }
    } catch (ex: ReflectiveOperationException) {
        // If the WindowRecomposer does not exist or the animationScale does not exist anymore,
        // ignore.
        Logger.warning(TAG, "Unable to dispose the recompose animationScale $ex")
    }
    applyObserversRef.set(WeakReference(findApplyObservers()))
    toRunTrampolinedRef.set(WeakReference(findToRunTrampolined()))

    execute {
        rootViews
            .filterNotNull()
            .forEach { v -> disposeIfCompose(v) }
    }
    val weakApplyObservers = applyObserversRef.get()
    if (weakApplyObservers != null) {
        val applyObservers = weakApplyObservers.get()
        applyObservers?.clear()
    }
    val weakToRunTrampolined = toRunTrampolinedRef.get()
    if (weakToRunTrampolined != null) {
        val toRunTrampolined = weakToRunTrampolined.get()
        toRunTrampolined?.clear()
    }
    dispose()
}

/**
 * Performs dispose() call against View object associated with [ViewInfo] if that object is an
 * instance of [ComposeViewAdapter]
 *
 * @param viewInfo a [ViewInfo] associated with the View object to be potentially disposed of
 */
private fun disposeIfCompose(viewInfo: ViewInfo) {
    val viewObject: Any? = viewInfo.viewObject
    if (viewObject !is ComposeViewAdapter) {
        return
    }
    try {
        val composeView = viewInfo.children[0].viewObject as ComposeView
        composeView.disposeComposition()
    } catch (ex: IllegalAccessException) {
        Logger.warning(TAG, "Unexpected error while disposing compose view $ex")
    } catch (ex: InvocationTargetException) {
        Logger.warning(TAG, "Unexpected error while disposing compose view $ex")
    }
}

private fun findApplyObservers(): MutableCollection<*>? {
    try {
        val applyObserversField = Class.forName(SNAPSHOT_KT_FQN).getDeclaredField("applyObservers")
        applyObserversField.isAccessible = true
        val applyObservers = applyObserversField[null]
        if (applyObservers is MutableCollection<*>) {
            return applyObservers
        }
        Logger.warning(TAG, "SnapshotsKt.applyObservers found but it is not a List")
    } catch (ex: ReflectiveOperationException) {
        Logger.warning(TAG, "Unable to find SnapshotsKt.applyObservers $ex")
    }
    return null
}

private fun findToRunTrampolined(): MutableCollection<*>? {
    try {
        val uiDispatcher = AndroidUiDispatcher::class.java
        val uiDispatcherCompanion = AndroidUiDispatcher.Companion::class.java
        val uiDispatcherCompanionField = uiDispatcher.getDeclaredField("Companion")
        val uiDispatcherCompanionObj = uiDispatcherCompanionField[null]
        val getMainMethod =
            uiDispatcherCompanion.getDeclaredMethod("getMain").apply { isAccessible = true }
        val mainObj = getMainMethod.invoke(uiDispatcherCompanionObj)
        val combinedContext = Class.forName(COMBINED_CONTEXT_FQN)
        val elementField = combinedContext.getDeclaredField("element").apply { isAccessible = true }
        val uiDispatcherObj = elementField[mainObj]

        val toRunTrampolinedField =
            uiDispatcher.getDeclaredField("toRunTrampolined").apply { isAccessible = true }
        val toRunTrampolinedObj = toRunTrampolinedField[uiDispatcherObj]
        if (toRunTrampolinedObj is MutableCollection<*>) {
            return toRunTrampolinedObj
        }
        Logger.warning(TAG, "AndroidUiDispatcher.toRunTrampolined found but it is not a MutableCollection")
    } catch (ex: ReflectiveOperationException) {
        Logger.warning(TAG, "Unable to find AndroidUiDispatcher.toRunTrampolined $ex")
    }
    return null
}
agrosner commented 11 months ago

Debugging through the ported version of the provided code from google in the issue, this does not fix the memory leak. Maybe with someone with better eyes could tell me what ive done wrong here.

confirming upgrading to 1.9.10 + compose 2023.10.01 + the code i posted, fixes the memory leak.

KatieBarnett commented 11 months ago

@agrosner I have tried what you suggest above and upgraded to kotlin 1.9.20 and compose 2023.10.01 but I am still getting OutOfMemoryError. Perhaps I have included your code incorrectly. Would have you have the full rule file?

agrosner commented 11 months ago

@KatieBarnett oops i did fix the code above. just updated prior comment with working code.

KatieBarnett commented 11 months ago

Got it working now, thanks @agrosner!

KennyGoers commented 9 months ago

Since we are still running out of memory in 1.3.2 I am trying this out of memory fix, but there is a reference to a MainTestWatcher that I can't find definition of, any one else trying this? Thanks,

Any thoughts on when a fix for this will be coming? 1.4? 1.3.3 snapshot?

KatieBarnett commented 8 months ago

This issue is still happening with version 1.3.3. The above hack needs to be altered to comment out

    if (viewObject !is ComposeViewAdapter) {
        return
    }

as ComposeViewAdapter is now internal.

The hack still appears to work with this commented out.

KatieBarnett commented 8 months ago

but there is a reference to a MainTestWatcher that I can't find definition of, any one else trying this?

@KennyGoers I am using it, I just included the rule in my test class directly as:

    @get:Rule
    val paparazziCleanupRule = PaparazziCleanupRule(paparazzi = paparazzi)

(paparazzi being the Paparazzi rule)

rschattauer commented 7 months ago

@jrodbx any chance we get this workaround into paparazzi itself or what is actually needed? I don't see this issue as resolved as it's not working with Iguana + 1.3.3 and still running into OOM.

KennyGoers commented 7 months ago

We are still very much having this issue as well, even with individual paparazzi instances for each view class where we generated tests dynamically previously. It's actually worse in 1.3.3 where things hang instead of just failing like they did in 1.3.1 :(

KennyGoers commented 7 months ago

but there is a reference to a MainTestWatcher that I can't find definition of, any one else trying this?

@KennyGoers I am using it, I just included the rule in my test class directly as:

    @get:Rule
    val paparazziCleanupRule = PaparazziCleanupRule(paparazzi = paparazzi)

(paparazzi being the Paparazzi rule)

I still can't see what MainTestWatcher is @KatieBarnett thoughts on that? Desperately trying to get this to work :/

Thanks for any help in this.

Working on this more, you meant to say you didn't need the Watcher thing? I am trying that with no change, not sure why but 1.3.2 and 1.3.3 hang gradle with this fix as I've attempted it from the description :/

SimonMarquis commented 7 months ago

Here are the results on adevinta/spark-android when giving 2g to the workers (the default 512M would prevent us from upgrading, Gradle indefinitely trying to GC to reclaim memory):

Version Output
1.3.1 image
1.3.2 image
1.3.3 image
SumeraMartin commented 6 months ago

Hi, just wanted to let you know that we faced the same problem. I have used the "hack" mentioned before with manual cleanup and seems like that it is working. I also didn't find what is the MainTestWatcher is but I just removed it and it seems like that it is working correctly event without that.

override fun apply(base: Statement, description: Description): Statement = RunRules(
            base,
            listOf(PaparazziCleanupRule(paparazzi), paparazzi),
            description,
        )
nuhkoca commented 4 months ago

In the version of 1.3.4, this line gives exception

.first { it.name == "bridgeRenderSession" } as KProperty1<Paparazzi, RenderSession>

java.util.NoSuchElementException: Collection contains no element matching the predicate.

agrosner commented 4 months ago

as of 1.3.4, its not needed. in our app with thousands of screenshot tests, there is no more leak upon upgrading. so you can remove the workaround.

nuhkoca commented 4 months ago

Thanks for informing @agrosner!