From 40b2a35a9bed74a2c30c144d9dc7d878eee0651a Mon Sep 17 00:00:00 2001
From: Patrick Baumann <patb@google.com>
Date: Tue, 1 Sep 2020 16:52:14 -0700
Subject: [PATCH] Build AppsFilter cache without holding packages lock

This change attempts to update the apps filter cache without holding the
package manager lock. It makes a local copy of the package settings and
gets the hashcode of each setting's package object. After building the
cache, it checks that all of the packges are the same as before the
cache was built. If there was a change, the cache is discarded and
rebuilt while holding the package lock.

Before change:
MakePackageManagerServiceReady took to complete: ~30ms
MakeDisplayManagerServiceReady took to complete: ~180ms

After change:
MakePackageManagerServiceReady took to complete: ~110ms
MakeDisplayManagerServiceReady took to complete: ~1ms

Test: atest AppEnumerationTests
Fixes: 167169644
Fixes: 161214066
Bug: 161250592
Bug: 162347084
This change reverts commit 112a64a6dd94903f56272bc5c0d11d32f7b4e2ed.

Change-Id: I910d73f4d3b4cb1fe54d667752d40445b1c56906
---
 .../com/android/server/pm/AppsFilter.java     | 68 ++++++++++++++++---
 .../server/pm/PackageManagerService.java      |  5 +-
 2 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java
index f168ac70dda80..e1d45c4a3fd18 100644
--- a/services/core/java/com/android/server/pm/AppsFilter.java
+++ b/services/core/java/com/android/server/pm/AppsFilter.java
@@ -639,24 +639,74 @@ public class AppsFilter {
         }
     }
 
-    private void updateEntireShouldFilterCacheAsync() {
-        mBackgroundExecutor.execute(this::updateEntireShouldFilterCache);
-    }
-
     private void updateEntireShouldFilterCache() {
         mStateProvider.runWithState((settings, users) -> {
             SparseArray<SparseBooleanArray> cache =
-                    new SparseArray<>(users.length * settings.size());
-            for (int i = settings.size() - 1; i >= 0; i--) {
-                updateShouldFilterCacheForPackage(cache,
-                        null /*skipPackage*/, settings.valueAt(i), settings, users, i);
-            }
+                    updateEntireShouldFilterCacheInner(settings, users);
             synchronized (mCacheLock) {
                 mShouldFilterCache = cache;
             }
         });
     }
 
+    private SparseArray<SparseBooleanArray> updateEntireShouldFilterCacheInner(
+            ArrayMap<String, PackageSetting> settings, UserInfo[] users) {
+        SparseArray<SparseBooleanArray> cache =
+                new SparseArray<>(users.length * settings.size());
+        for (int i = settings.size() - 1; i >= 0; i--) {
+            updateShouldFilterCacheForPackage(cache,
+                    null /*skipPackage*/, settings.valueAt(i), settings, users, i);
+        }
+        return cache;
+    }
+
+    private void updateEntireShouldFilterCacheAsync() {
+        mBackgroundExecutor.execute(() -> {
+            final ArrayMap<String, PackageSetting> settingsCopy = new ArrayMap<>();
+            final ArrayMap<String, AndroidPackage> packagesCache = new ArrayMap<>();
+            final UserInfo[][] usersRef = new UserInfo[1][];
+            mStateProvider.runWithState((settings, users) -> {
+                packagesCache.ensureCapacity(settings.size());
+                settingsCopy.putAll(settings);
+                usersRef[0] = users;
+                // store away the references to the immutable packages, since settings are retained
+                // during updates.
+                for (int i = 0, max = settings.size(); i < max; i++) {
+                    final AndroidPackage pkg = settings.valueAt(i).pkg;
+                    packagesCache.put(settings.keyAt(i), pkg);
+                }
+            });
+            SparseArray<SparseBooleanArray> cache =
+                    updateEntireShouldFilterCacheInner(settingsCopy, usersRef[0]);
+            boolean[] changed = new boolean[1];
+            // We have a cache, let's make sure the world hasn't changed out from under us.
+            mStateProvider.runWithState((settings, users) -> {
+                if (settings.size() != settingsCopy.size()) {
+                    changed[0] = true;
+                    return;
+                }
+                for (int i = 0, max = settings.size(); i < max; i++) {
+                    final AndroidPackage pkg = settings.valueAt(i).pkg;
+                    if (!Objects.equals(pkg, packagesCache.get(settings.keyAt(i)))) {
+                        changed[0] = true;
+                        return;
+                    }
+                }
+            });
+            if (changed[0]) {
+                // Something has changed, just update the cache inline with the lock held
+                updateEntireShouldFilterCache();
+                if (DEBUG_LOGGING) {
+                    Slog.i(TAG, "Rebuilding cache with lock due to package change.");
+                }
+            } else {
+                synchronized (mCacheLock) {
+                    mShouldFilterCache = cache;
+                }
+            }
+        });
+    }
+
     public void onUsersChanged() {
         synchronized (mCacheLock) {
             if (mShouldFilterCache != null) {
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 7945d84933345..b65335a41790b 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -21632,6 +21632,8 @@ public class PackageManagerService extends IPackageManager.Stub
                         .getUriFor(Secure.INSTANT_APPS_ENABLED), false, co, UserHandle.USER_ALL);
         co.onChange(true);
 
+        mAppsFilter.onSystemReady();
+
         // Disable any carrier apps. We do this very early in boot to prevent the apps from being
         // disabled after already being started.
         CarrierAppUtils.disableCarrierAppsUntilPrivileged(
@@ -21780,9 +21782,6 @@ public class PackageManagerService extends IPackageManager.Stub
         mInstallerService.restoreAndApplyStagedSessionIfNeeded();
 
         mExistingPackages = null;
-
-        // We'll do this last as it builds its cache while holding mLock via callback.
-        mAppsFilter.onSystemReady();
     }
 
     public void waitForAppDataPrepared() {