From 0167ebd9a4ca8f6c84b93f88568b30c4679a1406 Mon Sep 17 00:00:00 2001 From: "Xavier F. Gouchet" Date: Wed, 22 May 2024 19:07:58 +0200 Subject: [PATCH 1/3] Delegate Drawable copy to background thread --- .../recorder/SessionReplayRecorder.kt | 6 +++- .../internal/recorder/TreeViewTraversal.kt | 32 +++++++++++++++---- .../internal/utils/DrawableUtils.kt | 19 ++++++----- .../internal/SessionReplayFeatureTest.kt | 7 ++++ .../internal/utils/DrawableUtilsTest.kt | 27 ++++++---------- 5 files changed, 56 insertions(+), 35 deletions(-) diff --git a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/SessionReplayRecorder.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/SessionReplayRecorder.kt index 0169680f4d..794e814722 100644 --- a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/SessionReplayRecorder.kt +++ b/features/dd-sdk-android-session-replay/src/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/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/TreeViewTraversal.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/TreeViewTraversal.kt index 8dbfc7ea50..842f88efdd 100644 --- a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/TreeViewTraversal.kt +++ b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/TreeViewTraversal.kt @@ -9,6 +9,7 @@ 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 @@ -16,6 +17,7 @@ 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 + 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, 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/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt index 33db3aeaf0..42b5903ef7 100644 --- a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt +++ b/features/dd-sdk-android-session-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/features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeatureTest.kt b/features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeatureTest.kt index a94d790e06..82c89c06a7 100644 --- a/features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeatureTest.kt +++ b/features/dd-sdk-android-session-replay/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/features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtilsTest.kt b/features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtilsTest.kt index dfe95a707f..038d445c9e 100644 --- a/features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtilsTest.kt +++ b/features/dd-sdk-android-session-replay/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 + @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>() + whenever(mockExecutorService.submit(any())) doAnswer { + val runnable = it.getArgument(0) + runnable.run() + mockFuture } testedDrawableUtils = DrawableUtils( bitmapWrapper = mockBitmapWrapper, canvasWrapper = mockCanvasWrapper, bitmapCachesManager = mockBitmapCachesManager, - mainThreadHandler = mockMainThreadHandler, - logger = mockLogger + executorService = mockExecutorService, + internalLogger = mockLogger ) } From 0020db06cf2dbd88e0e3cd373f4778585c8796e6 Mon Sep 17 00:00:00 2001 From: "Xavier F. Gouchet" Date: Thu, 23 May 2024 10:06:01 +0200 Subject: [PATCH 2/3] Prevent obfuscation of Mapper classes --- features/dd-sdk-android-session-replay/consumer-rules.pro | 1 + 1 file changed, 1 insertion(+) diff --git a/features/dd-sdk-android-session-replay/consumer-rules.pro b/features/dd-sdk-android-session-replay/consumer-rules.pro index 63bcb5f702..fedb8b58fa 100644 --- 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 From 312221057c94ac8d3adf08ce06f34a6f1bf59a46 Mon Sep 17 00:00:00 2001 From: "Xavier F. Gouchet" Date: Thu, 23 May 2024 11:49:22 +0200 Subject: [PATCH 3/3] RUM-4561 update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a19070ee60..f9bdad7dd1 100644 --- 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)