perf(replay): Use RGB_565 when alpha is unnecessary

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 ### Fixes
 
+- Prevent concurrent PixelCopy access during Session Replay masking and bitmap cleanup ([#5808](https://github.com/getsentry/sentry-java/pull/5808))
+- Reduce Session Replay screenshot memory usage ([#5821](https://github.com/getsentry/sentry-java/pull/5821))
 - Backfill release, environment, distribution, tags, and app version/build—and use the matching replay-on-error sample rate—for `ApplicationExitInfo` ANR and native crash events captured before SDK initialization, without reusing options cached by a later app update ([#5762](https://github.com/getsentry/sentry-java/pull/5762))
 - `SentryTagModifierNode.isImportantForBounds` now matches the default behavior and returns `true` ([#5789](https://github.com/getsentry/sentry-java/pull/5789))
 - Prevent a `StackOverflowError` when a `beforeSend`, `beforeBreadcrumb`, `beforeSendLog`, or `beforeEnvelope` callback triggers another capture (directly or through a logging integration such as Timber) ([#5737](https://github.com/getsentry/sentry-java/pull/5737))

sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt

@@ -97,7 +97,7 @@ internal class CanvasStrategy(
               Bitmap.createBitmap(
                 config.recordingWidth,
                 config.recordingHeight,
-                Bitmap.Config.ARGB_8888,
+                Bitmap.Config.RGB_565,
               )
           }
         }

sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt

@@ -51,14 +51,24 @@ internal class PixelCopyStrategy(
   private val executor = executorProvider.getExecutor()
   private val mainLooperHandler = executorProvider.getMainLooperHandler()
   private val screenshot =
-    Bitmap.createBitmap(config.recordingWidth, config.recordingHeight, Bitmap.Config.ARGB_8888)
+    Bitmap.createBitmap(
+      config.recordingWidth,
+      config.recordingHeight,
+      if (options.sessionReplay.isCaptureSurfaceViews) {
+        Bitmap.Config.ARGB_8888
+      } else {
+        Bitmap.Config.RGB_565
+      },
+    )
   private val prescaledMatrix by
     lazy(NONE) { Matrix().apply { preScale(config.scaleFactorX, config.scaleFactorY) } }
   private val lastCaptureSuccessful = AtomicBoolean(false)
   private val maskRenderer = MaskRenderer()
   private val contentChanged = AtomicBoolean(false)
   private val unstableCaptures = AtomicInteger(0)
   private val isClosed = AtomicBoolean(false)
+  private val frameInFlight = AtomicBoolean(false)
+  private val cleanupScheduled = AtomicBoolean(false)
   private val dstOverPaint by
     lazy(NONE) { Paint().apply { xfermode = PorterDuffXfermode(PorterDuff.Mode.DST_OVER) } }
   private val screenshotCanvas by lazy(NONE) { Canvas(screenshot) }
@@ -77,8 +87,15 @@ internal class PixelCopyStrategy(
       return
     }
 
+    if (!frameInFlight.compareAndSet(false, true)) {
+      options.logger.log(DEBUG, "PixelCopyStrategy capture is already in flight, skipping")
+      markContentChanged()
+      return
+    }
+
     if (isClosed.get()) {
       options.logger.log(DEBUG, "PixelCopyStrategy is closed, not capturing screenshot")
+      finishFrame()
       return
     }
 
@@ -90,18 +107,21 @@ internal class PixelCopyStrategy(
         { copyResult: Int ->
           if (isClosed.get()) {
             options.logger.log(DEBUG, "PixelCopyStrategy is closed, ignoring capture result")
+            finishFrame()
             return@request
           }
 
           if (copyResult != PixelCopy.SUCCESS) {
             options.logger.log(INFO, "Failed to capture replay recording: %d", copyResult)
             unstableCaptures.set(0)
             lastCaptureSuccessful.set(false)
+            finishFrame()
             return@request
           }
 
           val changedDuringCapture = contentChanged.get()
           if (changedDuringCapture && shouldSkipUnstableCapture()) {
+            finishFrame()
             return@request
           }
 
@@ -116,15 +136,23 @@ internal class PixelCopyStrategy(
           root.traverse(viewHierarchy, options.sessionReplay, options.logger, surfaceViewNodes)
 
           if (surfaceViewNodes.isNullOrEmpty()) {
-            executor.submit(
-              ReplayRunnable("screenshot_recorder.mask") {
-                applyMaskingAndNotify(
-                  root,
-                  viewHierarchy,
-                  resetUnstableCaptures = !changedDuringCapture,
-                )
-              }
-            )
+            val submitted =
+              executor.submit(
+                ReplayRunnable("screenshot_recorder.mask") {
+                  try {
+                    applyMaskingAndNotify(
+                      root,
+                      viewHierarchy,
+                      resetUnstableCaptures = !changedDuringCapture,
+                    )
+                  } finally {
+                    finishFrame()
+                  }
+                }
+              )
+            if (submitted == null) {
+              finishFrame()
+            }
           } else {
             // Re-arm the recorder's contentChanged gate; SurfaceView redraws don't trigger
             // ViewTreeObserver.OnDrawListener, so we'd otherwise emit the same frame forever.
@@ -143,6 +171,7 @@ internal class PixelCopyStrategy(
       options.logger.log(WARNING, "Failed to capture replay recording", e)
       unstableCaptures.set(0)
       lastCaptureSuccessful.set(false)
+      finishFrame()
     }
   }
 
@@ -272,37 +301,46 @@ internal class PixelCopyStrategy(
     windowY: Int,
     resetUnstableCaptures: Boolean,
   ) {
-    executor.submit(
-      ReplayRunnable("screenshot_recorder.composite") {
-        if (isClosed.get() || screenshot.isRecycled) {
-          options.logger.log(DEBUG, "PixelCopyStrategy is closed, skipping compositing")
-          recycleCaptures(captures)
-          return@ReplayRunnable
-        }
+    val submitted =
+      executor.submit(
+        ReplayRunnable("screenshot_recorder.composite") {
+          try {
+            if (isClosed.get() || screenshot.isRecycled) {
+              options.logger.log(DEBUG, "PixelCopyStrategy is closed, skipping compositing")
+              recycleCaptures(captures)
+              return@ReplayRunnable
+            }
 
-        for (capture in captures) {
-          if (capture == null) continue
-          if (capture.bitmap.isRecycled) continue
-
-          compositeSurfaceViewInto(
-            screenshotCanvas,
-            dstOverPaint,
-            tmpSrcRect,
-            tmpDstRect,
-            capture.bitmap,
-            capture.x,
-            capture.y,
-            windowX,
-            windowY,
-            config.scaleFactorX,
-            config.scaleFactorY,
-          )
-          capture.bitmap.recycle()
-        }
+            for (capture in captures) {
+              if (capture == null) continue
+              if (capture.bitmap.isRecycled) continue
+
+              compositeSurfaceViewInto(
+                screenshotCanvas,
+                dstOverPaint,
+                tmpSrcRect,
+                tmpDstRect,
+                capture.bitmap,
+                capture.x,
+                capture.y,
+                windowX,
+                windowY,
+                config.scaleFactorX,
+                config.scaleFactorY,
+              )
+              capture.bitmap.recycle()
+            }
 
-        applyMaskingAndNotify(root, viewHierarchy, resetUnstableCaptures)
-      }
-    )
+            applyMaskingAndNotify(root, viewHierarchy, resetUnstableCaptures)
+          } finally {
+            finishFrame()
+          }
+        }
+      )
+    if (submitted == null) {
+      recycleCaptures(captures)
+      finishFrame()
+    }
   }
 
   private fun recycleCaptures(captures: Array<SurfaceViewCapture?>) {
@@ -322,14 +360,30 @@ internal class PixelCopyStrategy(
   }
 
   override fun emitLastScreenshot() {
-    if (lastCaptureSuccessful() && !screenshot.isRecycled) {
+    if (!frameInFlight.get() && lastCaptureSuccessful() && !screenshot.isRecycled) {
       screenshotRecorderCallback?.onScreenshotRecorded(screenshot)
     }
   }
 
   override fun close() {
     isClosed.set(true)
     unstableCaptures.set(0)
+    if (!frameInFlight.get()) {
+      scheduleCleanup()
+    }
+  }
+
+  private fun finishFrame() {
+    frameInFlight.set(false)
+    if (isClosed.get()) {
+      scheduleCleanup()
+    }
+  }
+
+  private fun scheduleCleanup() {
+    if (!cleanupScheduled.compareAndSet(false, true)) {
+      return
+    }
     executor.submit(
       ReplayRunnable(
         "PixelCopyStrategy.close",

sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt

@@ -27,6 +27,7 @@ import io.sentry.android.replay.ScreenshotRecorderCallback
 import io.sentry.android.replay.ScreenshotRecorderConfig
 import io.sentry.android.replay.util.DebugOverlayDrawable
 import io.sentry.android.replay.util.MainLooperHandler
+import java.util.concurrent.Future
 import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicReference
@@ -37,6 +38,7 @@ import kotlin.test.assertFalse
 import kotlin.test.assertTrue
 import org.junit.runner.RunWith
 import org.mockito.kotlin.any
+import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.doAnswer
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
@@ -140,6 +142,101 @@ class PixelCopyStrategyTest {
     if (failure.get() != null) throw failure.get()
   }
 
+  @Test
+  @Config(shadows = [DeferredWindowPixelCopyShadow::class])
+  fun `capture drops frame while PixelCopy is in flight`() {
+    val activity = buildActivity(SimpleActivity::class.java).setup()
+    shadowOf(Looper.getMainLooper()).idle()
+    val root = activity.get().findViewById<View>(android.R.id.content)
+    val strategy = fixture.getSut(executor = fixture.inlineExecutor())
+
+    strategy.capture(root)
+    strategy.capture(root)
+
+    assertTrue(fixture.contentChangedMarked.get())
+
+    DeferredWindowPixelCopyShadow.flush()
+    shadowOf(Looper.getMainLooper()).idle()
+
+    verify(fixture.callback).onScreenshotRecorded(any<Bitmap>())
+
+    strategy.capture(root)
+    DeferredWindowPixelCopyShadow.flush()
+    shadowOf(Looper.getMainLooper()).idle()
+
+    verify(fixture.callback, times(2)).onScreenshotRecorded(any<Bitmap>())
+  }
+
+  @Test
+  @Config(shadows = [DeferredWindowPixelCopyShadow::class])
+  fun `capture drops frame while masking is in flight`() {
+    val activity = buildActivity(SimpleActivity::class.java).setup()
+    shadowOf(Looper.getMainLooper()).idle()
+    val root = activity.get().findViewById<View>(android.R.id.content)
+    val tasks = mutableListOf<Runnable>()
+    val executor = mock<ScheduledExecutorService>()
+    whenever(executor.submit(any<Runnable>())).doAnswer {
+      tasks += it.getArgument<Runnable>(0)
+      mock<Future<*>>()
+    }
+    val strategy = fixture.getSut(executor)
+
+    strategy.capture(root)
+    DeferredWindowPixelCopyShadow.flush()
+    shadowOf(Looper.getMainLooper()).idle()
+    strategy.capture(root)
+    DeferredWindowPixelCopyShadow.flush()
+    shadowOf(Looper.getMainLooper()).idle()
+
+    assertEquals(1, tasks.size)
+    tasks.removeAt(0).run()
+
+    strategy.capture(root)
+    DeferredWindowPixelCopyShadow.flush()
+    shadowOf(Looper.getMainLooper()).idle()
+
+    assertEquals(1, tasks.size)
+  }
+
+  @Test
+  @Config(shadows = [DeferredWindowPixelCopyShadow::class])
+  fun `emitLastScreenshot skips while frame is in flight`() {
+    val activity = buildActivity(SimpleActivity::class.java).setup()
+    shadowOf(Looper.getMainLooper()).idle()
+    val root = activity.get().findViewById<View>(android.R.id.content)
+    val strategy = fixture.getSut(executor = fixture.inlineExecutor())
+    captureStableFrame(strategy, root)
+
+    strategy.capture(root)
+    strategy.emitLastScreenshot()
+
+    verify(fixture.callback).onScreenshotRecorded(any<Bitmap>())
+
+    DeferredWindowPixelCopyShadow.flush()
+    shadowOf(Looper.getMainLooper()).idle()
+    verify(fixture.callback, times(2)).onScreenshotRecorded(any<Bitmap>())
+  }
+
+  @Test
+  @Config(shadows = [DeferredWindowPixelCopyShadow::class])
+  fun `close defers cleanup until PixelCopy completes`() {
+    val activity = buildActivity(SimpleActivity::class.java).setup()
+    shadowOf(Looper.getMainLooper()).idle()
+    val root = activity.get().findViewById<View>(android.R.id.content)
+    val executor = mock<ScheduledExecutorService>()
+    val strategy = fixture.getSut(executor)
+
+    strategy.capture(root)
+    strategy.close()
+
+    verify(executor, never()).submit(any<Runnable>())
+
+    DeferredWindowPixelCopyShadow.flush()
+    shadowOf(Looper.getMainLooper()).idle()
+
+    verify(executor).submit(any<Runnable>())
+  }
+
   @Test
   @Config(shadows = [DeferredWindowPixelCopyShadow::class])
   fun `capture skips the first unstable PixelCopy result`() {
@@ -214,7 +311,9 @@ class PixelCopyStrategyTest {
 
     assertFalse(fixture.contentChangedMarked.get())
     assertTrue(strategy.lastCaptureSuccessful())
-    verify(fixture.callback).onScreenshotRecorded(any<Bitmap>())
+    val screenshot = argumentCaptor<Bitmap>()
+    verify(fixture.callback).onScreenshotRecorded(screenshot.capture())
+    assertEquals(Bitmap.Config.RGB_565, screenshot.firstValue.config)
   }
 
   @Test
@@ -246,7 +345,9 @@ class PixelCopyStrategyTest {
     shadowOf(Looper.getMainLooper()).idle()
 
     assertTrue(strategy.lastCaptureSuccessful())
-    verify(fixture.callback).onScreenshotRecorded(any<Bitmap>())
+    val screenshot = argumentCaptor<Bitmap>()
+    verify(fixture.callback).onScreenshotRecorded(screenshot.capture())
+    assertEquals(Bitmap.Config.ARGB_8888, screenshot.firstValue.config)
   }
 
   @Test