From 642aab2c507270b27a32746644114edf593aaee2 Mon Sep 17 00:00:00 2001
From: Lee Shombert <shombert@google.com>
Date: Thu, 8 Oct 2020 22:06:22 -0700
Subject: [PATCH] Enable in-process permission caches for system_server

Bug: 170472470

Do not disable the permission caches inside the system server.  This
is beneficial because permission checks require many binder calls, and
the cache, even though it is in-process, eliminates those calls.
1. Make the PermissionManager cache more effective by increasing the
   maximum size to 2048.  Experiments show that the high-water mark is
   about 600.
2. Suppress cache content in the 'dumpsys cacheinfo' output.  dumpsys
   fails if the output is too large in a single process, and the new
   caches push the system server cacheinfo over the limit.  The
   content can be added to the output by setting the new DETAILED
   boolean in PropertyInvalidatedCache to true, but this flag should
   never be committed with a value of true.

Test: two atest runs
 * atest FrameworksServicesTests:com.android.server.devicepolicy.DevicePolicyManagerTest#testGetPermissionGrantState
 * atest FrameworksServicesTests:NetworkPolicyManagerServiceTest

Change-Id: I7d3c1f34b44216bb510319ca5b6aced1cc53e05d
---
 core/java/android/app/PropertyInvalidatedCache.java      | 9 ++++++---
 core/java/android/permission/PermissionManager.java      | 2 +-
 .../server/pm/permission/PermissionManagerService.java   | 3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/core/java/android/app/PropertyInvalidatedCache.java b/core/java/android/app/PropertyInvalidatedCache.java
index 3e3a956c97880..04a12afb80392 100644
--- a/core/java/android/app/PropertyInvalidatedCache.java
+++ b/core/java/android/app/PropertyInvalidatedCache.java
@@ -206,6 +206,10 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
     private static final String TAG = "PropertyInvalidatedCache";
     private static final boolean DEBUG = false;
     private static final boolean VERIFY = false;
+    // If this is true, dumpsys will dump the cache entries along with cache statistics.
+    // Most of the time this causes dumpsys to fail because the output stream is too
+    // large.  Only set it to true in development images.
+    private static final boolean DETAILED = false;
 
     // Per-Cache performance counters. As some cache instances are declared static,
     @GuardedBy("mLock")
@@ -912,14 +916,13 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
                     "    Current Size: %d, Max Size: %d, HW Mark: %d, Overflows: %d",
                     mCache.size(), mMaxEntries, mHighWaterMark, mMissOverflow));
             pw.println(String.format("    Enabled: %s", mDisabled ? "false" : "true"));
+            pw.println("");
 
             Set<Map.Entry<Query, Result>> cacheEntries = mCache.entrySet();
-            if (cacheEntries.size() == 0) {
-                pw.println("");
+            if (!DETAILED || cacheEntries.size() == 0) {
                 return;
             }
 
-            pw.println("");
             pw.println("    Contents:");
             for (Map.Entry<Query, Result> entry : cacheEntries) {
                 String key = Objects.toString(entry.getKey());
diff --git a/core/java/android/permission/PermissionManager.java b/core/java/android/permission/PermissionManager.java
index d80a7e794220d..3cfa59be5aa80 100644
--- a/core/java/android/permission/PermissionManager.java
+++ b/core/java/android/permission/PermissionManager.java
@@ -609,7 +609,7 @@ public final class PermissionManager {
     /** @hide */
     private static final PropertyInvalidatedCache<PermissionQuery, Integer> sPermissionCache =
             new PropertyInvalidatedCache<PermissionQuery, Integer>(
-                    16, CACHE_KEY_PACKAGE_INFO, "checkPermission") {
+                    2048, CACHE_KEY_PACKAGE_INFO, "checkPermission") {
                 @Override
                 protected Integer recompute(PermissionQuery query) {
                     return checkPermissionUncached(query.permission, query.pid, query.uid);
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
index 25e1848816d17..231a80a3a3105 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
@@ -378,8 +378,9 @@ public class PermissionManagerService extends IPermissionManager.Stub {
             @NonNull Injector injector) {
         mInjector = injector;
         // The package info cache is the cache for package and permission information.
+        // Disable the package info and package permission caches locally but leave the
+        // checkPermission cache active.
         mInjector.invalidatePackageInfoCache();
-        mInjector.disablePermissionCache();
         mInjector.disablePackageNamePermissionCache();
 
         mContext = context;