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.

Sign up for GitHub

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

RUMM-3356 fix jankstats #1512

Merged
merged 3 commits into from
Jul 3, 2023
Merged

RUMM-3356 fix jankstats #1512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85038b5
RUMM-3356 update the JankStats usage to report refresh rate metrics
xgouchet Jun 29, 2023
492cbe8
RUMM-3356 create a custom view to illustrate slow framerate
xgouchet Jun 29, 2023
d22aeed
Use WeakHashMap to store window keys
xgouchet Jul 3, 2023
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
2 changes: 0 additions & 2 deletions dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import com.datadog.android.core.configuration.VitalsUpdateFrequency
import com.datadog.android.core.internal.CoreFeature
import com.datadog.android.core.internal.event.NoOpEventMapper
import com.datadog.android.core.internal.persistence.file.batch.BatchFileReaderWriter
import com.datadog.android.core.internal.system.DefaultBuildSdkVersionProvider
import com.datadog.android.core.internal.thread.LoggingScheduledThreadPoolExecutor
import com.datadog.android.core.internal.thread.NoOpScheduledExecutorService
import com.datadog.android.core.internal.utils.executeSafe
Expand Down Expand Up @@ -238,7 +237,6 @@ internal class RumFeature(
initializeVitalMonitor(MemoryVitalReader(), memoryVitalMonitor, periodInMs)
jankStatsActivityLifecycleListener = JankStatsActivityLifecycleListener(
frameRateVitalMonitor,
DefaultBuildSdkVersionProvider(),
internalLogger
)
(appContext as? Application)?.registerActivityLifecycleCallbacks(
Expand Down
101 changes: 69 additions & 32 deletions ...main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,65 @@

package com.datadog.android.rum.internal.vitals

import android.annotation.SuppressLint
import android.app.Activity
import android.app.Application.ActivityLifecycleCallbacks
import android.content.Context
import android.os.Build
import android.os.Bundle
import android.view.WindowManager
import android.view.Window
import androidx.annotation.MainThread
import androidx.metrics.performance.FrameData
import androidx.metrics.performance.JankStats
import com.datadog.android.core.internal.system.BuildSdkVersionProvider
import com.datadog.android.v2.api.InternalLogger
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 buildSdkVersionProvider: BuildSdkVersionProvider,
private val internalLogger: InternalLogger
private val internalLogger: InternalLogger,
private val jankStatsProvider: JankStatsProvider = JankStatsProvider.DEFAULT
) : ActivityLifecycleCallbacks, JankStats.OnFrameListener {

private var displayRefreshRateScale: Float = 1.0f
private val activeWindowsListener = WeakHashMap<Window, JankStats>()
private val activeActivities = WeakHashMap<Window, MutableList<WeakReference<Activity>>>()

// region ActivityLifecycleCallbacks

@Suppress("DEPRECATION")
@SuppressLint("NewApi")
@MainThread
override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {
try {
val display = if (buildSdkVersionProvider.version() >= Build.VERSION_CODES.R) {
activity.display
} else {
(activity.getSystemService(Context.WINDOW_SERVICE) as? WindowManager)?.defaultDisplay
}
val displayRefreshRate = display?.refreshRate ?: STANDARD_FPS.toFloat()
displayRefreshRateScale = STANDARD_FPS / displayRefreshRate
JankStats.createAndTrack(activity.window, this)
} catch (e: IllegalStateException) {
internalLogger.log(
InternalLogger.Level.ERROR,
InternalLogger.Target.MAINTAINER,
"Unable to attach JankStats to the current window.",
e
)
}
}

@MainThread
override fun onActivityStarted(activity: Activity) {
val window = activity.window
trackActivity(window, activity)

val knownJankStats = activeWindowsListener[window]
if (knownJankStats != null) {
internalLogger.log(
InternalLogger.Level.DEBUG,
InternalLogger.Target.MAINTAINER,
"Resuming jankStats for window $window"
)
knownJankStats.isTrackingEnabled = true
} else {
internalLogger.log(
InternalLogger.Level.DEBUG,
InternalLogger.Target.MAINTAINER,
"Starting jankStats for window $window"
)
val jankStats = jankStatsProvider.createJankStatsAndTrack(window, this, internalLogger)
if (jankStats == null) {
internalLogger.log(
InternalLogger.Level.WARN,
InternalLogger.Target.MAINTAINER,
"Unable to create JankStats"
)
} else {
activeWindowsListener[window] = jankStats
}
}
}

@MainThread
Expand All @@ -70,6 +77,27 @@ internal class JankStatsActivityLifecycleListener(

@MainThread
override fun onActivityStopped(activity: Activity) {
val window = activity.window
if (!activeActivities.containsKey(window)) {
internalLogger.log(
InternalLogger.Level.WARN,
InternalLogger.Target.MAINTAINER,
"Activity stopped but window was not tracked"
)
}
val list = activeActivities[window] ?: mutableListOf()
list.removeAll {
it.get() == null || it.get() == activity
}
activeActivities[window] = list
if (list.isEmpty()) {
internalLogger.log(
InternalLogger.Level.DEBUG,
InternalLogger.Target.MAINTAINER,
"Disabling jankStats for window $window"
)
activeWindowsListener[window]?.isTrackingEnabled = false
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we should use WeakHashMap for activeActivities and activeWindowsListener, in case if the window was temporary and will be destroyed once a particular activity is destroyed.

I can see that PolicyManager.makeNewWindow can be called from different places, Dialog constructor included. So if there is a dialog shown and then destroyed, we will be keeping Window instance in the memory forever, preventing it from be GCed.

}
}

@MainThread
Expand All @@ -87,7 +115,7 @@ internal class JankStatsActivityLifecycleListener(
override fun onFrame(volatileFrameData: FrameData) {
val durationNs = volatileFrameData.frameDurationUiNanos
if (durationNs > 0.0) {
val frameRate = (ONE_SECOND_NS / durationNs) * displayRefreshRateScale
val frameRate = (ONE_SECOND_NS / durationNs)
if (frameRate in VALID_FPS_RANGE) {
vitalObserver.onNewSample(frameRate)
}
Expand All @@ -96,12 +124,21 @@ internal class JankStatsActivityLifecycleListener(

// endregion

// region Internal

private fun trackActivity(window: Window, activity: Activity) {
val list = activeActivities[window] ?: mutableListOf()
list.add(WeakReference(activity))
activeActivities[window] = list
}

// endregion

companion object {
val ONE_SECOND_NS: Double = TimeUnit.SECONDS.toNanos(1).toDouble()
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 = 240.0
private const val STANDARD_FPS: Float = 60.0f
val VALID_FPS_RANGE = MIN_FPS.rangeTo(MAX_FPS)
private val VALID_FPS_RANGE = MIN_FPS.rangeTo(MAX_FPS)
}
}
46 changes: 46 additions & 0 deletions dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* 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.view.Window
import androidx.annotation.UiThread
import androidx.metrics.performance.JankStats
import com.datadog.android.v2.api.InternalLogger

internal interface JankStatsProvider {

@UiThread
fun createJankStatsAndTrack(
window: Window,
listener: JankStats.OnFrameListener,
internalLogger: InternalLogger
): JankStats?

companion object {

val DEFAULT = object : JankStatsProvider {
@UiThread
override fun createJankStatsAndTrack(
window: Window,
listener: JankStats.OnFrameListener,
internalLogger: InternalLogger
): JankStats? {
return try {
JankStats.createAndTrack(window, listener)
} catch (e: IllegalStateException) {
internalLogger.log(
InternalLogger.Level.ERROR,
InternalLogger.Target.MAINTAINER,
"Unable to attach JankStats to the current window",
e
)
null
}
}
}
}
}
Loading