From be6f712cb508b32c51ed16f6147af28f928a751e Mon Sep 17 00:00:00 2001 From: Timur Valeev Date: Wed, 26 Feb 2025 17:38:53 +0000 Subject: [PATCH 1/4] RUM-8785: refactoring current jank stats logic - decoupling from FPS computing --- .../android/rum/internal/RumFeature.kt | 6 +- .../rum/internal/domain/FrameMetricsData.kt | 27 +++ .../rum/internal/vitals/FPSVitalListener.kt | 58 ++++++ .../vitals/FrameMetricsDataListener.kt | 12 ++ .../rum/internal/vitals/FrameStateListener.kt | 10 + .../JankStatsActivityLifecycleListener.kt | 94 +++++---- .../internal/vitals/FPSVitalListenerTest.kt | 161 ++++++++++++++++ .../JankStatsActivityLifecycleListenerTest.kt | 180 ++++++------------ .../TimeBasedInteractionIdentifierTest.kt | 2 +- .../android/rum/utils/forge/Configurator.kt | 2 + .../utils/forge/FrameDataForgeryFactory.kt | 22 +++ .../forge/FrameMetricDataForgeryFactory.kt | 32 ++++ 12 files changed, 430 insertions(+), 176 deletions(-) create mode 100644 features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/FrameMetricsData.kt create mode 100644 features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt create mode 100644 features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameMetricsDataListener.kt create mode 100644 features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameStateListener.kt create mode 100644 features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListenerTest.kt create mode 100644 features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/utils/forge/FrameDataForgeryFactory.kt create mode 100644 features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/utils/forge/FrameMetricDataForgeryFactory.kt diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt index 3f4edd047e..cb613fd8fa 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt @@ -60,6 +60,7 @@ import com.datadog.android.rum.internal.tracking.NoOpUserActionTrackingStrategy import com.datadog.android.rum.internal.tracking.UserActionTrackingStrategy import com.datadog.android.rum.internal.vitals.AggregatingVitalMonitor import com.datadog.android.rum.internal.vitals.CPUVitalReader +import com.datadog.android.rum.internal.vitals.FPSVitalListener import com.datadog.android.rum.internal.vitals.JankStatsActivityLifecycleListener import com.datadog.android.rum.internal.vitals.MemoryVitalReader import com.datadog.android.rum.internal.vitals.NoOpVitalMonitor @@ -416,7 +417,10 @@ internal class RumFeature( ) jankStatsActivityLifecycleListener = JankStatsActivityLifecycleListener( - frameRateVitalMonitor, + @Suppress("UnsafeThirdPartyFunctionCall") // it's safe to call listOf here + listOf( + FPSVitalListener(frameRateVitalMonitor) + ), sdkCore.internalLogger ) (appContext as? Application)?.registerActivityLifecycleCallbacks( diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/FrameMetricsData.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/FrameMetricsData.kt new file mode 100644 index 0000000000..ed7cd47b32 --- /dev/null +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/FrameMetricsData.kt @@ -0,0 +1,27 @@ +/* + * 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.android.rum.internal.domain + +import android.os.Build +import androidx.annotation.RequiresApi + +internal data class FrameMetricsData( + @RequiresApi(Build.VERSION_CODES.N) var unknownDelayDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var inputHandlingDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var animationDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var layoutMeasureDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var drawDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var syncDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var commandIssueDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var swapBuffersDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var totalDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.N) var firstDrawFrame: Boolean = false, + @RequiresApi(Build.VERSION_CODES.O) var intendedVsyncTimestamp: Long = 0L, + @RequiresApi(Build.VERSION_CODES.O) var vsyncTimestamp: Long = 0L, + @RequiresApi(Build.VERSION_CODES.S) var gpuDuration: Long = 0L, + @RequiresApi(Build.VERSION_CODES.S) var deadline: Long = 0L, + var displayRefreshRate: Double = 1.0 +) diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt new file mode 100644 index 0000000000..cbe4905e9e --- /dev/null +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt @@ -0,0 +1,58 @@ +/* + * 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.android.rum.internal.vitals + +import android.os.Build +import androidx.metrics.performance.FrameData +import com.datadog.android.core.internal.system.BuildSdkVersionProvider +import com.datadog.android.rum.internal.domain.FrameMetricsData +import java.util.concurrent.TimeUnit + +internal class FPSVitalListener( + private val vitalObserver: VitalObserver, + private val buildSdkVersionProvider: BuildSdkVersionProvider = BuildSdkVersionProvider.DEFAULT, + private var screenRefreshRate: Double = 60.0 +) : FrameStateListener { + private var frameDeadline = SIXTEEN_MS_NS + private var displayRefreshRate: Double = SIXTY_FPS + + override fun onFrame(volatileFrameData: FrameData) { + val durationNs = volatileFrameData.frameDurationUiNanos + if (durationNs > 0.0) { + var frameRate = (ONE_SECOND_NS / durationNs) + + if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S) { + screenRefreshRate = ONE_SECOND_NS / frameDeadline + } else if (buildSdkVersionProvider.version == Build.VERSION_CODES.R) { + screenRefreshRate = displayRefreshRate + } + + // If normalized frame rate is still at over 60fps it means the frame rendered + // quickly enough for the devices refresh rate. + frameRate = (frameRate * (SIXTY_FPS / screenRefreshRate)).coerceAtMost(MAX_FPS) + + if (frameRate > MIN_FPS) { + vitalObserver.onNewSample(frameRate) + } + } + } + + override fun onFrameMetricsData(data: FrameMetricsData) { + displayRefreshRate = data.displayRefreshRate + if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S) { + frameDeadline = data.deadline + } + } + + companion object { + private const val SIXTEEN_MS_NS: Long = 16666666 + private val ONE_SECOND_NS: Double = TimeUnit.SECONDS.toNanos(1).toDouble() + + private const val MIN_FPS: Double = 1.0 + private const val MAX_FPS: Double = 60.0 + private const val SIXTY_FPS: Double = 60.0 + } +} diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameMetricsDataListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameMetricsDataListener.kt new file mode 100644 index 0000000000..940b34e350 --- /dev/null +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameMetricsDataListener.kt @@ -0,0 +1,12 @@ +/* + * 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.android.rum.internal.vitals + +import com.datadog.android.rum.internal.domain.FrameMetricsData + +internal interface FrameMetricsDataListener { + fun onFrameMetricsData(data: FrameMetricsData) +} diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameStateListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameStateListener.kt new file mode 100644 index 0000000000..5a55fec83d --- /dev/null +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameStateListener.kt @@ -0,0 +1,10 @@ +/* + * 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.android.rum.internal.vitals + +import androidx.metrics.performance.JankStats + +internal interface FrameStateListener : JankStats.OnFrameListener, FrameMetricsDataListener diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt index f574693c90..77ed1d1fcc 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt @@ -24,18 +24,17 @@ import androidx.metrics.performance.FrameData import androidx.metrics.performance.JankStats import com.datadog.android.api.InternalLogger import com.datadog.android.core.internal.system.BuildSdkVersionProvider +import com.datadog.android.rum.internal.domain.FrameMetricsData import java.lang.ref.WeakReference import java.util.WeakHashMap -import java.util.concurrent.TimeUnit /** * Utility class listening to frame rate information. */ internal class JankStatsActivityLifecycleListener( - private val vitalObserver: VitalObserver, + private val delegates: List, private val internalLogger: InternalLogger, private val jankStatsProvider: JankStatsProvider = JankStatsProvider.DEFAULT, - private var screenRefreshRate: Double = 60.0, private var buildSdkVersionProvider: BuildSdkVersionProvider = BuildSdkVersionProvider.DEFAULT ) : ActivityLifecycleCallbacks, JankStats.OnFrameListener { @@ -44,7 +43,8 @@ internal class JankStatsActivityLifecycleListener( internal val activeActivities = WeakHashMap>>() internal var display: Display? = null private var frameMetricsListener: DDFrameMetricsListener? = null - internal var frameDeadline = SIXTEEN_MS_NS + + private val frameMetricsData = FrameMetricsData() // region ActivityLifecycleCallbacks @MainThread @@ -154,24 +154,7 @@ internal class JankStatsActivityLifecycleListener( // region JankStats.OnFrameListener override fun onFrame(volatileFrameData: FrameData) { - val durationNs = volatileFrameData.frameDurationUiNanos - if (durationNs > 0.0) { - var frameRate = (ONE_SECOND_NS / durationNs) - - if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S) { - screenRefreshRate = ONE_SECOND_NS / frameDeadline - } else if (buildSdkVersionProvider.version == Build.VERSION_CODES.R) { - screenRefreshRate = display?.refreshRate?.toDouble() ?: SIXTY_FPS - } - - // If normalized frame rate is still at over 60fps it means the frame rendered - // quickly enough for the devices refresh rate. - frameRate = (frameRate * (SIXTY_FPS / screenRefreshRate)).coerceAtMost(MAX_FPS) - - if (frameRate > MIN_FPS) { - vitalObserver.onNewSample(frameRate) - } - } + delegates.forEach { it.onFrame(volatileFrameData) } } // endregion @@ -233,30 +216,22 @@ internal class JankStatsActivityLifecycleListener( } val handler = Handler(Looper.getMainLooper()) // Only hardware accelerated views can be tracked with metrics listener - if (window.peekDecorView()?.isHardwareAccelerated == true) { - frameMetricsListener?.let { listener -> - try { - @Suppress("UnsafeThirdPartyFunctionCall") // Listener can't be null here - window.addOnFrameMetricsAvailableListener(listener, handler) - } catch (e: IllegalStateException) { - internalLogger.log( - InternalLogger.Level.ERROR, - InternalLogger.Target.MAINTAINER, - { "Unable to attach JankStatsListener to window" }, - e - ) - } + frameMetricsListener?.let { listener -> + try { + @Suppress("UnsafeThirdPartyFunctionCall") // Listener can't be null here + window.addOnFrameMetricsAvailableListener(listener, handler) + } catch (e: IllegalStateException) { + internalLogger.log( + InternalLogger.Level.ERROR, + InternalLogger.Target.MAINTAINER, + { "Unable to attach JankStatsListener to window" }, + e + ) } - } else { - internalLogger.log( - InternalLogger.Level.WARN, - InternalLogger.Target.MAINTAINER, - { "Unable to attach JankStatsListener to window, decorView is null or not hardware accelerated" } - ) } } - @RequiresApi(Build.VERSION_CODES.N) + @RequiresApi(Build.VERSION_CODES.S) private fun unregisterMetricListener(window: Window) { try { window.removeOnFrameMetricsAvailableListener(frameMetricsListener) @@ -279,7 +254,34 @@ internal class JankStatsActivityLifecycleListener( frameMetrics: FrameMetrics, dropCountSinceLastInvocation: Int ) { - frameDeadline = frameMetrics.getMetric(FrameMetrics.DEADLINE) + delegates.forEach { it.onFrameMetricsData(frameMetricsData.update(frameMetrics)) } + } + } + + @RequiresApi(Build.VERSION_CODES.N) + private fun FrameMetricsData.update(frameMetrics: FrameMetrics) = apply { + displayRefreshRate = display?.refreshRate?.toDouble() ?: SIXTY_FPS + if (buildSdkVersionProvider.version >= Build.VERSION_CODES.N) { + unknownDelayDuration = frameMetrics.getMetric(FrameMetrics.UNKNOWN_DELAY_DURATION) + inputHandlingDuration = frameMetrics.getMetric(FrameMetrics.INPUT_HANDLING_DURATION) + animationDuration = frameMetrics.getMetric(FrameMetrics.ANIMATION_DURATION) + layoutMeasureDuration = frameMetrics.getMetric(FrameMetrics.LAYOUT_MEASURE_DURATION) + drawDuration = frameMetrics.getMetric(FrameMetrics.DRAW_DURATION) + syncDuration = frameMetrics.getMetric(FrameMetrics.SYNC_DURATION) + commandIssueDuration = frameMetrics.getMetric(FrameMetrics.COMMAND_ISSUE_DURATION) + swapBuffersDuration = frameMetrics.getMetric(FrameMetrics.SWAP_BUFFERS_DURATION) + totalDuration = frameMetrics.getMetric(FrameMetrics.TOTAL_DURATION) + firstDrawFrame = frameMetrics.getMetric(FrameMetrics.FIRST_DRAW_FRAME) == 1L + } + @SuppressLint("InlinedApi") + if (buildSdkVersionProvider.version >= Build.VERSION_CODES.O) { + intendedVsyncTimestamp = frameMetrics.getMetric(FrameMetrics.INTENDED_VSYNC_TIMESTAMP) + vsyncTimestamp = frameMetrics.getMetric(FrameMetrics.VSYNC_TIMESTAMP) + } + @SuppressLint("InlinedApi") + if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S) { + gpuDuration = frameMetrics.getMetric(FrameMetrics.GPU_DURATION) + deadline = frameMetrics.getMetric(FrameMetrics.DEADLINE) } } @@ -292,12 +294,6 @@ internal class JankStatsActivityLifecycleListener( " shouldn't happen." internal const val JANK_STATS_TRACKING_DISABLE_ERROR = "Failed to disable JankStats tracking" - - private val ONE_SECOND_NS: Double = TimeUnit.SECONDS.toNanos(1).toDouble() - - private const val MIN_FPS: Double = 1.0 - private const val MAX_FPS: Double = 60.0 private const val SIXTY_FPS: Double = 60.0 - private const val SIXTEEN_MS_NS: Long = 16666666 } } diff --git a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListenerTest.kt b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListenerTest.kt new file mode 100644 index 0000000000..51a8c4a45d --- /dev/null +++ b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListenerTest.kt @@ -0,0 +1,161 @@ +/* + * 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.android.rum.internal.vitals + +import android.os.Build +import androidx.metrics.performance.FrameData +import com.datadog.android.api.InternalLogger +import com.datadog.android.core.internal.system.BuildSdkVersionProvider +import com.datadog.android.rum.internal.domain.FrameMetricsData +import com.datadog.android.rum.internal.vitals.JankStatsActivityLifecycleListenerTest.Companion.MAX_FPS +import com.datadog.android.rum.internal.vitals.JankStatsActivityLifecycleListenerTest.Companion.MIN_FPS +import com.datadog.android.rum.internal.vitals.JankStatsActivityLifecycleListenerTest.Companion.ONE_MILLISECOND_NS +import com.datadog.android.rum.internal.vitals.JankStatsActivityLifecycleListenerTest.Companion.ONE_SECOND_NS +import com.datadog.android.rum.utils.forge.Configurator +import com.datadog.tools.unit.extensions.TestConfigurationExtension +import fr.xgouchet.elmyr.annotation.BoolForgery +import fr.xgouchet.elmyr.annotation.DoubleForgery +import fr.xgouchet.elmyr.annotation.LongForgery +import fr.xgouchet.elmyr.junit5.ForgeConfiguration +import fr.xgouchet.elmyr.junit5.ForgeExtension +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import org.junit.jupiter.api.extension.Extensions +import org.mockito.AdditionalMatchers.eq +import org.mockito.Mock +import org.mockito.Mockito.never +import org.mockito.junit.jupiter.MockitoExtension +import org.mockito.junit.jupiter.MockitoSettings +import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.mockito.quality.Strictness +import kotlin.math.min + +@Extensions( + ExtendWith(MockitoExtension::class), + ExtendWith(ForgeExtension::class), + ExtendWith(TestConfigurationExtension::class) +) +@MockitoSettings(strictness = Strictness.LENIENT) +@ForgeConfiguration(Configurator::class) +internal class FPSVitalListenerTest { + + @Mock + lateinit var mockObserver: VitalObserver + + @Mock + lateinit var mockInternalLogger: InternalLogger + + private val mockBuildSdkVersionProvider: BuildSdkVersionProvider = mock { + on { version } doReturn Build.VERSION_CODES.VANILLA_ICE_CREAM + } + + private lateinit var listenerUnderTest: FPSVitalListener + + @BeforeEach + fun `set up`() { + listenerUnderTest = FPSVitalListener(mockObserver, mockBuildSdkVersionProvider) + } + + @Test + fun `M forward frame rate to observer W doFrame() {acceptable frame rate}`( + @LongForgery timestampNs: Long, + @LongForgery(ONE_MILLISECOND_NS, ONE_SECOND_NS) frameDurationNs: Long, + @BoolForgery isJank: Boolean + ) { + // Given + val expectedFrameRate = (ONE_SECOND_NS.toDouble() / frameDurationNs.toDouble()).coerceAtMost(MAX_FPS) + val frameData = FrameData(timestampNs, frameDurationNs, isJank, emptyList()) + + // When + listenerUnderTest.onFrame(frameData) + + // Then + verify(mockObserver).onNewSample(eq(expectedFrameRate, 0.0001)) + } + + fun `M do nothing W onFrame() {zero ns duration}`( + @LongForgery timestampNs: Long, + @BoolForgery isJank: Boolean + ) { + // Given + val frameData = FrameData(timestampNs, 0L, isJank, emptyList()) + + // When + listenerUnderTest.onFrame(frameData) + + // Then + verify(mockObserver, never()).onNewSample(any()) + } + + @Test + fun `M adjust sample value to refresh rate W doFrame() {S, refresh rate over 60hz}`( + @LongForgery timestampNs: Long, + @LongForgery(ONE_MILLISECOND_NS, ONE_SECOND_NS) frameDurationNs: Long, + @BoolForgery isJank: Boolean, + @DoubleForgery(60.0, 120.0) displayRefreshRate: Double + ) { + // Given + val expectedFrameRate = ONE_SECOND_NS.toDouble() / frameDurationNs.toDouble() + val refreshRateMultiplier = 60.0 / displayRefreshRate + + val frameData = FrameData(timestampNs, frameDurationNs, isJank, emptyList()) + + whenever(mockBuildSdkVersionProvider.version) doReturn Build.VERSION_CODES.S + + listenerUnderTest.onFrameMetricsData( + FrameMetricsData( + deadline = (ONE_SECOND_NS / displayRefreshRate).toLong() + ) + ) + + // When + listenerUnderTest.onFrame(frameData) + + // Then + if (expectedFrameRate * refreshRateMultiplier > MIN_FPS) { + verify(mockObserver).onNewSample(eq(min(expectedFrameRate * refreshRateMultiplier, MAX_FPS), 0.0001)) + } else { + verify(mockObserver, never()).onNewSample(any()) + } + } + + @Test + fun `M adjust sample value to refresh rate W doFrame() {R, refresh rate over 60hz}`( + @LongForgery timestampNs: Long, + @LongForgery(ONE_MILLISECOND_NS, ONE_SECOND_NS) frameDurationNs: Long, + @BoolForgery isJank: Boolean, + @DoubleForgery(60.0, 120.0) displayRefreshRate: Double + ) { + // Given + val expectedFrameRate = ONE_SECOND_NS.toDouble() / frameDurationNs.toDouble() + val refreshRateMultiplier = 60.0 / displayRefreshRate + + val frameData = FrameData(timestampNs, frameDurationNs, isJank, emptyList()) + + whenever(mockBuildSdkVersionProvider.version) doReturn Build.VERSION_CODES.R + + listenerUnderTest.onFrameMetricsData( + FrameMetricsData( + displayRefreshRate = displayRefreshRate + ) + ) + + // When + listenerUnderTest.onFrame(frameData) + + // Then + if (expectedFrameRate * refreshRateMultiplier > MIN_FPS) { + verify(mockObserver).onNewSample(eq(min(expectedFrameRate * refreshRateMultiplier, MAX_FPS), 0.0001)) + } else { + verify(mockObserver, never()).onNewSample(any()) + } + } +} diff --git a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt index 9d875d4230..823ce4d0d3 100644 --- a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt +++ b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt @@ -10,21 +10,21 @@ import android.app.Activity import android.os.Build import android.os.Bundle import android.view.Display +import android.view.FrameMetrics import android.view.View import android.view.Window import androidx.metrics.performance.FrameData import androidx.metrics.performance.JankStats import com.datadog.android.api.InternalLogger import com.datadog.android.core.internal.system.BuildSdkVersionProvider +import com.datadog.android.rum.internal.domain.FrameMetricsData import com.datadog.android.rum.utils.config.MainLooperTestConfiguration import com.datadog.android.rum.utils.forge.Configurator import com.datadog.android.rum.utils.verifyLog import com.datadog.tools.unit.annotations.TestConfigurationsProvider import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration -import fr.xgouchet.elmyr.annotation.BoolForgery -import fr.xgouchet.elmyr.annotation.DoubleForgery -import fr.xgouchet.elmyr.annotation.LongForgery +import fr.xgouchet.elmyr.Forge import fr.xgouchet.elmyr.junit5.ForgeConfiguration import fr.xgouchet.elmyr.junit5.ForgeExtension import org.assertj.core.api.Assertions.assertThat @@ -32,12 +32,11 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.api.extension.Extensions -import org.mockito.AdditionalMatchers.eq import org.mockito.Mock -import org.mockito.Mockito.never 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.doThrow import org.mockito.kotlin.inOrder @@ -47,7 +46,6 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever import org.mockito.quality.Strictness -import kotlin.math.min @Extensions( ExtendWith(MockitoExtension::class), @@ -58,10 +56,7 @@ import kotlin.math.min @ForgeConfiguration(Configurator::class) internal class JankStatsActivityLifecycleListenerTest { - lateinit var testedJankListener: JankStatsActivityLifecycleListener - - @Mock - lateinit var mockObserver: VitalObserver + private lateinit var testedJankListener: JankStatsActivityLifecycleListener @Mock lateinit var mockInternalLogger: InternalLogger @@ -69,6 +64,9 @@ internal class JankStatsActivityLifecycleListenerTest { @Mock lateinit var mockActivity: Activity + @Mock + lateinit var mockFPSVitalListener: FPSVitalListener + @Mock lateinit var mockDisplay: Display @@ -84,6 +82,10 @@ internal class JankStatsActivityLifecycleListenerTest { @Mock lateinit var mockJankStats: JankStats + private val mockBuildSdkVersionProvider: BuildSdkVersionProvider = mock { + on { version } doReturn Build.VERSION_CODES.VANILLA_ICE_CREAM + } + @BeforeEach fun `set up`() { whenever(mockWindow.decorView) doReturn mockDecorView @@ -94,9 +96,10 @@ internal class JankStatsActivityLifecycleListenerTest { whenever(mockJankStats.isTrackingEnabled) doReturn true testedJankListener = JankStatsActivityLifecycleListener( - mockObserver, + listOf(mockFPSVitalListener), mockInternalLogger, - mockJankStatsProvider + mockJankStatsProvider, + mockBuildSdkVersionProvider ) } @@ -130,7 +133,7 @@ internal class JankStatsActivityLifecycleListenerTest { whenever(mockActivity2.window) doReturn mockWindow whenever(mockActivity2.display) doReturn mockDisplay testedJankListener = JankStatsActivityLifecycleListener( - mockObserver, + listOf(mockFPSVitalListener), mockInternalLogger, mockJankStatsProvider ) @@ -280,15 +283,7 @@ internal class JankStatsActivityLifecycleListenerTest { fun `M add listener to window only once W onActivityStarted()`() { // Given whenever(mockDecorView.isHardwareAccelerated) doReturn true - val mockBuildSdkVersionProvider: BuildSdkVersionProvider = mock() whenever(mockBuildSdkVersionProvider.version) doReturn Build.VERSION_CODES.S - testedJankListener = JankStatsActivityLifecycleListener( - mockObserver, - mockInternalLogger, - mockJankStatsProvider, - 60.0, - mockBuildSdkVersionProvider - ) // When testedJankListener.onActivityStarted(mockActivity) @@ -350,143 +345,78 @@ internal class JankStatsActivityLifecycleListenerTest { verifyNoInteractions(mockJankStatsProvider, mockJankStats, mockBundle) } - fun `M do nothing W onFrame() {zero ns duration}`( - @LongForgery timestampNs: Long, - @BoolForgery isJank: Boolean - ) { - // Given - val frameData = FrameData(timestampNs, 0L, isJank, emptyList()) - - // When - testedJankListener.onFrame(frameData) - - // Then - verify(mockObserver, never()).onNewSample(any()) - } - @Test - fun `M forward frame rate to observer W doFrame() {acceptable frame rate}`( - @LongForgery timestampNs: Long, - @LongForgery(ONE_MILLISECOND_NS, ONE_SECOND_NS) frameDurationNs: Long, - @BoolForgery isJank: Boolean - ) { + fun `M do nothing W onActivityStarted() {android framework throws an exception}`() { // Given - val expectedFrameRate = (ONE_SECOND_NS.toDouble() / frameDurationNs.toDouble()).coerceAtMost(MAX_FPS) - val frameData = FrameData(timestampNs, frameDurationNs, isJank, emptyList()) + whenever(mockWindow.addOnFrameMetricsAvailableListener(any(), any())) doThrow IllegalStateException() // When - testedJankListener.onFrame(frameData) + testedJankListener.onActivityStarted(mockActivity) // Then - verify(mockObserver).onNewSample(eq(expectedFrameRate, 0.0001)) + // no-crash } @Test - fun `M adjust sample value to refresh rate W doFrame() {S, refresh rate over 60hz}`( - @LongForgery timestampNs: Long, - @LongForgery(ONE_MILLISECOND_NS, ONE_SECOND_NS) frameDurationNs: Long, - @BoolForgery isJank: Boolean, - @DoubleForgery(60.0, 120.0) displayRefreshRate: Double - ) { + fun `M do nothing W onActivityStarted() + onActivityDestroyed() {android framework throws an exception}`() { // Given - val expectedFrameRate = ONE_SECOND_NS.toDouble() / frameDurationNs.toDouble() - val refreshRateMultiplier = 60.0 / displayRefreshRate - - val frameData = FrameData(timestampNs, frameDurationNs, isJank, emptyList()) - - val mockBuildSdkVersionProvider: BuildSdkVersionProvider = mock() - whenever(mockBuildSdkVersionProvider.version) doReturn Build.VERSION_CODES.S - - val variableRefreshRateListener = JankStatsActivityLifecycleListener( - mockObserver, - mockInternalLogger, - mockJankStatsProvider, - displayRefreshRate, - mockBuildSdkVersionProvider - ) - variableRefreshRateListener.frameDeadline = (ONE_SECOND_NS / displayRefreshRate).toLong() + whenever(mockWindow.removeOnFrameMetricsAvailableListener(any())) doThrow IllegalArgumentException() // When - variableRefreshRateListener.onFrame(frameData) + testedJankListener.onActivityStarted(mockActivity) + testedJankListener.onActivityDestroyed(mockActivity) // Then - if (expectedFrameRate * refreshRateMultiplier > MIN_FPS) { - verify(mockObserver).onNewSample(eq(min(expectedFrameRate * refreshRateMultiplier, MAX_FPS), 0.0001)) - } else { - verify(mockObserver, never()).onNewSample(any()) - } + // no-crash } @Test - fun `M adjust sample value to refresh rate W doFrame() {R, refresh rate over 60hz}`( - @LongForgery timestampNs: Long, - @LongForgery(ONE_MILLISECOND_NS, ONE_SECOND_NS) frameDurationNs: Long, - @BoolForgery isJank: Boolean, - @DoubleForgery(60.0, 120.0) displayRefreshRate: Double - ) { - // Given - val expectedFrameRate = ONE_SECOND_NS.toDouble() / frameDurationNs.toDouble() - val refreshRateMultiplier = 60.0 / displayRefreshRate - - val frameData = FrameData(timestampNs, frameDurationNs, isJank, emptyList()) - - val mockBuildSdkVersionProvider: BuildSdkVersionProvider = mock() - whenever(mockBuildSdkVersionProvider.version) doReturn Build.VERSION_CODES.R - - val mockDisplay: Display = mock() - whenever(mockDisplay.refreshRate) doReturn displayRefreshRate.toFloat() - - val variableRefreshRateListener = JankStatsActivityLifecycleListener( - mockObserver, - mockInternalLogger, - mockJankStatsProvider, - displayRefreshRate, - mockBuildSdkVersionProvider - ) - - variableRefreshRateListener.display = mockDisplay + fun `M forward FrameData W onFrame`(forge: Forge) { + val frameData = forge.getForgery() - // When - variableRefreshRateListener.onFrame(frameData) + testedJankListener.onFrame(frameData) - // Then - if (expectedFrameRate * refreshRateMultiplier > MIN_FPS) { - verify(mockObserver).onNewSample(eq(min(expectedFrameRate * refreshRateMultiplier, MAX_FPS), 0.0001)) - } else { - verify(mockObserver, never()).onNewSample(any()) - } + verify(mockFPSVitalListener).onFrame(frameData) } @Test - fun `M do nothing W onActivityStarted() {android framework throws an exception}`() { + fun `M forward onFrameMetricsAvailable W onFrame`(forge: Forge) { // Given - whenever(mockWindow.addOnFrameMetricsAvailableListener(any(), any())) doThrow IllegalStateException() + val dropCountSinceLastInvocation = forge.aSmallInt() + val frameMetricsData = forge.getForgery() + + val frameMetrics = mock { + on { getMetric(FrameMetrics.UNKNOWN_DELAY_DURATION) } doReturn frameMetricsData.unknownDelayDuration + on { getMetric(FrameMetrics.INPUT_HANDLING_DURATION) } doReturn frameMetricsData.inputHandlingDuration + on { getMetric(FrameMetrics.ANIMATION_DURATION) } doReturn frameMetricsData.animationDuration + on { getMetric(FrameMetrics.LAYOUT_MEASURE_DURATION) } doReturn frameMetricsData.layoutMeasureDuration + on { getMetric(FrameMetrics.DRAW_DURATION) } doReturn frameMetricsData.drawDuration + on { getMetric(FrameMetrics.SYNC_DURATION) } doReturn frameMetricsData.syncDuration + on { getMetric(FrameMetrics.COMMAND_ISSUE_DURATION) } doReturn frameMetricsData.commandIssueDuration + on { getMetric(FrameMetrics.SWAP_BUFFERS_DURATION) } doReturn frameMetricsData.swapBuffersDuration + on { getMetric(FrameMetrics.TOTAL_DURATION) } doReturn frameMetricsData.totalDuration + on { getMetric(FrameMetrics.FIRST_DRAW_FRAME) } doReturn if (frameMetricsData.firstDrawFrame) 1 else 0 + on { getMetric(FrameMetrics.INTENDED_VSYNC_TIMESTAMP) } doReturn frameMetricsData.intendedVsyncTimestamp + on { getMetric(FrameMetrics.VSYNC_TIMESTAMP) } doReturn frameMetricsData.vsyncTimestamp + on { getMetric(FrameMetrics.GPU_DURATION) } doReturn frameMetricsData.gpuDuration + on { getMetric(FrameMetrics.DEADLINE) } doReturn frameMetricsData.deadline + } - // When testedJankListener.onActivityStarted(mockActivity) - // Then - // no-crash - } - - @Test - fun `M do nothing W onActivityStarted() + onActivityDestroyed() {android framework throws an exception}`() { - // Given - whenever(mockWindow.removeOnFrameMetricsAvailableListener(any())) doThrow IllegalArgumentException() - - // When - testedJankListener.onActivityStarted(mockActivity) - testedJankListener.onActivityDestroyed(mockActivity) + argumentCaptor { + verify(mockWindow).addOnFrameMetricsAvailableListener(capture(), any()) + // When + firstValue.onFrameMetricsAvailable(mock(), frameMetrics, dropCountSinceLastInvocation) + } // Then - // no-crash + verify(mockFPSVitalListener).onFrameMetricsData(frameMetricsData.copy(displayRefreshRate = 60.0)) } companion object { const val ONE_MILLISECOND_NS: Long = 1000L * 1000L const val ONE_SECOND_NS: Long = 1000L * 1000L * 1000L - const val TEN_SECOND_NS: Long = 10L * ONE_SECOND_NS - const val ONE_MINUTE_NS: Long = 60L * ONE_SECOND_NS const val MIN_FPS: Double = 1.0 const val MAX_FPS: Double = 60.0 diff --git a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/metric/interactiontonextview/TimeBasedInteractionIdentifierTest.kt b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/metric/interactiontonextview/TimeBasedInteractionIdentifierTest.kt index 23699969c1..2c38b779b1 100644 --- a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/metric/interactiontonextview/TimeBasedInteractionIdentifierTest.kt +++ b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/metric/interactiontonextview/TimeBasedInteractionIdentifierTest.kt @@ -132,7 +132,7 @@ internal class TimeBasedInteractionIdentifierTest : ObjectTest { + override fun getForgery(forge: Forge): FrameData { + return FrameData( + frameStartNanos = forge.aLong(min = 1), + frameDurationUiNanos = forge.aLong(min = 1), + isJank = forge.aBool(), + states = forge.aList { StateInfo(aString(), aString()) } + ) + } +} diff --git a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/utils/forge/FrameMetricDataForgeryFactory.kt b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/utils/forge/FrameMetricDataForgeryFactory.kt new file mode 100644 index 0000000000..b19a9920af --- /dev/null +++ b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/utils/forge/FrameMetricDataForgeryFactory.kt @@ -0,0 +1,32 @@ +/* + * 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.android.rum.utils.forge + +import com.datadog.android.rum.internal.domain.FrameMetricsData +import fr.xgouchet.elmyr.Forge +import fr.xgouchet.elmyr.ForgeryFactory + +internal class FrameMetricDataForgeryFactory : ForgeryFactory { + override fun getForgery(forge: Forge): FrameMetricsData { + return FrameMetricsData( + unknownDelayDuration = forge.aLong(min = 1), + inputHandlingDuration = forge.aLong(min = 1), + animationDuration = forge.aLong(min = 1), + layoutMeasureDuration = forge.aLong(min = 1), + drawDuration = forge.aLong(min = 1), + syncDuration = forge.aLong(min = 1), + commandIssueDuration = forge.aLong(min = 1), + swapBuffersDuration = forge.aLong(min = 1), + totalDuration = forge.aLong(min = 1), + firstDrawFrame = forge.aBool(), + intendedVsyncTimestamp = forge.aLong(min = 0), + vsyncTimestamp = forge.aLong(min = 0), + gpuDuration = forge.aLong(min = 1), + deadline = forge.aLong(min = 1), + displayRefreshRate = forge.aDouble(min = 1.0, max = 200.0) + ) + } +} From e317b45ee50fbffaefcd89c7f042672e2e499f89 Mon Sep 17 00:00:00 2001 From: Timur Valeev Date: Thu, 27 Feb 2025 16:05:13 +0000 Subject: [PATCH 2/4] RUM-8785: post-review fixes --- detekt_custom.yml | 2 + .../android/rum/internal/RumFeature.kt | 1 - .../rum/internal/domain/FrameMetricsData.kt | 8 ++- .../rum/internal/vitals/FPSVitalListener.kt | 4 +- .../JankStatsActivityLifecycleListener.kt | 56 ++++++++++++++----- .../internal/vitals/FPSVitalListenerTest.kt | 16 +++--- .../JankStatsActivityLifecycleListenerTest.kt | 27 +++++---- 7 files changed, 76 insertions(+), 38 deletions(-) diff --git a/detekt_custom.yml b/detekt_custom.yml index ebc07b31f5..fe0c33520e 100644 --- a/detekt_custom.yml +++ b/detekt_custom.yml @@ -424,6 +424,7 @@ datadog: - "android.view.View.getChildAt(kotlin.Int)" - "android.view.View.getTag(kotlin.Int)" - "android.view.View.hashCode()" + - "android.view.View.post(java.lang.Runnable?)" - "android.view.View.setTag(kotlin.Int, kotlin.Any?)" - "android.view.ViewGroup.findViewById(kotlin.Int)" - "android.view.ViewGroup.getChildAt(kotlin.Int)" @@ -1097,6 +1098,7 @@ datadog: - "kotlin.collections.emptySet()" - "kotlin.collections.listOf(android.view.Window)" - "kotlin.collections.listOf(com.datadog.android.api.InternalLogger.Target)" + - "kotlin.collections.listOf(com.datadog.android.rum.internal.vitals.FPSVitalListener)" - "kotlin.collections.listOf(com.datadog.android.rum.model.ActionEvent.Interface)" - "kotlin.collections.listOf(com.datadog.android.rum.model.ErrorEvent.Interface)" - "kotlin.collections.listOf(com.datadog.android.rum.model.LongTaskEvent.Interface)" diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt index cb613fd8fa..4a8360f3f7 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt @@ -417,7 +417,6 @@ internal class RumFeature( ) jankStatsActivityLifecycleListener = JankStatsActivityLifecycleListener( - @Suppress("UnsafeThirdPartyFunctionCall") // it's safe to call listOf here listOf( FPSVitalListener(frameRateVitalMonitor) ), diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/FrameMetricsData.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/FrameMetricsData.kt index ed7cd47b32..55791a5708 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/FrameMetricsData.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/FrameMetricsData.kt @@ -23,5 +23,9 @@ internal data class FrameMetricsData( @RequiresApi(Build.VERSION_CODES.O) var vsyncTimestamp: Long = 0L, @RequiresApi(Build.VERSION_CODES.S) var gpuDuration: Long = 0L, @RequiresApi(Build.VERSION_CODES.S) var deadline: Long = 0L, - var displayRefreshRate: Double = 1.0 -) + var displayRefreshRate: Double = SIXTY_FPS +) { + companion object { + private const val SIXTY_FPS: Double = 60.0 + } +} diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt index cbe4905e9e..d1f06de9de 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt @@ -16,7 +16,7 @@ internal class FPSVitalListener( private val buildSdkVersionProvider: BuildSdkVersionProvider = BuildSdkVersionProvider.DEFAULT, private var screenRefreshRate: Double = 60.0 ) : FrameStateListener { - private var frameDeadline = SIXTEEN_MS_NS + private var frameDeadline = EXPECTED_60_FPS_FRAME_DURATION_NS private var displayRefreshRate: Double = SIXTY_FPS override fun onFrame(volatileFrameData: FrameData) { @@ -48,7 +48,7 @@ internal class FPSVitalListener( } companion object { - private const val SIXTEEN_MS_NS: Long = 16666666 + private const val EXPECTED_60_FPS_FRAME_DURATION_NS: Long = 16_666_666L private val ONE_SECOND_NS: Double = TimeUnit.SECONDS.toNanos(1).toDouble() private const val MIN_FPS: Double = 1.0 diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt index 77ed1d1fcc..b78420ed2b 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt @@ -143,7 +143,7 @@ internal class JankStatsActivityLifecycleListener( if (activeActivities[activity.window].isNullOrEmpty()) { activeWindowsListener.remove(activity.window) activeActivities.remove(activity.window) - if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S) { + if (buildSdkVersionProvider.version >= Build.VERSION_CODES.N) { unregisterMetricListener(activity.window) } } @@ -199,7 +199,7 @@ internal class JankStatsActivityLifecycleListener( @SuppressLint("NewApi") @MainThread private fun trackWindowMetrics(isKnownWindow: Boolean, window: Window, activity: Activity) { - if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S && !isKnownWindow) { + if (buildSdkVersionProvider.version >= Build.VERSION_CODES.N && !isKnownWindow) { registerMetricListener(window) } else if (display == null && buildSdkVersionProvider.version == Build.VERSION_CODES.R) { // Fallback - Android 30 allows apps to not run at a fixed 60hz, but didn't yet have @@ -209,29 +209,54 @@ internal class JankStatsActivityLifecycleListener( } } - @RequiresApi(Build.VERSION_CODES.S) + @RequiresApi(Build.VERSION_CODES.N) private fun registerMetricListener(window: Window) { if (frameMetricsListener == null) { frameMetricsListener = DDFrameMetricsListener() } + // todo RUM-8799: handler thread can be used instead val handler = Handler(Looper.getMainLooper()) - // Only hardware accelerated views can be tracked with metrics listener - frameMetricsListener?.let { listener -> - try { - @Suppress("UnsafeThirdPartyFunctionCall") // Listener can't be null here - window.addOnFrameMetricsAvailableListener(listener, handler) - } catch (e: IllegalStateException) { + val decorView = window.peekDecorView() + + if (decorView == null) { + internalLogger.log( + InternalLogger.Level.WARN, + InternalLogger.Target.MAINTAINER, + { "Unable to attach JankStatsListener to window, decorView is null" } + ) + return + } + + // We need to postpone this operation because isHardwareAccelerated will return + // false until the view is attached to the window. Note that in this case main looper should be used + decorView.post { + // Only hardware accelerated views can be tracked with metrics listener + if (!decorView.isHardwareAccelerated) { internalLogger.log( - InternalLogger.Level.ERROR, + InternalLogger.Level.WARN, InternalLogger.Target.MAINTAINER, - { "Unable to attach JankStatsListener to window" }, - e + { "Unable to attach JankStatsListener to window, decorView is not hardware accelerated" } ) + return@post + } + + frameMetricsListener?.let { listener -> + try { + @Suppress("UnsafeThirdPartyFunctionCall") // Listener can't be null here + window.addOnFrameMetricsAvailableListener(listener, handler) + } catch (e: IllegalStateException) { + internalLogger.log( + InternalLogger.Level.ERROR, + InternalLogger.Target.MAINTAINER, + { "Unable to attach JankStatsListener to window" }, + e + ) + } } } } - @RequiresApi(Build.VERSION_CODES.S) + @RequiresApi(Build.VERSION_CODES.N) private fun unregisterMetricListener(window: Window) { try { window.removeOnFrameMetricsAvailableListener(frameMetricsListener) @@ -248,7 +273,7 @@ internal class JankStatsActivityLifecycleListener( @RequiresApi(Build.VERSION_CODES.N) inner class DDFrameMetricsListener : Window.OnFrameMetricsAvailableListener { - @RequiresApi(Build.VERSION_CODES.S) + @RequiresApi(Build.VERSION_CODES.N) override fun onFrameMetricsAvailable( window: Window, frameMetrics: FrameMetrics, @@ -271,7 +296,7 @@ internal class JankStatsActivityLifecycleListener( commandIssueDuration = frameMetrics.getMetric(FrameMetrics.COMMAND_ISSUE_DURATION) swapBuffersDuration = frameMetrics.getMetric(FrameMetrics.SWAP_BUFFERS_DURATION) totalDuration = frameMetrics.getMetric(FrameMetrics.TOTAL_DURATION) - firstDrawFrame = frameMetrics.getMetric(FrameMetrics.FIRST_DRAW_FRAME) == 1L + firstDrawFrame = frameMetrics.getMetric(FrameMetrics.FIRST_DRAW_FRAME) == TRUE } @SuppressLint("InlinedApi") if (buildSdkVersionProvider.version >= Build.VERSION_CODES.O) { @@ -295,5 +320,6 @@ internal class JankStatsActivityLifecycleListener( internal const val JANK_STATS_TRACKING_DISABLE_ERROR = "Failed to disable JankStats tracking" private const val SIXTY_FPS: Double = 60.0 + private const val TRUE = 1L } } diff --git a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListenerTest.kt b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListenerTest.kt index 51a8c4a45d..b2312df982 100644 --- a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListenerTest.kt +++ b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListenerTest.kt @@ -57,11 +57,11 @@ internal class FPSVitalListenerTest { on { version } doReturn Build.VERSION_CODES.VANILLA_ICE_CREAM } - private lateinit var listenerUnderTest: FPSVitalListener + private lateinit var testedListener: FPSVitalListener @BeforeEach fun `set up`() { - listenerUnderTest = FPSVitalListener(mockObserver, mockBuildSdkVersionProvider) + testedListener = FPSVitalListener(mockObserver, mockBuildSdkVersionProvider) } @Test @@ -75,7 +75,7 @@ internal class FPSVitalListenerTest { val frameData = FrameData(timestampNs, frameDurationNs, isJank, emptyList()) // When - listenerUnderTest.onFrame(frameData) + testedListener.onFrame(frameData) // Then verify(mockObserver).onNewSample(eq(expectedFrameRate, 0.0001)) @@ -89,7 +89,7 @@ internal class FPSVitalListenerTest { val frameData = FrameData(timestampNs, 0L, isJank, emptyList()) // When - listenerUnderTest.onFrame(frameData) + testedListener.onFrame(frameData) // Then verify(mockObserver, never()).onNewSample(any()) @@ -110,14 +110,14 @@ internal class FPSVitalListenerTest { whenever(mockBuildSdkVersionProvider.version) doReturn Build.VERSION_CODES.S - listenerUnderTest.onFrameMetricsData( + testedListener.onFrameMetricsData( FrameMetricsData( deadline = (ONE_SECOND_NS / displayRefreshRate).toLong() ) ) // When - listenerUnderTest.onFrame(frameData) + testedListener.onFrame(frameData) // Then if (expectedFrameRate * refreshRateMultiplier > MIN_FPS) { @@ -142,14 +142,14 @@ internal class FPSVitalListenerTest { whenever(mockBuildSdkVersionProvider.version) doReturn Build.VERSION_CODES.R - listenerUnderTest.onFrameMetricsData( + testedListener.onFrameMetricsData( FrameMetricsData( displayRefreshRate = displayRefreshRate ) ) // When - listenerUnderTest.onFrame(frameData) + testedListener.onFrame(frameData) // Then if (expectedFrameRate * refreshRateMultiplier > MIN_FPS) { diff --git a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt index 823ce4d0d3..112977be65 100644 --- a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt +++ b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt @@ -30,6 +30,7 @@ import fr.xgouchet.elmyr.junit5.ForgeExtension import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertDoesNotThrow import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.api.extension.Extensions import org.mockito.Mock @@ -289,6 +290,10 @@ internal class JankStatsActivityLifecycleListenerTest { testedJankListener.onActivityStarted(mockActivity) testedJankListener.onActivityStopped(mockActivity) testedJankListener.onActivityStarted(mockActivity) + argumentCaptor { + verify(mockDecorView).post(capture()) + lastValue.run() + } // Then verify(mockWindow).addOnFrameMetricsAvailableListener(any(), any()) // should be called only once @@ -351,10 +356,9 @@ internal class JankStatsActivityLifecycleListenerTest { whenever(mockWindow.addOnFrameMetricsAvailableListener(any(), any())) doThrow IllegalStateException() // When - testedJankListener.onActivityStarted(mockActivity) - - // Then - // no-crash + assertDoesNotThrow { + testedJankListener.onActivityStarted(mockActivity) + } } @Test @@ -363,11 +367,10 @@ internal class JankStatsActivityLifecycleListenerTest { whenever(mockWindow.removeOnFrameMetricsAvailableListener(any())) doThrow IllegalArgumentException() // When - testedJankListener.onActivityStarted(mockActivity) - testedJankListener.onActivityDestroyed(mockActivity) - - // Then - // no-crash + assertDoesNotThrow { + testedJankListener.onActivityStarted(mockActivity) + testedJankListener.onActivityDestroyed(mockActivity) + } } @Test @@ -384,7 +387,6 @@ internal class JankStatsActivityLifecycleListenerTest { // Given val dropCountSinceLastInvocation = forge.aSmallInt() val frameMetricsData = forge.getForgery() - val frameMetrics = mock { on { getMetric(FrameMetrics.UNKNOWN_DELAY_DURATION) } doReturn frameMetricsData.unknownDelayDuration on { getMetric(FrameMetrics.INPUT_HANDLING_DURATION) } doReturn frameMetricsData.inputHandlingDuration @@ -401,8 +403,13 @@ internal class JankStatsActivityLifecycleListenerTest { on { getMetric(FrameMetrics.GPU_DURATION) } doReturn frameMetricsData.gpuDuration on { getMetric(FrameMetrics.DEADLINE) } doReturn frameMetricsData.deadline } + whenever(mockDecorView.isHardwareAccelerated) doReturn true testedJankListener.onActivityStarted(mockActivity) + argumentCaptor { + verify(mockDecorView).post(capture()) + firstValue.run() + } argumentCaptor { verify(mockWindow).addOnFrameMetricsAvailableListener(capture(), any()) From 474b942829d6df71a2197bbac15ede80e8f6b016 Mon Sep 17 00:00:00 2001 From: Timur Valeev Date: Fri, 28 Feb 2025 11:57:27 +0000 Subject: [PATCH 3/4] RUM-8785: post-review fixes --- .../datadog/android/rum/internal/vitals/FPSVitalListener.kt | 5 +++++ .../internal/vitals/JankStatsActivityLifecycleListener.kt | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt index d1f06de9de..c6af1ce176 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt @@ -5,7 +5,9 @@ */ package com.datadog.android.rum.internal.vitals +import android.annotation.SuppressLint import android.os.Build +import androidx.annotation.RequiresApi import androidx.metrics.performance.FrameData import com.datadog.android.core.internal.system.BuildSdkVersionProvider import com.datadog.android.rum.internal.domain.FrameMetricsData @@ -16,6 +18,7 @@ internal class FPSVitalListener( private val buildSdkVersionProvider: BuildSdkVersionProvider = BuildSdkVersionProvider.DEFAULT, private var screenRefreshRate: Double = 60.0 ) : FrameStateListener { + @RequiresApi(Build.VERSION_CODES.S) private var frameDeadline = EXPECTED_60_FPS_FRAME_DURATION_NS private var displayRefreshRate: Double = SIXTY_FPS @@ -24,6 +27,7 @@ internal class FPSVitalListener( if (durationNs > 0.0) { var frameRate = (ONE_SECOND_NS / durationNs) + @SuppressLint("NewApi") if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S) { screenRefreshRate = ONE_SECOND_NS / frameDeadline } else if (buildSdkVersionProvider.version == Build.VERSION_CODES.R) { @@ -42,6 +46,7 @@ internal class FPSVitalListener( override fun onFrameMetricsData(data: FrameMetricsData) { displayRefreshRate = data.displayRefreshRate + @SuppressLint("NewApi") if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S) { frameDeadline = data.deadline } diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt index b78420ed2b..e25a6658bb 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt @@ -214,7 +214,7 @@ internal class JankStatsActivityLifecycleListener( if (frameMetricsListener == null) { frameMetricsListener = DDFrameMetricsListener() } - // todo RUM-8799: handler thread can be used instead + // TODO RUM-8799: handler thread can be used instead val handler = Handler(Looper.getMainLooper()) val decorView = window.peekDecorView() @@ -296,7 +296,7 @@ internal class JankStatsActivityLifecycleListener( commandIssueDuration = frameMetrics.getMetric(FrameMetrics.COMMAND_ISSUE_DURATION) swapBuffersDuration = frameMetrics.getMetric(FrameMetrics.SWAP_BUFFERS_DURATION) totalDuration = frameMetrics.getMetric(FrameMetrics.TOTAL_DURATION) - firstDrawFrame = frameMetrics.getMetric(FrameMetrics.FIRST_DRAW_FRAME) == TRUE + firstDrawFrame = frameMetrics.getMetric(FrameMetrics.FIRST_DRAW_FRAME) == IS_FIRST_DRAW_FRAME } @SuppressLint("InlinedApi") if (buildSdkVersionProvider.version >= Build.VERSION_CODES.O) { @@ -320,6 +320,6 @@ internal class JankStatsActivityLifecycleListener( internal const val JANK_STATS_TRACKING_DISABLE_ERROR = "Failed to disable JankStats tracking" private const val SIXTY_FPS: Double = 60.0 - private const val TRUE = 1L + private const val IS_FIRST_DRAW_FRAME = 1L } } From a03d8e85a00cdc74bf606d73b927a40ec0b34642 Mon Sep 17 00:00:00 2001 From: Timur Valeev Date: Fri, 28 Feb 2025 12:00:00 +0000 Subject: [PATCH 4/4] RUM-8785: reducing gc pressure or 'hot' methods --- .../internal/vitals/JankStatsActivityLifecycleListener.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt index e25a6658bb..4be2516258 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt @@ -154,7 +154,9 @@ internal class JankStatsActivityLifecycleListener( // region JankStats.OnFrameListener override fun onFrame(volatileFrameData: FrameData) { - delegates.forEach { it.onFrame(volatileFrameData) } + for (i in delegates.indices) { + delegates[i].onFrame(volatileFrameData) + } } // endregion @@ -279,7 +281,9 @@ internal class JankStatsActivityLifecycleListener( frameMetrics: FrameMetrics, dropCountSinceLastInvocation: Int ) { - delegates.forEach { it.onFrameMetricsData(frameMetricsData.update(frameMetrics)) } + for (i in delegates.indices) { + delegates[i].onFrameMetricsData(frameMetricsData.update(frameMetrics)) + } } }