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 Delegate Drawable copy to background thread #2048

Merged
merged 2 commits into from
May 23, 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
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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to what was done with WindowsOnDrawListener, do we need to also add this class to proguard rules to prevent classname obfuscation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense indeeed

"$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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it safe to operate with bitmap/canvas from the worker thread on all Android versions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the tests I've done, it seems so. I'll do more test to ensure it works on all levels.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does work on all versions 👍

@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