From f73a41c1cf024420072462412540035f0e18930f Mon Sep 17 00:00:00 2001 From: Daniel Colascione Date: Wed, 4 Mar 2020 12:21:42 -0800 Subject: [PATCH 1/3] RESTRICT AUTOMERGE: Cork package information cache invalidations during boot Bug: 140788621 Bug: 150331002 Test: enable DEBUG; watch log Change-Id: Ife4f8c4e7d10524d6000399947a4e4b9d36db750 --- core/java/android/content/pm/PackageManager.java | 15 +++++++++++++++ .../android/server/pm/PackageManagerService.java | 9 ++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/core/java/android/content/pm/PackageManager.java b/core/java/android/content/pm/PackageManager.java index f48d78ac9cc3e..21f27f646fc14 100644 --- a/core/java/android/content/pm/PackageManager.java +++ b/core/java/android/content/pm/PackageManager.java @@ -8132,4 +8132,19 @@ public abstract class PackageManager { sPackageInfoCache.disableLocal(); } + /** + * Inhibit package info cache invalidations when correct. + * + * @hide */ + public static void corkPackageInfoCache() { + PropertyInvalidatedCache.corkInvalidations(PermissionManager.CACHE_KEY_PACKAGE_INFO); + } + + /** + * Enable package info cache invalidations. + * + * @hide */ + public static void uncorkPackageInfoCache() { + PropertyInvalidatedCache.uncorkInvalidations(PermissionManager.CACHE_KEY_PACKAGE_INFO); + } } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index f96ab1d9a0427..14882f8039e7d 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2810,10 +2810,14 @@ public class PackageManagerService extends IPackageManager.Stub } public PackageManagerService(Injector injector, boolean onlyCore, boolean factoryTest) { - PackageManager.invalidatePackageInfoCache(); PackageManager.disableApplicationInfoCache(); PackageManager.disablePackageInfoCache(); + // 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 + // done initializing. + PackageManager.corkPackageInfoCache(); + final TimingsTraceAndSlog t = new TimingsTraceAndSlog(TAG + "Timing", Trace.TRACE_TAG_PACKAGE_MANAGER); mPendingBroadcasts = new PendingPackageBroadcasts(); @@ -3614,6 +3618,9 @@ public class PackageManagerService extends IPackageManager.Stub mModuleInfoProvider = new ModuleInfoProvider(mContext, this); + // Uncork cache invalidations and allow clients to cache package information. + PackageManager.uncorkPackageInfoCache(); + // Now after opening every single application zip, make sure they // are all flushed. Not really needed, but keeps things nice and // tidy. From 33fb4ef029f2ba1771eeb76f2ea9e2854c852df7 Mon Sep 17 00:00:00 2001 From: Daniel Colascione Date: Wed, 4 Mar 2020 17:27:56 -0800 Subject: [PATCH 2/3] RESTRICT AUTOMERGE: Cork permission and package cache around bulk permission update Test: watch with DEBUG=true Bug: 149255086 Bug: 140788621 Change-Id: I32f9b89ae1aaa718ef0eef7bc668cdef49ade151 --- .../pm/permission/PermissionManagerService.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) 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 b7c9ecb604f8d..8555e28d9f0f7 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -3901,11 +3901,16 @@ public class PermissionManagerService extends IPermissionManager.Stub { */ private void updateAllPermissions(@Nullable String volumeUuid, boolean sdkUpdated, @NonNull PermissionCallback callback) { - final int flags = UPDATE_PERMISSIONS_ALL | - (sdkUpdated - ? UPDATE_PERMISSIONS_REPLACE_PKG | UPDATE_PERMISSIONS_REPLACE_ALL - : 0); - updatePermissions(null, null, volumeUuid, flags, callback); + PackageManager.corkPackageInfoCache(); // Prevent invalidation storm + try { + final int flags = UPDATE_PERMISSIONS_ALL | + (sdkUpdated + ? UPDATE_PERMISSIONS_REPLACE_PKG | UPDATE_PERMISSIONS_REPLACE_ALL + : 0); + updatePermissions(null, null, volumeUuid, flags, callback); + } finally { + PackageManager.uncorkPackageInfoCache(); + } } /** From f0e88b2f2c13b87220beebc8c0619e4002f14c95 Mon Sep 17 00:00:00 2001 From: Daniel Colascione Date: Thu, 19 Mar 2020 13:56:35 -0700 Subject: [PATCH 3/3] RESTRICT AUTOMERGE: Add a facility for time-based cache corking AutoCorker addresses the situation where big invalidation storms kill performance but we don't have a way to insert a manual cork around these update storms. Bug: 140788621 Test: m Change-Id: If07d693886fca340c7a18d5a607a4f235aa7107d --- .../android/app/PropertyInvalidatedCache.java | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/core/java/android/app/PropertyInvalidatedCache.java b/core/java/android/app/PropertyInvalidatedCache.java index a24a5b7823b6a..5b99c72dc55e8 100644 --- a/core/java/android/app/PropertyInvalidatedCache.java +++ b/core/java/android/app/PropertyInvalidatedCache.java @@ -17,6 +17,10 @@ package android.app; import android.annotation.NonNull; +import android.os.Handler; +import android.os.Looper; +import android.os.Message; +import android.os.SystemClock; import android.os.SystemProperties; import android.util.Log; @@ -492,6 +496,10 @@ public abstract class PropertyInvalidatedCache { public static void corkInvalidations(@NonNull String name) { synchronized (sCorkLock) { int numberCorks = sCorks.getOrDefault(name, 0); + if (DEBUG) { + Log.d(TAG, String.format("corking %s: numberCorks=%s", name, numberCorks)); + } + // If we're the first ones to cork this cache, set the cache to the unset state so // existing caches talk directly to their services while we've corked updates. // Make sure we don't clobber a disabled cache value. @@ -523,6 +531,10 @@ public abstract class PropertyInvalidatedCache { public static void uncorkInvalidations(@NonNull String name) { synchronized (sCorkLock) { int numberCorks = sCorks.getOrDefault(name, 0); + if (DEBUG) { + Log.d(TAG, String.format("uncorking %s: numberCorks=%s", name, numberCorks)); + } + if (numberCorks < 1) { throw new AssertionError("cork underflow: " + name); } @@ -538,6 +550,106 @@ public abstract class PropertyInvalidatedCache { } } + /** + * Time-based automatic corking helper. This class allows providers of cached data to + * amortize the cost of cache invalidations by corking the cache immediately after a + * modification (instructing clients to bypass the cache temporarily) and automatically + * uncork after some period of time has elapsed. + * + * It's better to use explicit cork and uncork pairs that tighly surround big batches of + * invalidations, but it's not always practical to tell where these invalidation batches + * might occur. AutoCorker's time-based corking is a decent alternative. + */ + public static final class AutoCorker { + public static final int DEFAULT_AUTO_CORK_DELAY_MS = 2000; + + private final String mPropertyName; + private final int mAutoCorkDelayMs; + private final Object mLock = new Object(); + @GuardedBy("mLock") + private long mUncorkDeadlineMs = -1; // SystemClock.uptimeMillis() + @GuardedBy("mLock") + private Handler mHandler; + + public AutoCorker(@NonNull String propertyName) { + this(propertyName, DEFAULT_AUTO_CORK_DELAY_MS); + } + + public AutoCorker(@NonNull String propertyName, int autoCorkDelayMs) { + mPropertyName = propertyName; + mAutoCorkDelayMs = autoCorkDelayMs; + // We can't initialize mHandler here: when we're created, the main loop might not + // be set up yet! Wait until we have a main loop to initialize our + // corking callback. + } + + public void autoCork() { + if (Looper.getMainLooper() == null) { + // We're not ready to auto-cork yet, so just invalidate the cache immediately. + if (DEBUG) { + Log.w(TAG, "invalidating instead of autocorking early in init: " + + mPropertyName); + } + PropertyInvalidatedCache.invalidateCache(mPropertyName); + return; + } + synchronized (mLock) { + boolean alreadyQueued = mUncorkDeadlineMs >= 0; + if (DEBUG) { + Log.w(TAG, String.format( + "autoCork mUncorkDeadlineMs=%s", mUncorkDeadlineMs)); + } + mUncorkDeadlineMs = SystemClock.uptimeMillis() + mAutoCorkDelayMs; + if (!alreadyQueued) { + getHandlerLocked().sendEmptyMessageAtTime(0, mUncorkDeadlineMs); + PropertyInvalidatedCache.corkInvalidations(mPropertyName); + } + } + } + + private void handleMessage(Message msg) { + synchronized (mLock) { + if (DEBUG) { + Log.w(TAG, String.format( + "handleMsesage mUncorkDeadlineMs=%s", mUncorkDeadlineMs)); + } + + if (mUncorkDeadlineMs < 0) { + return; // ??? + } + long nowMs = SystemClock.uptimeMillis(); + if (mUncorkDeadlineMs > nowMs) { + mUncorkDeadlineMs = nowMs + mAutoCorkDelayMs; + if (DEBUG) { + Log.w(TAG, String.format( + "scheduling uncork at %s", + mUncorkDeadlineMs)); + } + getHandlerLocked().sendEmptyMessageAtTime(0, mUncorkDeadlineMs); + return; + } + if (DEBUG) { + Log.w(TAG, "automatic uncorking " + mPropertyName); + } + mUncorkDeadlineMs = -1; + PropertyInvalidatedCache.uncorkInvalidations(mPropertyName); + } + } + + @GuardedBy("mLock") + private Handler getHandlerLocked() { + if (mHandler == null) { + mHandler = new Handler(Looper.getMainLooper()) { + @Override + public void handleMessage(Message msg) { + AutoCorker.this.handleMessage(msg); + } + }; + } + return mHandler; + } + } + protected Result maybeCheckConsistency(Query query, Result proposedResult) { if (VERIFY) { Result resultToCompare = recompute(query);