From 2de5510bdfcb0e27814bb25adc1aa85c42fa09e0 Mon Sep 17 00:00:00 2001
From: Charles Chen <charlesccchen@google.com>
Date: Tue, 11 Aug 2020 16:43:17 +0800
Subject: [PATCH] Fix lock contention in ResourcesManager

1. Scope dowm synchronized block to mAdjustedDisplays put and get
2. Use SoftReference instead of WeakReference because
   display's weak reference is cleared after getAdjustedDisplay
   returned and the cache didn't take effects previously.

fixes: 162445376
Test: ResourcesManagerPerfTest
Before:
    getDisplayMetrics_median: 3413
    getDisplayMetrics_mean: 3439
    getDisplayMetrics_min: 3364
    getDisplayMetrics_standardDeviation: 107
After:
    getDisplayMetrics_median: 3048
    getDisplayMetrics_mean: 3096
    getDisplayMetrics_min: 2999
    getDisplayMetrics_standardDeviation: 134

Change-Id: I8a5c3ba960c8231aee78b0e7fb958bf6845d5bb5
---
 .../android/app/ResourcesManagerPerfTest.java | 19 ++++++++-
 core/java/android/app/ResourcesManager.java   | 40 ++++++++++---------
 2 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/apct-tests/perftests/core/src/android/app/ResourcesManagerPerfTest.java b/apct-tests/perftests/core/src/android/app/ResourcesManagerPerfTest.java
index 050fecde82138..d3938f4c09268 100644
--- a/apct-tests/perftests/core/src/android/app/ResourcesManagerPerfTest.java
+++ b/apct-tests/perftests/core/src/android/app/ResourcesManagerPerfTest.java
@@ -17,7 +17,6 @@ package android.app;
 
 import android.content.Context;
 import android.content.res.Configuration;
-import android.content.res.Resources;
 import android.perftests.utils.BenchmarkState;
 import android.perftests.utils.PerfStatusReporter;
 import android.view.Display;
@@ -136,4 +135,22 @@ public class ResourcesManagerPerfTest {
             }
         }
     }
+
+    @Test
+    public void getDisplayMetrics() {
+        ResourcesManager resourcesManager = ResourcesManager.getInstance();
+
+        final BenchmarkState state = mPerfStatusReporter.getBenchmarkState();
+        while (state.keepRunning()) {
+            state.pauseTiming();
+            // Invalidate cache.
+            resourcesManager.applyConfigurationToResourcesLocked(
+                    resourcesManager.getConfiguration(), null);
+            state.resumeTiming();
+
+            // Invoke twice for testing cache.
+            resourcesManager.getDisplayMetrics();
+            resourcesManager.getDisplayMetrics();
+        }
+    }
 }
diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java
index 7cd3fcad177bd..9e4ab33c6aa05 100644
--- a/core/java/android/app/ResourcesManager.java
+++ b/core/java/android/app/ResourcesManager.java
@@ -54,6 +54,7 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.lang.ref.Reference;
 import java.lang.ref.ReferenceQueue;
+import java.lang.ref.SoftReference;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -245,7 +246,7 @@ public class ResourcesManager {
     /**
      * A cache of DisplayId, DisplayAdjustments to Display.
      */
-    private final ArrayMap<Pair<Integer, DisplayAdjustments>, WeakReference<Display>>
+    private final ArrayMap<Pair<Integer, DisplayAdjustments>, SoftReference<Display>>
             mAdjustedDisplays = new ArrayMap<>();
 
     /**
@@ -373,25 +374,28 @@ public class ResourcesManager {
                 ? new DisplayAdjustments(displayAdjustments) : new DisplayAdjustments();
         final Pair<Integer, DisplayAdjustments> key =
                 Pair.create(displayId, displayAdjustmentsCopy);
+        SoftReference<Display> sd;
         synchronized (this) {
-            WeakReference<Display> wd = mAdjustedDisplays.get(key);
-            if (wd != null) {
-                final Display display = wd.get();
-                if (display != null) {
-                    return display;
-                }
-            }
-            final DisplayManagerGlobal dm = DisplayManagerGlobal.getInstance();
-            if (dm == null) {
-                // may be null early in system startup
-                return null;
-            }
-            final Display display = dm.getCompatibleDisplay(displayId, key.second);
-            if (display != null) {
-                mAdjustedDisplays.put(key, new WeakReference<>(display));
-            }
-            return display;
+            sd = mAdjustedDisplays.get(key);
         }
+        if (sd != null) {
+            final Display display = sd.get();
+            if (display != null) {
+                return display;
+            }
+        }
+        final DisplayManagerGlobal dm = DisplayManagerGlobal.getInstance();
+        if (dm == null) {
+            // may be null early in system startup
+            return null;
+        }
+        final Display display = dm.getCompatibleDisplay(displayId, key.second);
+        if (display != null) {
+            synchronized (this) {
+                mAdjustedDisplays.put(key, new SoftReference<>(display));
+            }
+        }
+        return display;
     }
 
     /**