From f2e011daaec1cc243577e4272ad3c858a8cfddda Mon Sep 17 00:00:00 2001 From: Lee Shombert Date: Fri, 27 Dec 2019 07:30:58 -0800 Subject: [PATCH] Binder cache for getPackagesForUid(). Bug: 140788621 This changes adds a binder cache for the getPackagesForUid() binder call. Test: A special build that puts the PropertyInvalidatedCache in verification mode was loaded on the device. Then one iteration of MPTS was executed. No cache inconsistencies were found and no SELinux violations (associated with the binder cache) were found. The number of cache misses was approximately 15% of the total binder calls. 84.5% of all binder calls went through the cache. The bulk of the calls that bypassed the cache were singletons: one call per process. These calls would not be helped by a cache anyway. Besides the MPTS run, tested by switching between users. Change-Id: I9a5fb3b7bced031d568aa50b4f3cc495433fcbe7 --- .../app/ApplicationPackageManager.java | 87 +++++++++++++++++-- .../server/pm/PackageManagerService.java | 11 ++- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/core/java/android/app/ApplicationPackageManager.java b/core/java/android/app/ApplicationPackageManager.java index 0b0a803c7dcd0..60660b18871c2 100644 --- a/core/java/android/app/ApplicationPackageManager.java +++ b/core/java/android/app/ApplicationPackageManager.java @@ -110,6 +110,7 @@ import libcore.util.EmptyArray; import java.lang.ref.WeakReference; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -942,13 +943,89 @@ public class ApplicationPackageManager extends PackageManager { } } + /** + * Wrap the cached value in a class that does deep compares on string + * arrays. The comparison is needed only for the verification mode of + * PropertyInvalidatedCache; this mode is only enabled for debugging. + * The return result is an array of strings but the order in the array + * is not important. To properly compare two arrays, the arrays are + * sorted before the comparison. + */ + private static class GetPackagesForUidResult { + private final String [] mValue; + GetPackagesForUidResult(String []s) { + mValue = s; + } + public String[] value() { + return mValue; + } + @Override + public String toString() { + return Arrays.toString(mValue); + } + @Override + public int hashCode() { + return Arrays.hashCode(mValue); + } + /** + * Arrays.sort() throws an NPE if passed a null pointer, so nulls + * are handled first. + */ + @Override + public boolean equals(Object o) { + if (o instanceof GetPackagesForUidResult) { + String [] r = ((GetPackagesForUidResult) o).mValue; + String [] l = mValue; + if ((r == null) != (l == null)) { + return false; + } else if (r == null) { + return true; + } + // Both arrays are non-null. Sort before comparing. + Arrays.sort(r); + Arrays.sort(l); + return Arrays.equals(l, r); + } else { + return false; + } + } + } + + private static final String CACHE_KEY_PACKAGES_FOR_UID_PROPERTY = + "cache_key.get_packages_for_uid"; + private static final PropertyInvalidatedCache + mGetPackagesForUidCache = + new PropertyInvalidatedCache( + 32, CACHE_KEY_PACKAGES_FOR_UID_PROPERTY) { + @Override + protected GetPackagesForUidResult recompute(Integer uid) { + try { + return new GetPackagesForUidResult( + ActivityThread.currentActivityThread(). + getPackageManager().getPackagesForUid(uid)); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + @Override + public String queryToString(Integer uid) { + return String.format("uid=%d", uid.intValue()); + } + }; + @Override public String[] getPackagesForUid(int uid) { - try { - return mPM.getPackagesForUid(uid); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + return mGetPackagesForUidCache.query(uid).value(); + } + + /** @hide */ + public static void disableGetPackagesForUidCache() { + mGetPackagesForUidCache.disableLocal(); + } + + /** @hide */ + public static void invalidateGetPackagesForUidCache() { + PropertyInvalidatedCache.invalidateCache(CACHE_KEY_PACKAGES_FOR_UID_PROPERTY); } @Override diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 09ba888d73a4b..f0b10d86948e9 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2654,6 +2654,9 @@ public class PackageManagerService extends IPackageManager.Stub public PackageManagerService(Injector injector, boolean onlyCore, boolean factoryTest) { PackageManager.disableApplicationInfoCache(); PackageManager.disablePackageInfoCache(); + ApplicationPackageManager.invalidateGetPackagesForUidCache(); + ApplicationPackageManager.disableGetPackagesForUidCache(); + ApplicationPackageManager.invalidateHasSystemFeatureCache(); // Avoid invalidation-thrashing by preventing cache invalidations from causing property // writes if the cache isn't enabled yet. We re-enable writes later when we're @@ -2747,7 +2750,6 @@ public class PackageManagerService extends IPackageManager.Stub t.traceBegin("get system config"); SystemConfig systemConfig = SystemConfig.getInstance(); mAvailableFeatures = systemConfig.getAvailableFeatures(); - ApplicationPackageManager.invalidateHasSystemFeatureCache(); t.traceEnd(); mProtectedPackages = new ProtectedPackages(mContext); @@ -5947,6 +5949,11 @@ public class PackageManagerService extends IPackageManager.Stub * * The second is an artifact of the current data structures and should be fixed. See * b/111075456 for one such instance. + * This binder API is cached. If the algorithm in this method changes, + * or if the underlying objecs (as returned by getSettingLPr()) change + * then the logic that invalidates the cache must be revisited. See + * calls to invalidateGetPackagesForUidCache() to locate the points at + * which the cache is invalidated. */ @Override public String[] getPackagesForUid(int uid) { @@ -16216,6 +16223,7 @@ public class PackageManagerService extends IPackageManager.Stub updateInstantAppInstallerLocked(packageName); } } + ApplicationPackageManager.invalidateGetPackagesForUidCache(); } /** @@ -17834,6 +17842,7 @@ public class PackageManagerService extends IPackageManager.Stub updateInstantAppInstallerLocked(packageName); } } + ApplicationPackageManager.invalidateGetPackagesForUidCache(); } if (res) {