From d78fd7bf507d4d3606b71e52c1525a9ccd632631 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Thu, 14 Oct 2021 19:00:57 +0800 Subject: [PATCH] Only update cache for new user on user create #2 - Snapshots whole cache and enlarge the capacity to accommodate the new user. Removes the logic of copying over the prior cached values for the other users to improve the user creation performance. This CL focus on the improvements of the creation of multiple users such as the fourth, fifth or sixth user. No obvious improvement for the first user creation. The execution time of AppsFilter#onUserCreated got 60% improved in the creation of sixth user compared with rebuilding whole cache. - Handle the user delete event to remove the cached values of the invalid user. Test: atest --user-type secondary_user AppsFilterTest Test: atest --user-type secondary_user AppEnumerationTests Test: [create 2nd user; run as primary] atest AppsFilterTest AppEnumerationTests Test: atest UserLifecycleTests#createUser Bug: 187853334 Change-Id: Iab93083509093ac3225adf0df466154f86e95bf2 --- .../com/android/server/pm/AppsFilter.java | 62 +++++++++++++------ .../server/pm/PackageManagerService.java | 1 + .../com/android/server/pm/AppsFilterTest.java | 19 +++++- 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java index afb91dad700c2..d3a5c6a2d1dfe 100644 --- a/services/core/java/com/android/server/pm/AppsFilter.java +++ b/services/core/java/com/android/server/pm/AppsFilter.java @@ -26,6 +26,7 @@ import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIV import android.Manifest; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.UserIdInt; import android.content.Intent; import android.content.IntentFilter; import android.content.pm.PackageManager; @@ -874,24 +875,6 @@ public class AppsFilter implements Watchable, Snappable { WatchedSparseBooleanMatrix cache = updateEntireShouldFilterCacheInner(settings, users, userId); synchronized (mCacheLock) { - if (userId != USER_ALL) { - // if we're only updating a single user id, we need to copy over the prior - // cached values for the other users. - int[] uids = mShouldFilterCache.keys(); - for (int i = 0; i < uids.length; i++) { - int uid1 = uids[i]; - if (UserHandle.getUserId(uid1) == userId) { - continue; - } - for (int j = 0; j < uids.length; j++) { - int uid2 = uids[j]; - if (UserHandle.getUserId(uid2) == userId) { - continue; - } - cache.put(uid1, uid2, mShouldFilterCache.get(uid1, uid2)); - } - } - } mShouldFilterCache = cache; } }); @@ -899,8 +882,15 @@ public class AppsFilter implements Watchable, Snappable { private WatchedSparseBooleanMatrix updateEntireShouldFilterCacheInner( ArrayMap settings, UserInfo[] users, int subjectUserId) { - WatchedSparseBooleanMatrix cache = - new WatchedSparseBooleanMatrix(users.length * settings.size()); + final WatchedSparseBooleanMatrix cache; + if (subjectUserId == USER_ALL) { + cache = new WatchedSparseBooleanMatrix(users.length * settings.size()); + } else { + synchronized (mCacheLock) { + cache = mShouldFilterCache.snapshot(); + } + cache.setCapacity(users.length * settings.size()); + } for (int i = settings.size() - 1; i >= 0; i--) { updateShouldFilterCacheForPackage(cache, null /*skipPackage*/, settings.valueAt(i), settings, users, subjectUserId, i); @@ -964,6 +954,15 @@ public class AppsFilter implements Watchable, Snappable { } } + public void onUserDeleted(@UserIdInt int userId) { + synchronized (mCacheLock) { + if (mShouldFilterCache != null) { + removeShouldFilterCacheForUser(userId); + onChanged(); + } + } + } + private void updateShouldFilterCacheForPackage(String packageName) { mStateProvider.runWithState((settings, users) -> { synchronized (mCacheLock) { @@ -1018,6 +1017,29 @@ public class AppsFilter implements Watchable, Snappable { } } + @GuardedBy("mCacheLock") + private void removeShouldFilterCacheForUser(int userId) { + // Sorted uids with the ascending order + final int[] cacheUids = mShouldFilterCache.keys(); + final int size = cacheUids.length; + int pos = Arrays.binarySearch(cacheUids, UserHandle.getUid(userId, 0)); + final int fromIndex = (pos >= 0 ? pos : ~pos); + if (fromIndex >= size || UserHandle.getUserId(cacheUids[fromIndex]) != userId) { + Slog.w(TAG, "Failed to remove should filter cache for user " + userId + + ", fromIndex=" + fromIndex); + return; + } + pos = Arrays.binarySearch(cacheUids, UserHandle.getUid(userId + 1, 0) - 1); + final int toIndex = (pos >= 0 ? pos + 1 : ~pos); + if (fromIndex >= toIndex || UserHandle.getUserId(cacheUids[toIndex - 1]) != userId) { + Slog.w(TAG, "Failed to remove should filter cache for user " + userId + + ", fromIndex=" + fromIndex + ", toIndex=" + toIndex); + return; + } + mShouldFilterCache.removeRange(fromIndex, toIndex); + mShouldFilterCache.compact(); + } + private static boolean isSystemSigned(@NonNull SigningDetails sysSigningDetails, PackageSetting pkgSetting) { return pkgSetting.isSystem() diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 0b2fe3d4337d2..498485f5525c3 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -9279,6 +9279,7 @@ public class PackageManagerService extends IPackageManager.Stub mPendingBroadcasts.remove(userId); mInstantAppRegistry.onUserRemovedLPw(userId); mDeletePackageHelper.removeUnusedPackagesLPw(userManager, userId); + mAppsFilter.onUserDeleted(userId); } } diff --git a/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java b/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java index db1082faa92e4..b7161f8672d51 100644 --- a/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java @@ -326,7 +326,7 @@ public class AppsFilterTest { } @Test - public void testOnUserCreated_FilterMatches() throws Exception { + public void testOnUserUpdated_FilterMatches() throws Exception { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); @@ -364,6 +364,23 @@ public class AppsFilterTest { otherUserId)); } } + + // delete user + doAnswer(invocation -> { + ((AppsFilter.StateProvider.CurrentStateCallback) invocation.getArgument(0)) + .currentState(mExisting, USER_INFO_LIST); + return new Object(); + }).when(mStateProvider) + .runWithState(any(AppsFilter.StateProvider.CurrentStateCallback.class)); + appsFilter.onUserDeleted(ADDED_USER); + + for (int subjectUserId : USER_ARRAY) { + for (int otherUserId : USER_ARRAY) { + assertFalse(appsFilter.shouldFilterApplication( + UserHandle.getUid(DUMMY_CALLING_APPID, subjectUserId), calling, target, + otherUserId)); + } + } } @Test