Merge "Add Merging Logic to Jank Processor" into main

5 files changed, 173 insertions, 8 deletions

diff --git a/core/java/android/app/jank/FrameOverrunHistogram.java b/core/java/android/app/jank/FrameOverrunHistogram.java
index e28ac126a90a..3ad6531a46bf 100644
--- a/core/java/android/app/jank/FrameOverrunHistogram.java
+++ b/core/java/android/app/jank/FrameOverrunHistogram.java
@@ -39,7 +39,7 @@ public class FrameOverrunHistogram {
      * Create a new instance of FrameOverrunHistogram.
      */
     public FrameOverrunHistogram() {
-        mBucketCounts = new int[sBucketEndpoints.length - 1];
+        mBucketCounts = new int[sBucketEndpoints.length];
     }
 
     /**
diff --git a/core/java/android/app/jank/JankDataProcessor.java b/core/java/android/app/jank/JankDataProcessor.java
index 7525d0402ee4..7ceaeb3fb070 100644
--- a/core/java/android/app/jank/JankDataProcessor.java
+++ b/core/java/android/app/jank/JankDataProcessor.java
@@ -59,7 +59,6 @@ public class JankDataProcessor {
      * @param appUid the uid of the app.
      */
     public void processJankData(List<JankData> jankData, String activityName, int appUid) {
-        mCurrentBatchCount++;
         // add all the previous and active states to the pending states list.
         mStateTracker.retrieveAllStates(mPendingStates);
 
@@ -79,9 +78,8 @@ public class JankDataProcessor {
             }
         }
         // At this point we have attributed all frames to a state.
-        if (mCurrentBatchCount >= LOG_BATCH_FREQUENCY) {
-            logMetricCounts();
-        }
+        incrementBatchCountAndMaybeLogStats();
+
         // return the StatData object back to the pool to be reused.
         jankDataProcessingComplete();
     }
@@ -91,7 +89,73 @@ public class JankDataProcessor {
      * stats
      */
     public void mergeJankStats(AppJankStats jankStats, String activityName) {
-        // TODO b/377572463 Add Merging Logic
+        // Each state has a key which is a combination of widget category, widget id and widget
+        // state, this key is also used to identify pending stats, a pending stat is essentially a
+        // state with frames associated with it.
+        String stateKey = mStateTracker.getStateKey(jankStats.getWidgetCategory(),
+                jankStats.getWidgetId(), jankStats.getWidgetState());
+
+        if (mPendingJankStats.containsKey(stateKey)) {
+            mergeExistingStat(stateKey, jankStats);
+        } else {
+            mergeNewStat(stateKey, activityName, jankStats);
+        }
+
+        incrementBatchCountAndMaybeLogStats();
+    }
+
+    private void mergeExistingStat(String stateKey, AppJankStats jankStat) {
+        PendingJankStat pendingStat = mPendingJankStats.get(stateKey);
+
+        pendingStat.mJankyFrames += jankStat.getJankyFrameCount();
+        pendingStat.mTotalFrames += jankStat.getTotalFrameCount();
+
+        mergeOverrunHistograms(pendingStat.mFrameOverrunBuckets,
+                jankStat.getFrameOverrunHistogram().getBucketCounters());
+    }
+
+    private void mergeNewStat(String stateKey, String activityName, AppJankStats jankStats) {
+        // Check if we have space for a new stat
+        if (mPendingJankStats.size() > MAX_IN_MEMORY_STATS) {
+            return;
+        }
+
+        PendingJankStat pendingStat = mPendingJankStatsPool.acquire();
+        if (pendingStat == null) {
+            pendingStat = new PendingJankStat();
+
+        }
+        pendingStat.clearStats();
+
+        pendingStat.mActivityName = activityName;
+        pendingStat.mUid = jankStats.getUid();
+        pendingStat.mWidgetId = jankStats.getWidgetId();
+        pendingStat.mWidgetCategory = jankStats.getWidgetCategory();
+        pendingStat.mWidgetState = jankStats.getWidgetState();
+        pendingStat.mTotalFrames = jankStats.getTotalFrameCount();
+        pendingStat.mJankyFrames = jankStats.getJankyFrameCount();
+
+        mergeOverrunHistograms(pendingStat.mFrameOverrunBuckets,
+                jankStats.getFrameOverrunHistogram().getBucketCounters());
+
+        mPendingJankStats.put(stateKey, pendingStat);
+    }
+
+    private void mergeOverrunHistograms(int[] mergeTarget, int[] mergeSource) {
+        // The length of each histogram should be identical, if they are not then its possible the
+        // buckets are not in sync, these records should not be recorded.
+        if (mergeTarget.length != mergeSource.length) return;
+
+        for (int i = 0; i < mergeTarget.length; i++) {
+            mergeTarget[i] += mergeSource[i];
+        }
+    }
+
+    private void incrementBatchCountAndMaybeLogStats() {
+        mCurrentBatchCount++;
+        if (mCurrentBatchCount >= LOG_BATCH_FREQUENCY) {
+            logMetricCounts();
+        }
     }
 
     /**
diff --git a/core/java/android/app/jank/JankTracker.java b/core/java/android/app/jank/JankTracker.java
index 202281f98c97..469521668d25 100644
--- a/core/java/android/app/jank/JankTracker.java
+++ b/core/java/android/app/jank/JankTracker.java
@@ -89,7 +89,12 @@ public class JankTracker {
      * stats
      */
     public void mergeAppJankStats(AppJankStats appJankStats) {
-        mJankDataProcessor.mergeJankStats(appJankStats, mActivityName);
+        getHandler().post(new Runnable() {
+            @Override
+            public void run() {
+                mJankDataProcessor.mergeJankStats(appJankStats, mActivityName);
+            }
+        });
     }
 
     public void setActivityName(@NonNull String activityName) {
diff --git a/core/java/android/app/jank/StateTracker.java b/core/java/android/app/jank/StateTracker.java
index c86d5a5cff20..21bb5e8280ee 100644
--- a/core/java/android/app/jank/StateTracker.java
+++ b/core/java/android/app/jank/StateTracker.java
@@ -180,7 +180,11 @@ public class StateTracker {
         }
     }
 
-    private String getStateKey(String widgetCategory, String widgetId, String widgetState) {
+    /**
+     * Returns a concatenated string of the inputs. This key can be used to retrieve both pending
+     * stats and the state that was used to create the pending stat.
+     */
+    public String getStateKey(String widgetCategory, String widgetId, String widgetState) {
         return widgetCategory + widgetId + widgetState;
     }
 
diff --git a/tests/AppJankTest/src/android/app/jank/tests/JankDataProcessorTest.java b/tests/AppJankTest/src/android/app/jank/tests/JankDataProcessorTest.java
index 2cd625eec032..4d495adf727b 100644
--- a/tests/AppJankTest/src/android/app/jank/tests/JankDataProcessorTest.java
+++ b/tests/AppJankTest/src/android/app/jank/tests/JankDataProcessorTest.java
@@ -18,7 +18,9 @@ package android.app.jank.tests;
 
 import static org.junit.Assert.assertEquals;
 
+import android.app.jank.AppJankStats;
 import android.app.jank.Flags;
+import android.app.jank.FrameOverrunHistogram;
 import android.app.jank.JankDataProcessor;
 import android.app.jank.StateTracker;
 import android.platform.test.annotations.RequiresFlagsEnabled;
@@ -39,6 +41,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 
 @RunWith(AndroidJUnit4.class)
@@ -154,6 +157,73 @@ public class JankDataProcessorTest {
         assertEquals(totalFrames, histogramFrames);
     }
 
+    @Test
+    @RequiresFlagsEnabled(Flags.FLAG_DETAILED_APP_JANK_METRICS_API)
+    public void mergeAppJankStats_confirmStatAddedToPendingStats() {
+        HashMap<String, JankDataProcessor.PendingJankStat> pendingStats =
+                mJankDataProcessor.getPendingJankStats();
+
+        assertEquals(pendingStats.size(), 0);
+
+        AppJankStats jankStats = getAppJankStats();
+        mJankDataProcessor.mergeJankStats(jankStats, sActivityName);
+
+        pendingStats = mJankDataProcessor.getPendingJankStats();
+
+        assertEquals(pendingStats.size(), 1);
+    }
+
+    /**
+     * This test confirms matching states are combined into one pending stat.  When JankStats are
+     * merged from outside the platform they will contain widget category, widget id and widget
+     * state. If an incoming JankStats matches a pending stat on all those fields the incoming
+     * JankStat will be merged into the existing stat.
+     */
+    @Test
+    @RequiresFlagsEnabled(Flags.FLAG_DETAILED_APP_JANK_METRICS_API)
+    public void mergeAppJankStats_confirmStatsWithMatchingStatesAreCombinedIntoOnePendingStat() {
+        AppJankStats jankStats = getAppJankStats();
+        mJankDataProcessor.mergeJankStats(jankStats, sActivityName);
+
+        HashMap<String, JankDataProcessor.PendingJankStat> pendingStats =
+                mJankDataProcessor.getPendingJankStats();
+        assertEquals(pendingStats.size(), 1);
+
+        AppJankStats secondJankStat = getAppJankStats();
+        mJankDataProcessor.mergeJankStats(secondJankStat, sActivityName);
+
+        pendingStats = mJankDataProcessor.getPendingJankStats();
+
+        assertEquals(pendingStats.size(), 1);
+    }
+
+    @Test
+    @RequiresFlagsEnabled(Flags.FLAG_DETAILED_APP_JANK_METRICS_API)
+    public void mergeAppJankStats_whenStatsWithMatchingStatesMerge_confirmFrameCountsAdded() {
+        AppJankStats jankStats = getAppJankStats();
+        mJankDataProcessor.mergeJankStats(jankStats, sActivityName);
+        mJankDataProcessor.mergeJankStats(jankStats, sActivityName);
+
+        HashMap<String, JankDataProcessor.PendingJankStat> pendingStats =
+                mJankDataProcessor.getPendingJankStats();
+
+        String statKey = pendingStats.keySet().iterator().next();
+        JankDataProcessor.PendingJankStat pendingStat = pendingStats.get(statKey);
+
+        assertEquals(pendingStats.size(), 1);
+        // The same jankStats objects are merged twice, this should result in the frame counts being
+        // doubled.
+        assertEquals(jankStats.getJankyFrameCount() * 2, pendingStat.getJankyFrames());
+        assertEquals(jankStats.getTotalFrameCount() * 2, pendingStat.getTotalFrames());
+
+        int[] originalHistogramBuckets = jankStats.getFrameOverrunHistogram().getBucketCounters();
+        int[] frameOverrunBuckets = pendingStat.getFrameOverrunBuckets();
+
+        for (int i = 0; i < frameOverrunBuckets.length; i++) {
+            assertEquals(originalHistogramBuckets[i] * 2, frameOverrunBuckets[i]);
+        }
+    }
+
     // TODO b/375005277 add tests that cover logging and releasing resources back to pool.
 
     private long getTotalFramesCounted() {
@@ -276,4 +346,26 @@ public class JankDataProcessorTest {
         return mockData;
     }
 
+    private AppJankStats getAppJankStats() {
+        AppJankStats jankStats = new AppJankStats(
+                /*App Uid*/APP_ID,
+                /*Widget Id*/"test widget id",
+                /*Widget Category*/AppJankStats.SCROLL,
+                /*Widget State*/AppJankStats.SCROLLING,
+                /*Total Frames*/100,
+                /*Janky Frames*/25,
+                getOverrunHistogram()
+        );
+        return jankStats;
+    }
+
+    private FrameOverrunHistogram getOverrunHistogram() {
+        FrameOverrunHistogram overrunHistogram = new FrameOverrunHistogram();
+        overrunHistogram.addFrameOverrunMillis(-2);
+        overrunHistogram.addFrameOverrunMillis(1);
+        overrunHistogram.addFrameOverrunMillis(5);
+        overrunHistogram.addFrameOverrunMillis(25);
+        return overrunHistogram;
+    }
+
 }