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-4720] Improve JS Refresh Rate reliability on Android #689

Merged
merged 1 commit into from
Jul 17, 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 packages/core/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ dependencies {
testImplementation "com.github.xgouchet.Elmyr:jvm:1.3.1"
testImplementation "org.mockito.kotlin:mockito-kotlin:5.1.0"
testImplementation "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version"

unmock 'org.robolectric:android-all:4.4_r1-robolectric-r2'
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ package com.datadog.reactnative

import android.content.Context
import android.util.Log
import android.view.Choreographer
import com.datadog.android.privacy.TrackingConsent
import com.datadog.android.rum.configuration.VitalsUpdateFrequency
import com.datadog.android.rum.RumPerformanceMetric
import com.facebook.react.bridge.LifecycleEventListener
import com.facebook.react.bridge.Promise
import com.facebook.react.bridge.ReactApplicationContext
import com.facebook.react.bridge.ReadableMap
Expand All @@ -21,12 +21,13 @@ import java.util.concurrent.atomic.AtomicBoolean

/** The entry point to initialize Datadog's features. */
class DdSdkImplementation(
reactContext: ReactApplicationContext,
private val datadog: DatadogWrapper = DatadogSDKWrapper()
private val reactContext: ReactApplicationContext,
private val datadog: DatadogWrapper = DatadogSDKWrapper(),
private val uiThreadExecutor: UiThreadExecutor = ReactUiThreadExecutor()
) {
internal val appContext: Context = reactContext.applicationContext
internal val reactContext: ReactApplicationContext = reactContext
internal val initialized = AtomicBoolean(false)
private var frameRateProvider: FrameRateProvider? = null

// region DdSdk

Expand All @@ -39,7 +40,23 @@ class DdSdkImplementation(

val nativeInitialization = DdSdkNativeInitialization(appContext, datadog)
nativeInitialization.initialize(ddSdkConfiguration)
monitorJsRefreshRate(ddSdkConfiguration)

this.frameRateProvider = createFrameRateProvider(ddSdkConfiguration)

reactContext.addLifecycleEventListener(object : LifecycleEventListener {
override fun onHostResume() {
frameRateProvider?.start()
}

override fun onHostPause() {
frameRateProvider?.stop()
}

override fun onHostDestroy() {
frameRateProvider?.stop()
}
marco-saia-datadog marked this conversation as resolved.
Show resolved Hide resolved
})

initialized.set(true)

promise.resolve(null)
Expand Down Expand Up @@ -150,26 +167,16 @@ class DdSdkImplementation(
}
}

private fun handlePostFrameCallbackError(e: IllegalStateException) {
datadog.telemetryError(e.message ?: MONITOR_JS_ERROR_MESSAGE, e)
}

private fun monitorJsRefreshRate(ddSdkConfiguration: DdSdkConfiguration) {
val frameTimeCallback = buildFrameTimeCallback(ddSdkConfiguration)
if (frameTimeCallback != null) {
reactContext.runOnJSQueueThread {
val vitalFrameCallback =
VitalFrameCallback(frameTimeCallback, ::handlePostFrameCallbackError) {
initialized.get()
}
try {
Choreographer.getInstance().postFrameCallback(vitalFrameCallback)
} catch (e: IllegalStateException) {
// This should never happen as the React Native thread always has a Looper
handlePostFrameCallbackError(e)
}
}
private fun createFrameRateProvider(
ddSdkConfiguration: DdSdkConfiguration
): FrameRateProvider? {
val frameTimeCallback = buildFrameTimeCallback(ddSdkConfiguration) ?: return null
val frameRateProvider = FrameRateProvider(frameTimeCallback, uiThreadExecutor)
reactContext.runOnJSQueueThread {
frameRateProvider.start()
}

return frameRateProvider
}

private fun buildFrameTimeCallback(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.reactnative

import com.facebook.react.modules.core.ChoreographerCompat

internal class FrameRateProvider(
reactFrameRateCallback: ((Double) -> Unit),
uiThreadExecutor: UiThreadExecutor
) {
private val frameCallback: FpsFrameCallback = FpsFrameCallback(
reactFrameRateCallback,
uiThreadExecutor
)

fun start() {
frameCallback.reset()
frameCallback.start()
}

fun stop() {
frameCallback.stop()
}
}

internal class FpsFrameCallback(
private val reactFrameRateCallback: ((Double) -> Unit),
private val uiThreadExecutor: UiThreadExecutor
) : ChoreographerCompat.FrameCallback() {

private var choreographer: ChoreographerCompat? = null
private var lastFrameTime = -1L

override fun doFrame(time: Long) {
if (lastFrameTime != -1L) {
reactFrameRateCallback((time - lastFrameTime).toDouble())
}
lastFrameTime = time
choreographer?.postFrameCallback(this)
}

fun start() {
uiThreadExecutor.runOnUiThread {
choreographer = ChoreographerCompat.getInstance()
choreographer?.postFrameCallback(this@FpsFrameCallback)
}
}

fun stop() {
uiThreadExecutor.runOnUiThread {
choreographer = ChoreographerCompat.getInstance()
choreographer?.removeFrameCallback(this@FpsFrameCallback)
}
}

fun reset() {
lastFrameTime = -1L
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* 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/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.reactnative

import com.facebook.react.bridge.UiThreadUtil

/**
* Simple UI Thread Executor. By default it is based on [UiThreadUtil.runOnUiThread].
*/
interface UiThreadExecutor {
/**
* Runs the given runnable on the UI Thread.
*/
fun runOnUiThread(runnable: Runnable)
}

internal class ReactUiThreadExecutor : UiThreadExecutor {
override fun runOnUiThread(runnable: Runnable) {
UiThreadUtil.runOnUiThread(runnable)
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package com.datadog.reactnative

import android.content.pm.PackageInfo
import android.os.Looper
import android.util.Log
import android.view.Choreographer
import com.datadog.android.DatadogSite
Expand All @@ -28,6 +29,7 @@ import com.datadog.android.trace.TraceConfiguration
import com.datadog.android.trace.TracingHeaderType
import com.datadog.tools.unit.GenericAssert.Companion.assertThat
import com.datadog.tools.unit.MockRumMonitor
import com.datadog.tools.unit.TestUiThreadExecutor
import com.datadog.tools.unit.forge.BaseConfigurator
import com.datadog.tools.unit.setStaticValue
import com.datadog.tools.unit.toReadableArray
Expand All @@ -36,6 +38,7 @@ import com.datadog.tools.unit.toReadableMap
import com.facebook.react.bridge.Promise
import com.facebook.react.bridge.ReactApplicationContext
import com.facebook.react.bridge.ReadableMap
import com.facebook.react.modules.core.ChoreographerCompat
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.annotation.AdvancedForgery
import fr.xgouchet.elmyr.annotation.BoolForgery
Expand Down Expand Up @@ -78,6 +81,13 @@ import org.mockito.kotlin.verifyNoInteractions
import org.mockito.kotlin.whenever
import org.mockito.quality.Strictness

fun mockChoreographerCompatInstance(mock: ChoreographerCompat = mock()) {
ChoreographerCompat::class.java.setStaticValue(
"sInstance",
mock
)
}

fun mockChoreographerInstance(mock: Choreographer = mock()) {
Choreographer::class.java.setStaticValue(
"sThreadInstance",
Expand All @@ -96,7 +106,6 @@ fun mockChoreographerInstance(mock: Choreographer = mock()) {
@MockitoSettings(strictness = Strictness.LENIENT)
@ForgeConfiguration(value = BaseConfigurator::class)
internal class DdSdkTest {

lateinit var testedBridgeSdk: DdSdkImplementation

@Mock(answer = Answers.RETURNS_DEEP_STUBS)
Expand Down Expand Up @@ -126,13 +135,24 @@ internal class DdSdkTest {
@Mock
lateinit var mockChoreographer: Choreographer
marco-saia-datadog marked this conversation as resolved.
Show resolved Hide resolved

@Mock
lateinit var mockChoreographerCompat: ChoreographerCompat

@BeforeEach
fun `set up`() {
val mockLooper = mock<Looper>()
whenever(mockLooper.thread) doReturn Thread.currentThread()
Looper::class.java.setStaticValue("sMainLooper", mockLooper)

whenever(mockDatadog.getRumMonitor()) doReturn mockRumMonitor
whenever(mockRumMonitor._getInternal()) doReturn mockRumInternalProxy

doNothing().whenever(mockChoreographer).postFrameCallback(any())
doNothing().whenever(mockChoreographerCompat).postFrameCallback(any())

mockChoreographerInstance(mockChoreographer)
mockChoreographerCompatInstance(mockChoreographerCompat)

whenever(mockReactContext.applicationContext) doReturn mockContext
whenever(mockContext.packageName) doReturn "packageName"
whenever(
Expand All @@ -145,7 +165,7 @@ internal class DdSdkTest {
answer.getArgument<Runnable>(0).run()
true
}
testedBridgeSdk = DdSdkImplementation(mockReactContext, mockDatadog)
testedBridgeSdk = DdSdkImplementation(mockReactContext, mockDatadog, TestUiThreadExecutor())

DatadogSDKWrapperStorage.setSdkCore(null)
DatadogSDKWrapperStorage.onInitializedListeners.clear()
Expand Down Expand Up @@ -1561,9 +1581,9 @@ internal class DdSdkTest {
.hasField("featureConfiguration") {
it.hasFieldEqualTo("vitalsMonitorUpdateFrequency", VitalsUpdateFrequency.RARE)
}
argumentCaptor<Choreographer.FrameCallback> {
verify(mockChoreographer).postFrameCallback(capture())
assertThat(firstValue).isInstanceOf(VitalFrameCallback::class.java)
argumentCaptor<ChoreographerCompat.FrameCallback> {
verify(mockChoreographerCompat).postFrameCallback(capture())
assertThat(firstValue).isInstanceOf(FpsFrameCallback::class.java)
}
}

Expand All @@ -1572,7 +1592,7 @@ internal class DdSdkTest {
@Forgery configuration: DdSdkConfiguration
) {
// Given
doThrow(IllegalStateException()).whenever(mockChoreographer).postFrameCallback(any())
doThrow(IllegalStateException()).whenever(mockChoreographerCompat).postFrameCallback(any())
val bridgeConfiguration = configuration.copy(
vitalsUpdateFrequency = "NEVER",
longTaskThresholdMs = 0.0
Expand Down Expand Up @@ -1600,7 +1620,7 @@ internal class DdSdkTest {
.hasField("featureConfiguration") {
it.hasFieldEqualTo("vitalsMonitorUpdateFrequency", VitalsUpdateFrequency.NEVER)
}
verifyNoInteractions(mockChoreographer)
verifyNoInteractions(mockChoreographerCompat)
}

@Test
Expand Down Expand Up @@ -1640,9 +1660,9 @@ internal class DdSdkTest {
.hasField("featureConfiguration") {
it.hasFieldEqualTo("vitalsMonitorUpdateFrequency", VitalsUpdateFrequency.AVERAGE)
}
argumentCaptor<Choreographer.FrameCallback> {
verify(mockChoreographer).postFrameCallback(capture())
assertThat(firstValue).isInstanceOf(VitalFrameCallback::class.java)
argumentCaptor<ChoreographerCompat.FrameCallback> {
verify(mockChoreographerCompat).postFrameCallback(capture())
assertThat(firstValue).isInstanceOf(FpsFrameCallback::class.java)

// When
firstValue.doFrame(timestampNs)
Expand Down Expand Up @@ -1678,8 +1698,8 @@ internal class DdSdkTest {
testedBridgeSdk.initialize(bridgeConfiguration.toReadableJavaOnlyMap(), mockPromise)

// Then
argumentCaptor<Choreographer.FrameCallback> {
verify(mockChoreographer).postFrameCallback(capture())
argumentCaptor<ChoreographerCompat.FrameCallback> {
verify(mockChoreographerCompat).postFrameCallback(capture())

// When
firstValue.doFrame(timestampNs)
Expand Down Expand Up @@ -1715,8 +1735,8 @@ internal class DdSdkTest {
testedBridgeSdk.initialize(bridgeConfiguration.toReadableJavaOnlyMap(), mockPromise)

// Then
argumentCaptor<Choreographer.FrameCallback> {
verify(mockChoreographer).postFrameCallback(capture())
argumentCaptor<ChoreographerCompat.FrameCallback> {
verify(mockChoreographerCompat).postFrameCallback(capture())

// When
firstValue.doFrame(timestampNs)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* 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/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.tools.unit

import com.datadog.reactnative.UiThreadExecutor

internal class TestUiThreadExecutor : UiThreadExecutor {
override fun runOnUiThread(runnable: Runnable) {
// Run immediately in the same thread for tests
runnable.run()
}
}
Loading