[Launcher Jank] Make ChangeReporter faster

1. Use compare-and-set in ChangeReporter#reportChange() to reduce 1 read
   access of isAlreadyReported() to mReportedChanges. Also avoided
creating a new ChangeReport obj.
2. Make mReportedChanges thread safe by using ConcurrentHashMap<Uid, SynchronizedSet<ChangeReport>> to remove system-wide lock contention of `ChangeReporter#isAlreadyReported()`

The pattern of accessing the data structure is that a thread will serially call 2 `isAlreadyReported()` then 1 `markAsReported()` in a row from `ChangeReporter#reportChange()` https://screenshot.googleplex.com/5bhyuaaPXYQLcmq

- I have observed concurrent access to different uid from different tid (meaning two different threads accessing different Set<ChangeReport> concurrently)
- I have NOT observed concurrent access to same uid from different tid.

So there is no probably no need to optimized for concurrent access to `Set<ChangeReport>` and we can use `SynchronizedSet`.

Fix: 336364201
Test: presubmit
Flag: NONE
Change-Id: I1a0524302a58f948c51eef318ba35c4e907d855d

1 files changed, 60 insertions, 42 deletions

diff --git a/core/java/com/android/internal/compat/ChangeReporter.java b/core/java/com/android/internal/compat/ChangeReporter.java
index 6ff546fd77f8..ded142ce49e5 100644
--- a/core/java/com/android/internal/compat/ChangeReporter.java
+++ b/core/java/com/android/internal/compat/ChangeReporter.java
@@ -18,22 +18,24 @@ package com.android.internal.compat;
 
 import static android.text.TextUtils.formatSimple;
 
+import static java.util.Collections.EMPTY_SET;
+
 import android.annotation.IntDef;
 import android.util.Log;
 import android.util.Slog;
 
-import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.compat.flags.Flags;
 import com.android.internal.util.FrameworkStatsLog;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.HashSet;
-import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
 
 /**
  * A helper class to report changes to stats log.
@@ -42,6 +44,8 @@ import java.util.Set;
  */
 public final class ChangeReporter {
     private static final String TAG = "CompatChangeReporter";
+    private static final Function<Integer, Set<ChangeReport>> NEW_CHANGE_REPORT_SET =
+            uid -> Collections.synchronizedSet(new HashSet<>());
     private int mSource;
 
     private static final class ChangeReport {
@@ -69,15 +73,14 @@ public final class ChangeReporter {
     }
 
     // Maps uid to a set of ChangeReports (that were reported for that uid).
-    @GuardedBy("mReportedChanges")
-    private final Map<Integer, Set<ChangeReport>> mReportedChanges;
+    private final ConcurrentHashMap<Integer, Set<ChangeReport>> mReportedChanges;
 
     // When true will of every time to debug (logcat).
     private boolean mDebugLogAll;
 
     public ChangeReporter(@Source int source) {
         mSource = source;
-        mReportedChanges =  new HashMap<>();
+        mReportedChanges =  new ConcurrentHashMap<>();
         mDebugLogAll = false;
     }
 
@@ -93,14 +96,15 @@ public final class ChangeReporter {
      *                        actually log. If the sdk version does not matter, should be true.
      */
     public void reportChange(int uid, long changeId, int state, boolean isLoggableBySdk) {
-        if (shouldWriteToStatsLog(uid, changeId, state)) {
+        boolean isAlreadyReported =
+                checkAndSetIsAlreadyReported(uid, new ChangeReport(changeId, state));
+        if (!isAlreadyReported) {
             FrameworkStatsLog.write(FrameworkStatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid,
                     changeId, state, mSource);
         }
-        if (shouldWriteToDebug(uid, changeId, state, isLoggableBySdk)) {
+        if (shouldWriteToDebug(isAlreadyReported, state, isLoggableBySdk)) {
             debugLog(uid, changeId, state);
         }
-        markAsReported(uid, new ChangeReport(changeId, state));
     }
 
     /**
@@ -129,7 +133,6 @@ public final class ChangeReporter {
         mDebugLogAll = false;
     }
 
-
     /**
      * Returns whether the next report should be logged to FrameworkStatsLog.
      *
@@ -139,28 +142,26 @@ public final class ChangeReporter {
      * @return true if the report should be logged
      */
     @VisibleForTesting
-    public boolean shouldWriteToStatsLog(int uid, long changeId, int state) {
+    boolean shouldWriteToStatsLog(int uid, long changeId, int state) {
         return !isAlreadyReported(uid, new ChangeReport(changeId, state));
     }
 
     /**
      * Returns whether the next report should be logged to logcat.
      *
-     * @param uid             affected by the change
-     * @param changeId        the reported change id
-     * @param state           of the reported change - enabled/disabled/only logged
-     * @param isLoggableBySdk whether debug logging is allowed for this change based on target
-     *                        SDK version. This is combined with other logic to determine whether to
-     *                        actually log. If the sdk version does not matter, should be true.
+     * @param isAlreadyReported is the change already reported
+     * @param state             of the reported change - enabled/disabled/only logged
+     * @param isLoggableBySdk   whether debug logging is allowed for this change based on target SDK
+     *                          version. This is combined with other logic to determine whether to
+     *                          actually log. If the sdk version does not matter, should be true.
      * @return true if the report should be logged
      */
-    @VisibleForTesting
-    public boolean shouldWriteToDebug(
-            int uid, long changeId, int state, boolean isLoggableBySdk) {
+    private boolean shouldWriteToDebug(
+            boolean isAlreadyReported, int state, boolean isLoggableBySdk) {
         // If log all bit is on, always return true.
         if (mDebugLogAll) return true;
         // If the change has already been reported, do not write.
-        if (isAlreadyReported(uid, new ChangeReport(changeId, state))) return false;
+        if (isAlreadyReported) return false;
 
         // If the flag is turned off or the TAG's logging is forced to debug level with
         // `adb setprop log.tag.CompatChangeReporter=DEBUG`, write to debug since the above checks
@@ -178,33 +179,53 @@ public final class ChangeReporter {
      * @param uid         affected by the change
      * @param changeId    the reported change id
      * @param state       of the reported change - enabled/disabled/only logged
+     *
      * @return true if the report should be logged
      */
     @VisibleForTesting
-    public boolean shouldWriteToDebug(int uid, long changeId, int state) {
+    boolean shouldWriteToDebug(int uid, long changeId, int state) {
         return shouldWriteToDebug(uid, changeId, state, true);
     }
 
-    private boolean isAlreadyReported(int uid, ChangeReport report) {
-        synchronized (mReportedChanges) {
-            Set<ChangeReport> reportedChangesForUid = mReportedChanges.get(uid);
-            if (reportedChangesForUid == null) {
-                return false;
-            } else {
-                return reportedChangesForUid.contains(report);
-            }
+    /**
+     * Returns whether the next report should be logged to logcat.
+     *
+     * @param uid               affected by the change
+     * @param changeId          the reported change id
+     * @param state             of the reported change - enabled/disabled/only logged
+     * @param isLoggableBySdk   whether debug logging is allowed for this change based on target SDK
+     *                          version. This is combined with other logic to determine whether to
+     *                          actually log. If the sdk version does not matter, should be true.
+     * @return true if the report should be logged
+     */
+    @VisibleForTesting
+    boolean shouldWriteToDebug(int uid, long changeId, int state, boolean isLoggableBySdk) {
+        return shouldWriteToDebug(
+                isAlreadyReported(uid, new ChangeReport(changeId, state)), state, isLoggableBySdk);
+    }
+
+    /**
+     * Return if change has been reported. Also mark change as reported if not.
+     *
+     * @param uid affected by the change
+     * @param changeReport change reported to be checked and marked as reported.
+     *
+     * @return true if change has been reported, and vice versa.
+     */
+    private boolean checkAndSetIsAlreadyReported(int uid, ChangeReport changeReport) {
+        boolean isAlreadyReported = isAlreadyReported(uid, changeReport);
+        if (!isAlreadyReported) {
+            markAsReported(uid, changeReport);
         }
+        return isAlreadyReported;
+    }
+
+    private boolean isAlreadyReported(int uid, ChangeReport report) {
+        return mReportedChanges.getOrDefault(uid, EMPTY_SET).contains(report);
     }
 
     private void markAsReported(int uid, ChangeReport report) {
-        synchronized (mReportedChanges) {
-            Set<ChangeReport> reportedChangesForUid = mReportedChanges.get(uid);
-            if (reportedChangesForUid == null) {
-                mReportedChanges.put(uid, new HashSet<ChangeReport>());
-                reportedChangesForUid = mReportedChanges.get(uid);
-            }
-            reportedChangesForUid.add(report);
-        }
+        mReportedChanges.computeIfAbsent(uid, NEW_CHANGE_REPORT_SET).add(report);
     }
 
     /**
@@ -216,9 +237,7 @@ public final class ChangeReporter {
      * @param uid to reset
      */
     public void resetReportedChanges(int uid) {
-        synchronized (mReportedChanges) {
-            mReportedChanges.remove(uid);
-        }
+        mReportedChanges.remove(uid);
     }
 
     private void debugLog(int uid, long changeId, int state) {
@@ -229,7 +248,6 @@ public final class ChangeReporter {
         } else {
             Log.d(TAG, message);
         }
-
     }
 
     /**