Merge f6b0dc7 into 76d3f4a

getsentry · Oct 3, 2022 · 001064a · 001064a
2 parents 76d3f4a + f6b0dc7
commit 001064a
Show file tree

Hide file tree

Showing 3 changed files with 272 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- Fix slow and frozen frames tracking ([#2271](https://github.com/getsentry/sentry-java/pull/2271))
+
 ## 6.4.2
 
 ### Fixes

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityFramesTracker.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityFramesTracker.java
@@ -10,6 +10,7 @@
 import io.sentry.protocol.SentryId;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.WeakHashMap;
 import java.util.concurrent.ConcurrentHashMap;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -27,6 +28,8 @@ public final class ActivityFramesTracker {
 
   private final @NotNull Map<SentryId, Map<String, @NotNull MeasurementValue>>
       activityMeasurements = new ConcurrentHashMap<>();
+  private final @NotNull Map<Activity, FrameCounts> frameCountAtStartSnapshots =
+      new WeakHashMap<>();
 
   public ActivityFramesTracker(final @NotNull LoadClass loadClass, final @Nullable ILogger logger) {
     androidXAvailable =
@@ -55,33 +58,50 @@ public synchronized void addActivity(final @NotNull Activity activity) {
       return;
     }
     frameMetricsAggregator.add(activity);
+    snapshotFrameCountsAtStart(activity);
   }
 
-  @SuppressWarnings("NullAway")
-  public synchronized void setMetrics(
-      final @NotNull Activity activity, final @NotNull SentryId sentryId) {
+  private void snapshotFrameCountsAtStart(final @NotNull Activity activity) {
+    FrameCounts frameCounts = calculateCurrentFrameCounts();
+    if (frameCounts != null) {
+      frameCountAtStartSnapshots.put(activity, frameCounts);
+    }
+  }
+
+  private @Nullable FrameCounts diffFrameCountsAtEnd(final @NotNull Activity activity) {
+    @Nullable
+    final FrameCounts frameCountsAtStartFromMap = frameCountAtStartSnapshots.remove(activity);
+    @NotNull
+    final FrameCounts frameCountsAtStart =
+        frameCountsAtStartFromMap == null ? new FrameCounts(0, 0, 0) : frameCountsAtStartFromMap;
+
+    @Nullable final FrameCounts frameCountsAtEnd = calculateCurrentFrameCounts();
+    if (frameCountsAtEnd == null) {
+      return null;
+    }
+
+    final int diffTotalFrames = frameCountsAtEnd.totalFrames - frameCountsAtStart.totalFrames;
+    final int diffSlowFrames = frameCountsAtEnd.slowFrames - frameCountsAtStart.slowFrames;
+    final int diffFrozenFrames = frameCountsAtEnd.frozenFrames - frameCountsAtStart.frozenFrames;
+
+    return new FrameCounts(diffTotalFrames, diffSlowFrames, diffFrozenFrames);
+  }
+
+  private @Nullable FrameCounts calculateCurrentFrameCounts() {
     if (!isFrameMetricsAggregatorAvailable()) {
-      return;
+      return null;
+    }
+
+    if (frameMetricsAggregator == null) {
+      return null;
     }
+    final @Nullable SparseIntArray[] framesRates = frameMetricsAggregator.getMetrics();
 
     int totalFrames = 0;
     int slowFrames = 0;
     int frozenFrames = 0;
 
-    SparseIntArray[] framesRates = null;
-    try {
-      framesRates = frameMetricsAggregator.remove(activity);
-    } catch (Throwable ignored) {
-      // throws IllegalArgumentException when attempting to remove OnFrameMetricsAvailableListener
-      // that was never added.
-      // there's no contains method.
-      // throws NullPointerException when attempting to remove OnFrameMetricsAvailableListener and
-      // there was no
-      // Observers, See
-      // https://android.googlesource.com/platform/frameworks/base/+/140ff5ea8e2d99edc3fbe63a43239e459334c76b
-    }
-
-    if (framesRates != null) {
+    if (framesRates != null && framesRates.length > 0) {
       final SparseIntArray totalIndexArray = framesRates[FrameMetricsAggregator.TOTAL_INDEX];
       if (totalIndexArray != null) {
         for (int i = 0; i < totalIndexArray.size(); i++) {
@@ -100,13 +120,53 @@ public synchronized void setMetrics(
       }
     }
 
-    if (totalFrames == 0 && slowFrames == 0 && frozenFrames == 0) {
+    return new FrameCounts(totalFrames, slowFrames, frozenFrames);
+  }
+
+  private static final class FrameCounts {
+    private final int totalFrames;
+    private final int slowFrames;
+    private final int frozenFrames;
+
+    private FrameCounts(final int totalFrames, final int slowFrames, final int frozenFrames) {
+      this.totalFrames = totalFrames;
+      this.slowFrames = slowFrames;
+      this.frozenFrames = frozenFrames;
+    }
+  }
+
+  @SuppressWarnings("NullAway")
+  public synchronized void setMetrics(
+      final @NotNull Activity activity, final @NotNull SentryId sentryId) {
+    if (!isFrameMetricsAggregatorAvailable()) {
+      return;
+    }
+
+    try {
+      // NOTE: removing an activity does not reset the frame counts, only reset() does
+      frameMetricsAggregator.remove(activity);
+    } catch (Throwable ignored) {
+      // throws IllegalArgumentException when attempting to remove OnFrameMetricsAvailableListener
+      // that was never added.
+      // there's no contains method.
+      // throws NullPointerException when attempting to remove OnFrameMetricsAvailableListener and
+      // there was no
+      // Observers, See
+      // https://android.googlesource.com/platform/frameworks/base/+/140ff5ea8e2d99edc3fbe63a43239e459334c76b
+    }
+
+    final @Nullable FrameCounts frameCounts = diffFrameCountsAtEnd(activity);
+
+    if (frameCounts == null
+        || (frameCounts.totalFrames == 0
+            && frameCounts.slowFrames == 0
+            && frameCounts.frozenFrames == 0)) {
       return;
     }
 
-    final MeasurementValue tfValues = new MeasurementValue(totalFrames, NONE_UNIT);
-    final MeasurementValue sfValues = new MeasurementValue(slowFrames, NONE_UNIT);
-    final MeasurementValue ffValues = new MeasurementValue(frozenFrames, NONE_UNIT);
+    final MeasurementValue tfValues = new MeasurementValue(frameCounts.totalFrames, NONE_UNIT);
+    final MeasurementValue sfValues = new MeasurementValue(frameCounts.slowFrames, NONE_UNIT);
+    final MeasurementValue ffValues = new MeasurementValue(frameCounts.frozenFrames, NONE_UNIT);
     final Map<String, @NotNull MeasurementValue> measurements = new HashMap<>();
     measurements.put("frames_total", tfValues);
     measurements.put("frames_slow", sfValues);
@@ -132,6 +192,7 @@ public synchronized void setMetrics(
   public synchronized void stop() {
     if (isFrameMetricsAggregatorAvailable()) {
       frameMetricsAggregator.stop();
+      frameMetricsAggregator.reset();
     }
     activityMeasurements.clear();
   }