From 487e16c4af84b3e817015f95b6bb6a124f57fce0 Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Tue, 9 Mar 2021 18:33:47 -0800 Subject: Move disk reads to background Move disk reading to background to improve performance on user unlocking. Bug: 182098676 Test: atest AppHibernationServiceTest Change-Id: I087bd9d29f4f90c818a03648ee75e3cbfb2ded41 --- .../apphibernation/AppHibernationService.java | 76 +++++++++++++++------- .../apphibernation/HibernationStateDiskStore.java | 1 + .../apphibernation/AppHibernationServiceTest.java | 13 +++- 3 files changed, 63 insertions(+), 27 deletions(-) (limited to 'services') diff --git a/services/core/java/com/android/server/apphibernation/AppHibernationService.java b/services/core/java/com/android/server/apphibernation/AppHibernationService.java index b3373d0bb536..351231f34c4b 100644 --- a/services/core/java/com/android/server/apphibernation/AppHibernationService.java +++ b/services/core/java/com/android/server/apphibernation/AppHibernationService.java @@ -69,6 +69,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -101,6 +102,7 @@ public final class AppHibernationService extends SystemService { private final Map mGlobalHibernationStates = new ArrayMap<>(); private final HibernationStateDiskStore mGlobalLevelHibernationDiskStore; private final Injector mInjector; + private final Executor mBackgroundExecutor; @VisibleForTesting boolean mIsServiceEnabled; @@ -126,6 +128,7 @@ public final class AppHibernationService extends SystemService { mIActivityManager = injector.getActivityManager(); mUserManager = injector.getUserManager(); mGlobalLevelHibernationDiskStore = injector.getGlobalLevelDiskStore(); + mBackgroundExecutor = injector.getBackgroundExecutor(); mInjector = injector; final Context userAllContext = mContext.createContextAsUser(UserHandle.ALL, 0 /* flags */); @@ -147,11 +150,13 @@ public final class AppHibernationService extends SystemService { @Override public void onBootPhase(int phase) { if (phase == PHASE_BOOT_COMPLETED) { - List states = - mGlobalLevelHibernationDiskStore.readHibernationStates(); - synchronized (mLock) { - initializeGlobalHibernationStates(states); - } + mBackgroundExecutor.execute(() -> { + List states = + mGlobalLevelHibernationDiskStore.readHibernationStates(); + synchronized (mLock) { + initializeGlobalHibernationStates(states); + } + }); } if (phase == SystemService.PHASE_SYSTEM_SERVICES_READY) { mIsServiceEnabled = isAppHibernationEnabled(); @@ -170,16 +175,15 @@ public final class AppHibernationService extends SystemService { * @return true if package is hibernating for the user */ boolean isHibernatingForUser(String packageName, int userId) { - if (!checkHibernationEnabled("isHibernatingForUser")) { + String methodName = "isHibernatingForUser"; + if (!checkHibernationEnabled(methodName)) { return false; } getContext().enforceCallingOrSelfPermission( android.Manifest.permission.MANAGE_APP_HIBERNATION, "Caller does not have MANAGE_APP_HIBERNATION permission."); - userId = handleIncomingUser(userId, "isHibernating"); - if (!mUserManager.isUserUnlockingOrUnlocked(userId)) { - Slog.e(TAG, "Attempt to get hibernation state of stopped or nonexistent user " - + userId); + userId = handleIncomingUser(userId, methodName); + if (!checkUserStatesExist(userId, methodName)) { return false; } synchronized (mLock) { @@ -225,16 +229,15 @@ public final class AppHibernationService extends SystemService { * @param isHibernating new hibernation state */ void setHibernatingForUser(String packageName, int userId, boolean isHibernating) { - if (!checkHibernationEnabled("setHibernatingForUser")) { + String methodName = "setHibernatingForUser"; + if (!checkHibernationEnabled(methodName)) { return; } getContext().enforceCallingOrSelfPermission( android.Manifest.permission.MANAGE_APP_HIBERNATION, "Caller does not have MANAGE_APP_HIBERNATION permission."); - userId = handleIncomingUser(userId, "setHibernating"); - if (!mUserManager.isUserUnlockingOrUnlocked(userId)) { - Slog.w(TAG, "Attempt to set hibernation state for a stopped or nonexistent user " - + userId); + userId = handleIncomingUser(userId, methodName); + if (!checkUserStatesExist(userId, methodName)) { return; } synchronized (mLock) { @@ -298,16 +301,15 @@ public final class AppHibernationService extends SystemService { */ @NonNull List getHibernatingPackagesForUser(int userId) { ArrayList hibernatingPackages = new ArrayList<>(); - if (!checkHibernationEnabled("getHibernatingPackagesForUser")) { + String methodName = "getHibernatingPackagesForUser"; + if (!checkHibernationEnabled(methodName)) { return hibernatingPackages; } getContext().enforceCallingOrSelfPermission( android.Manifest.permission.MANAGE_APP_HIBERNATION, "Caller does not have MANAGE_APP_HIBERNATION permission."); - userId = handleIncomingUser(userId, "getHibernatingPackagesForUser"); - if (!mUserManager.isUserUnlockingOrUnlocked(userId)) { - Slog.w(TAG, "Attempt to get hibernating packages for a stopped or nonexistent user " - + userId); + userId = handleIncomingUser(userId, methodName); + if (!checkUserStatesExist(userId, methodName)) { return hibernatingPackages; } synchronized (mLock) { @@ -477,10 +479,15 @@ public final class AppHibernationService extends SystemService { HibernationStateDiskStore diskStore = mInjector.getUserLevelDiskStore(userId); mUserDiskStores.put(userId, diskStore); - List storedStates = diskStore.readHibernationStates(); - synchronized (mLock) { - initializeUserHibernationStates(userId, storedStates); - } + mBackgroundExecutor.execute(() -> { + List storedStates = diskStore.readHibernationStates(); + synchronized (mLock) { + // Ensure user hasn't stopped in the time to execute. + if (mUserManager.isUserUnlockingOrUnlocked(userId)) { + initializeUserHibernationStates(userId, storedStates); + } + } + }); } @Override @@ -550,6 +557,20 @@ public final class AppHibernationService extends SystemService { } } + private boolean checkUserStatesExist(int userId, String methodName) { + if (!mUserManager.isUserUnlockingOrUnlocked(userId)) { + Slog.e(TAG, String.format( + "Attempt to call %s on stopped or nonexistent user %d", methodName, userId)); + return false; + } + if (!mUserStates.contains(userId)) { + Slog.w(TAG, String.format( + "Attempt to call %s before states have been read from disk", methodName)); + return false; + } + return true; + } + private boolean checkHibernationEnabled(String methodName) { if (!mIsServiceEnabled) { Slog.w(TAG, String.format("Attempted to call %s on unsupported device.", methodName)); @@ -720,6 +741,8 @@ public final class AppHibernationService extends SystemService { UserManager getUserManager(); + Executor getBackgroundExecutor(); + HibernationStateDiskStore getGlobalLevelDiskStore(); HibernationStateDiskStore getUserLevelDiskStore(int userId); @@ -757,6 +780,11 @@ public final class AppHibernationService extends SystemService { return mContext.getSystemService(UserManager.class); } + @Override + public Executor getBackgroundExecutor() { + return mScheduledExecutorService; + } + @Override public HibernationStateDiskStore getGlobalLevelDiskStore() { File dir = new File(Environment.getDataSystemDirectory(), HIBERNATION_DIR_NAME); diff --git a/services/core/java/com/android/server/apphibernation/HibernationStateDiskStore.java b/services/core/java/com/android/server/apphibernation/HibernationStateDiskStore.java index c83659d2ff56..24cf43339847 100644 --- a/services/core/java/com/android/server/apphibernation/HibernationStateDiskStore.java +++ b/services/core/java/com/android/server/apphibernation/HibernationStateDiskStore.java @@ -109,6 +109,7 @@ class HibernationStateDiskStore { * @return the parsed list of hibernation states, null if file does not exist */ @Nullable + @WorkerThread List readHibernationStates() { synchronized (this) { if (!mHibernationFile.exists()) { diff --git a/services/tests/servicestests/src/com/android/server/apphibernation/AppHibernationServiceTest.java b/services/tests/servicestests/src/com/android/server/apphibernation/AppHibernationServiceTest.java index 1b8ab2175458..2f0d71a2a579 100644 --- a/services/tests/servicestests/src/com/android/server/apphibernation/AppHibernationServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/apphibernation/AppHibernationServiceTest.java @@ -58,6 +58,7 @@ import org.mockito.MockitoAnnotations; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.Executor; /** * Tests for {@link com.android.server.apphibernation.AppHibernationService} @@ -116,8 +117,8 @@ public final class AppHibernationServiceTest { mAppHibernationService.onBootPhase(SystemService.PHASE_BOOT_COMPLETED); UserInfo userInfo = addUser(USER_ID_1); - mAppHibernationService.onUserUnlocking(new SystemService.TargetUser(userInfo)); doReturn(true).when(mUserManager).isUserUnlockingOrUnlocked(USER_ID_1); + mAppHibernationService.onUserUnlocking(new SystemService.TargetUser(userInfo)); mAppHibernationService.mIsServiceEnabled = true; } @@ -150,8 +151,8 @@ public final class AppHibernationServiceTest { throws RemoteException { // WHEN a new user is added and a package from the user is hibernated UserInfo user2 = addUser(USER_ID_2); - mAppHibernationService.onUserUnlocking(new SystemService.TargetUser(user2)); doReturn(true).when(mUserManager).isUserUnlockingOrUnlocked(USER_ID_2); + mAppHibernationService.onUserUnlocking(new SystemService.TargetUser(user2)); mAppHibernationService.setHibernatingForUser(PACKAGE_NAME_1, USER_ID_2, true); // THEN the new user's package is hibernated @@ -188,8 +189,8 @@ public final class AppHibernationServiceTest { // GIVEN an unlocked user with all packages installed UserInfo userInfo = addUser(USER_ID_2, new String[]{PACKAGE_NAME_1, PACKAGE_NAME_2, PACKAGE_NAME_3}); - mAppHibernationService.onUserUnlocking(new SystemService.TargetUser(userInfo)); doReturn(true).when(mUserManager).isUserUnlockingOrUnlocked(USER_ID_2); + mAppHibernationService.onUserUnlocking(new SystemService.TargetUser(userInfo)); // WHEN packages are hibernated for the user mAppHibernationService.setHibernatingForUser(PACKAGE_NAME_1, USER_ID_2, true); @@ -258,6 +259,12 @@ public final class AppHibernationServiceTest { return mUserManager; } + @Override + public Executor getBackgroundExecutor() { + // Just execute immediately in tests. + return r -> r.run(); + } + @Override public HibernationStateDiskStore getGlobalLevelDiskStore() { return Mockito.mock(HibernationStateDiskStore.class); -- cgit v1.2.3