Merge pull request #2052 from DataDog/xgouchet/RUM-4561/sr_drawable_perf

RUM-4561 Cherry pick Drawable Performance improvements
DataDog · May 27, 2024 · a6f7ed2 · a6f7ed2
2 parents 45b2914 + 3122210
commit a6f7ed2
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 * [IMPROVEMENT] Session Replay: Add telemetry to detect uncovered View/Drawable in Session Replay. See [#2028](https://github.com/DataDog/dd-sdk-android/pull/2028)
 * [IMPROVEMENT] Session Replay: Improve `SeekBarMapper`. See [#2037](https://github.com/DataDog/dd-sdk-android/pull/2037)
 * [IMPROVEMENT] RUM: Flag critical events in custom persistence. See [#2044](https://github.com/DataDog/dd-sdk-android/pull/2044)
+* [IMPROVEMENT] Delegate Drawable copy to background thread. See [#2048](https://github.com/DataDog/dd-sdk-android/pull/2048)
 * [MAINTENANCE] Next dev iteration. See [#2020](https://github.com/DataDog/dd-sdk-android/pull/2020)
 * [MAINTENANCE] Merge release `2.9.0` into `develop` branch. See [#2023](https://github.com/DataDog/dd-sdk-android/pull/2023)
 * [MAINTENANCE] Session Replay: Improve UT for SR Obfuscators. See [#2031](https://github.com/DataDog/dd-sdk-android/pull/2031)

diff --git a/features/dd-sdk-android-session-replay/consumer-rules.pro b/features/dd-sdk-android-session-replay/consumer-rules.pro
@@ -5,3 +5,4 @@
 
 # Kept for our internal telemetry
 -keepnames class com.datadog.android.sessionreplay.internal.recorder.listener.WindowsOnDrawListener
+-keepnames class * extends com.datadog.android.sessionreplay.recorder.mapper.WireframeMapper
diff --git a/.../main/kotlin/com/datadog/android/sessionreplay/internal/recorder/SessionReplayRecorder.kt b/.../main/kotlin/com/datadog/android/sessionreplay/internal/recorder/SessionReplayRecorder.kt
@@ -130,7 +130,11 @@ internal class SessionReplayRecorder : OnWindowRefreshedCallback, Recorder {
             applicationId = applicationId,
             recordedDataQueueHandler = recordedDataQueueHandler,
             bitmapCachesManager = bitmapCachesManager,
-            drawableUtils = DrawableUtils(internalLogger, bitmapCachesManager),
+            drawableUtils = DrawableUtils(
+                internalLogger,
+                bitmapCachesManager,
+                sdkCore.createSingleThreadExecutorService("drawables")
+            ),
             logger = internalLogger,
             md5HashGenerator = MD5HashGenerator(internalLogger),
             webPImageCompression = WebPImageCompression(internalLogger)

diff --git a/.../src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/TreeViewTraversal.kt b/.../src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/TreeViewTraversal.kt
@@ -9,13 +9,15 @@ package com.datadog.android.sessionreplay.internal.recorder
 import android.view.View
 import android.view.ViewGroup
 import com.datadog.android.api.InternalLogger
+import com.datadog.android.api.feature.measureMethodCallPerf
 import com.datadog.android.sessionreplay.MapperTypeWrapper
 import com.datadog.android.sessionreplay.internal.async.RecordedDataQueueRefs
 import com.datadog.android.sessionreplay.internal.recorder.mapper.QueueStatusCallback
 import com.datadog.android.sessionreplay.model.MobileSegment
 import com.datadog.android.sessionreplay.recorder.MappingContext
 import com.datadog.android.sessionreplay.recorder.mapper.TraverseAllChildrenMapper
 import com.datadog.android.sessionreplay.recorder.mapper.WireframeMapper
+import com.datadog.android.sessionreplay.utils.AsyncJobStatusCallback
 import com.datadog.android.sessionreplay.utils.NoOpAsyncJobStatusCallback
 
 internal class TreeViewTraversal(
@@ -39,29 +41,32 @@ internal class TreeViewTraversal(
         }
 
         val traversalStrategy: TraversalStrategy
-        val resolvedWireframes: List<MobileSegment.Wireframe>
+
         val noOpCallback = NoOpAsyncJobStatusCallback()
+        val jobStatusCallback: AsyncJobStatusCallback
 
         // try to resolve from the exhaustive type mappers
-        val mapper = findMapperForView(view)
+        var mapper = findMapperForView(view)
 
         if (mapper != null) {
-            val queueStatusCallback = QueueStatusCallback(recordedDataQueueRefs)
+            jobStatusCallback = QueueStatusCallback(recordedDataQueueRefs)
             traversalStrategy = if (mapper is TraverseAllChildrenMapper) {
                 TraversalStrategy.TRAVERSE_ALL_CHILDREN
             } else {
                 TraversalStrategy.STOP_AND_RETURN_NODE
             }
-            resolvedWireframes = mapper.map(view, mappingContext, queueStatusCallback, internalLogger)
         } else if (isDecorView(view)) {
             traversalStrategy = TraversalStrategy.TRAVERSE_ALL_CHILDREN
-            resolvedWireframes = decorViewMapper.map(view, mappingContext, noOpCallback, internalLogger)
+            mapper = decorViewMapper
+            jobStatusCallback = noOpCallback
         } else if (view is ViewGroup) {
             traversalStrategy = TraversalStrategy.TRAVERSE_ALL_CHILDREN
-            resolvedWireframes = defaultViewMapper.map(view, mappingContext, noOpCallback, internalLogger)
+            mapper = defaultViewMapper
+            jobStatusCallback = noOpCallback
         } else {
             traversalStrategy = TraversalStrategy.STOP_AND_RETURN_NODE
-            resolvedWireframes = defaultViewMapper.map(view, mappingContext, noOpCallback, internalLogger)
+            mapper = defaultViewMapper
+            jobStatusCallback = noOpCallback
             val viewType = view.javaClass.canonicalName ?: view.javaClass.name
 
             internalLogger.log(
@@ -76,6 +81,14 @@ internal class TreeViewTraversal(
             )
         }
 
+        val resolvedWireframes = internalLogger.measureMethodCallPerf(
+            javaClass,
+            "$METHOD_CALL_MAP_PREFIX ${mapper.javaClass.simpleName}",
+            METHOD_CALL_SAMPLING_RATE
+        ) {
+            mapper.map(view, mappingContext, jobStatusCallback, internalLogger)
+        }
+
         return TraversedTreeView(resolvedWireframes, traversalStrategy)
     }
 
@@ -93,4 +106,9 @@ internal class TreeViewTraversal(
         val mappedWireframes: List<MobileSegment.Wireframe>,
         val nextActionStrategy: TraversalStrategy
     )
+
+    companion object {
+        const val METHOD_CALL_SAMPLING_RATE = 5f
+        private const val METHOD_CALL_MAP_PREFIX: String = "Map with"
+    }
 }
diff --git a/...-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt b/...-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt
@@ -1,4 +1,3 @@
-
 /*
  * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
  * This product includes software developed at Datadog (https://www.datadoghq.com/).
@@ -13,25 +12,25 @@ import android.graphics.Bitmap.Config
 import android.graphics.Color
 import android.graphics.PorterDuff
 import android.graphics.drawable.Drawable
-import android.os.Handler
-import android.os.Looper
 import android.util.DisplayMetrics
 import androidx.annotation.MainThread
 import androidx.annotation.VisibleForTesting
 import androidx.annotation.WorkerThread
 import com.datadog.android.api.InternalLogger
+import com.datadog.android.core.internal.utils.submitSafe
 import com.datadog.android.sessionreplay.internal.recorder.resources.BitmapCachesManager
 import com.datadog.android.sessionreplay.internal.recorder.resources.ResourceResolver
 import com.datadog.android.sessionreplay.internal.recorder.wrappers.BitmapWrapper
 import com.datadog.android.sessionreplay.internal.recorder.wrappers.CanvasWrapper
+import java.util.concurrent.ExecutorService
 import kotlin.math.sqrt
 
 internal class DrawableUtils(
-    private val logger: InternalLogger,
+    private val internalLogger: InternalLogger,
     private val bitmapCachesManager: BitmapCachesManager,
-    private val bitmapWrapper: BitmapWrapper = BitmapWrapper(logger),
-    private val canvasWrapper: CanvasWrapper = CanvasWrapper(logger),
-    private val mainThreadHandler: Handler = Handler(Looper.getMainLooper())
+    private val executorService: ExecutorService,
+    private val bitmapWrapper: BitmapWrapper = BitmapWrapper(internalLogger),
+    private val canvasWrapper: CanvasWrapper = CanvasWrapper(internalLogger)
 ) {
 
     /**
@@ -59,8 +58,8 @@ internal class DrawableUtils(
             resizeBitmapCallback = object :
                 ResizeBitmapCallback {
                 override fun onSuccess(bitmap: Bitmap) {
-                    mainThreadHandler.post {
-                        @Suppress("ThreadSafety") // this runs on the main thread
+                    executorService.submitSafe("drawOnCanvas", internalLogger) {
+                        @Suppress("ThreadSafety") // this runs on a background thread
                         drawOnCanvas(
                             resources,
                             bitmap,
@@ -71,7 +70,7 @@ internal class DrawableUtils(
                 }
 
                 override fun onFailure() {
-                    logger.log(
+                    internalLogger.log(
                         InternalLogger.Level.ERROR,
                         InternalLogger.Target.MAINTAINER,
                         { FAILED_TO_CREATE_SCALED_BITMAP_ERROR }

diff --git a/...ay/src/test/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeatureTest.kt b/...ay/src/test/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeatureTest.kt
@@ -36,6 +36,7 @@ import org.junit.jupiter.api.extension.Extensions
 import org.mockito.Mock
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.junit.jupiter.MockitoSettings
+import org.mockito.kotlin.any
 import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.eq
@@ -50,6 +51,7 @@ import org.mockito.quality.Strictness
 import java.util.Locale
 import java.util.UUID
 import java.util.concurrent.CountDownLatch
+import java.util.concurrent.ExecutorService
 import java.util.concurrent.TimeUnit
 
 @Extensions(
@@ -75,6 +77,9 @@ internal class SessionReplayFeatureTest {
     @Mock
     lateinit var mockInternalLogger: InternalLogger
 
+    @Mock
+    lateinit var mockExecutorService: ExecutorService
+
     @Mock
     lateinit var mockSampler: Sampler
 
@@ -88,6 +93,8 @@ internal class SessionReplayFeatureTest {
         whenever(mockSampler.getSampleRate()).thenReturn(fakeSampleRate)
         fakeSessionId = UUID.randomUUID().toString()
         whenever(mockSdkCore.internalLogger) doReturn mockInternalLogger
+        whenever(mockSdkCore.createSingleThreadExecutorService(any())) doReturn mockExecutorService
+
         testedFeature = SessionReplayFeature(
             sdkCore = mockSdkCore,
             customEndpointUrl = fakeConfiguration.customEndpointUrl,

diff --git a/...lay/src/test/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtilsTest.kt b/...lay/src/test/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtilsTest.kt
@@ -11,7 +11,6 @@ import android.graphics.Bitmap
 import android.graphics.Canvas
 import android.graphics.drawable.Drawable
 import android.graphics.drawable.Drawable.ConstantState
-import android.os.Handler
 import android.util.DisplayMetrics
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.sessionreplay.forge.ForgeConfigurator
@@ -48,6 +47,7 @@ import java.util.concurrent.Future
 @MockitoSettings(strictness = Strictness.LENIENT)
 @ForgeConfiguration(ForgeConfigurator::class)
 internal class DrawableUtilsTest {
+
     private lateinit var testedDrawableUtils: DrawableUtils
 
     @Mock
@@ -80,9 +80,6 @@ internal class DrawableUtilsTest {
     @Mock
     private lateinit var mockBitmapCreationCallback: ResourceResolver.BitmapCreationCallback
 
-    @Mock
-    private lateinit var mockMainThreadHandler: Handler
-
     @Mock
     lateinit var mockConstantState: ConstantState
 
@@ -95,6 +92,9 @@ internal class DrawableUtilsTest {
     @Mock
     private lateinit var mockLogger: InternalLogger
 
+    @Mock
+    private lateinit var mockFuture: Future<Unit>
+
     @BeforeEach
     fun setup() {
         whenever(mockConstantState.newDrawable(mockResources)).thenReturn(mockSecondDrawable)
@@ -106,25 +106,18 @@ internal class DrawableUtilsTest {
         whenever(mockBitmap.config).thenReturn(mockConfig)
         whenever(mockBitmapCachesManager.getBitmapByProperties(any(), any(), any())).thenReturn(null)
 
-        doAnswer { invocation ->
-            val work = invocation.getArgument(0) as Runnable
-            work.run()
-            null
-        }.whenever(mockMainThreadHandler).post(
-            any()
-        )
-
-        whenever(mockExecutorService.execute(any())).then {
-            (it.arguments[0] as Runnable).run()
-            mock<Future<Boolean>>()
+        whenever(mockExecutorService.submit(any())) doAnswer {
+            val runnable = it.getArgument<Runnable>(0)
+            runnable.run()
+            mockFuture
         }
 
         testedDrawableUtils = DrawableUtils(
             bitmapWrapper = mockBitmapWrapper,
             canvasWrapper = mockCanvasWrapper,
             bitmapCachesManager = mockBitmapCachesManager,
-            mainThreadHandler = mockMainThreadHandler,
-            logger = mockLogger
+            executorService = mockExecutorService,
+            internalLogger = mockLogger
         )
     }