From 6b6992bf9b633b6e0c35048acd3eca8653c2ac34 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Mon, 24 Aug 2020 17:04:19 -0700 Subject: [PATCH] Cached list of pre-created users on UserManagerService. PermissionManagerService calls UserManagerService to get the list of user ids multiple times on boot (O(1000) times), so these calls must be cached. PMS used to call UMS.getUserIds(), which didn't include the list of pre-created users, so it was recently changed to call UMS.getUsers() and calculate the ids, but such non-cached call increase the boot time on some devices in almost 50ms. This change fixes the regression by caching the user ids of the pre-created users as well. Test: manual verification Bug: 167265128 Bug: 165940683 Change-Id: Ib4068cce7d4cea56e0937099508469e49e3412e2 Merged-In: Ib4068cce7d4cea56e0937099508469e49e3412e2 (cherry picked from commit d4407fd5304cd71f6a42d04f59c59d943efc427a) --- .../android/server/pm/UserManagerService.java | 70 ++++++++++++++++--- .../permission/PermissionManagerService.java | 14 +--- 2 files changed, 61 insertions(+), 23 deletions(-) diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 27924a68ff485..01a3d14e05dfa 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -114,7 +114,6 @@ import com.android.server.SystemService; import com.android.server.am.UserState; import com.android.server.storage.DeviceStorageMonitorInternal; import com.android.server.utils.TimingsTraceAndSlog; -import com.android.server.wm.ActivityTaskManagerInternal; import libcore.io.IoUtils; @@ -134,6 +133,7 @@ import java.io.OutputStream; import java.io.PrintWriter; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -406,6 +406,10 @@ public class UserManagerService extends IUserManager.Stub { @GuardedBy("mUsersLock") private int[] mUserIds; + + @GuardedBy("mUsersLock") + private int[] mUserIdsIncludingPreCreated; + @GuardedBy("mPackagesLock") private int mNextSerialNumber; private int mUserVersion = 0; @@ -2480,16 +2484,35 @@ public class UserManagerService extends IUserManager.Stub { } /** - * Returns an array of user ids. This array is cached here for quick access, so do not modify or - * cache it elsewhere. + * Returns an array of user ids. + * + *

This array is cached here for quick access, so do not modify or cache it elsewhere. + * * @return the array of user ids. */ - public int[] getUserIds() { + public @NonNull int[] getUserIds() { synchronized (mUsersLock) { return mUserIds; } } + /** + * Returns an array of user ids, including pre-created users. + * + *

This method should only used for the specific cases that need to handle pre-created users; + * most callers should call {@link #getUserIds()} instead. + * + *

This array is cached here for quick access, so do not modify or + * cache it elsewhere. + * + * @return the array of user ids. + */ + public @NonNull int[] getUserIdsIncludingPreCreated() { + synchronized (mUsersLock) { + return mUserIdsIncludingPreCreated; + } + } + @GuardedBy({"mRestrictionsLock", "mPackagesLock"}) private void readUserListLP() { if (!mUserListFile.exists()) { @@ -4327,23 +4350,43 @@ public class UserManagerService extends IUserManager.Stub { */ private void updateUserIds() { int num = 0; + int numIncludingPreCreated = 0; synchronized (mUsersLock) { final int userSize = mUsers.size(); for (int i = 0; i < userSize; i++) { - UserInfo userInfo = mUsers.valueAt(i).info; - if (!userInfo.partial && !userInfo.preCreated) { - num++; + final UserInfo userInfo = mUsers.valueAt(i).info; + if (!userInfo.partial) { + numIncludingPreCreated++; + if (!userInfo.preCreated) { + num++; + } } } + if (DBG) { + Slog.d(LOG_TAG, "updateUserIds(): numberUsers= " + num + + " includingPreCreated=" + numIncludingPreCreated); + } final int[] newUsers = new int[num]; + final int[] newUsersIncludingPreCreated = new int[numIncludingPreCreated]; + int n = 0; + int nIncludingPreCreated = 0; for (int i = 0; i < userSize; i++) { - UserInfo userInfo = mUsers.valueAt(i).info; - if (!userInfo.partial && !userInfo.preCreated) { - newUsers[n++] = mUsers.keyAt(i); + final UserInfo userInfo = mUsers.valueAt(i).info; + if (!userInfo.partial) { + final int userId = mUsers.keyAt(i); + newUsersIncludingPreCreated[nIncludingPreCreated++] = userId; + if (!userInfo.preCreated) { + newUsers[n++] = userId; + } } } mUserIds = newUsers; + mUserIdsIncludingPreCreated = newUsersIncludingPreCreated; + if (DBG) { + Slog.d(LOG_TAG, "updateUserIds(): userIds= " + Arrays.toString(mUserIds) + + " includingPreCreated=" + Arrays.toString(mUserIdsIncludingPreCreated)); + } } } @@ -4791,6 +4834,13 @@ public class UserManagerService extends IUserManager.Stub { synchronized (mUserStates) { pw.println(" Started users state: " + mUserStates); } + synchronized (mUsersLock) { + pw.print(" Cached user IDs: "); + pw.println(Arrays.toString(mUserIds)); + pw.print(" Cached user IDs (including pre-created): "); + pw.println(Arrays.toString(mUserIdsIncludingPreCreated)); + } + } // synchronized (mPackagesLock) // Dump some capabilities 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 a4e8b1c016208..2533aa7c43021 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -83,7 +83,6 @@ import android.content.pm.PackageParser; import android.content.pm.ParceledListSlice; import android.content.pm.PermissionGroupInfo; import android.content.pm.PermissionInfo; -import android.content.pm.UserInfo; import android.content.pm.parsing.component.ParsedPermission; import android.content.pm.parsing.component.ParsedPermissionGroup; import android.content.pm.permission.SplitPermissionInfoParcelable; @@ -121,7 +120,6 @@ import android.util.Log; import android.util.Slog; import android.util.SparseArray; import android.util.SparseBooleanArray; -import android.util.TimingsTraceLog; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; @@ -3078,17 +3076,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { * @return user ids for created users and pre-created users */ private int[] getAllUserIds() { - final TimingsTraceLog t = new TimingsTraceLog(TAG, Trace.TRACE_TAG_SYSTEM_SERVER); - t.traceBegin("getAllUserIds"); - List users = UserManagerService.getInstance().getUsers( - /*excludePartial=*/ true, /*excludeDying=*/ true, /*excludePreCreated=*/ false); - int size = users.size(); - final int[] userIds = new int[size]; - for (int i = 0; i < size; i++) { - userIds[i] = users.get(i).id; - } - t.traceEnd(); - return userIds; + return UserManagerService.getInstance().getUserIdsIncludingPreCreated(); } /**