Skip to content

Commit

Permalink
Merge pull request #689 from DataDog/marcosaia/RUM-4720/js-fps-provider
Browse files Browse the repository at this point in the history
[RUM-4720] Improve JS Refresh Rate reliability on Android
  • Loading branch information
marco-saia-datadog authored Jul 17, 2024
2 parents 469e328 + ccfc9d8 commit 6d7b1a9
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 79 deletions.
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()
}
})

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

@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()
}
}

0 comments on commit 6d7b1a9

Please sign in to comment.