Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-4561 Cherry pick Drawable Performance improvements #2052

Merged
merged 3 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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)
}

Expand All @@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -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/).
Expand All @@ -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)
) {

/**
Expand Down Expand Up @@ -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,
Expand All @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -75,6 +77,9 @@ internal class SessionReplayFeatureTest {
@Mock
lateinit var mockInternalLogger: InternalLogger

@Mock
lateinit var mockExecutorService: ExecutorService

@Mock
lateinit var mockSampler: Sampler

Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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
)
}

Expand Down