From adde444c3f2e33ec85d3df213348aacbaab07819 Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Mon, 1 Nov 2021 21:20:04 +0000 Subject: [PATCH] Revert "Exempt hibernating apps from dex optimization" This reverts commit 633d8e12c15128fde3b41d6139273fdfd7625716. Reason for revert: Test breakage b/204803333 Change-Id: I9adbd5576af8c3619614d438106aa8e8da0536b9 --- .../AppHibernationManagerInternal.java | 5 -- .../apphibernation/AppHibernationService.java | 13 ----- .../com/android/server/pm/DexOptHelper.java | 11 ++++- .../android/server/pm/OtaDexoptService.java | 2 +- .../server/pm/PackageDexOptimizer.java | 49 ++----------------- .../PackageManagerServiceHibernationTests.kt | 22 --------- 6 files changed, 15 insertions(+), 87 deletions(-) diff --git a/services/core/java/com/android/server/apphibernation/AppHibernationManagerInternal.java b/services/core/java/com/android/server/apphibernation/AppHibernationManagerInternal.java index a3c9612e0e2c8..b0335fe404f42 100644 --- a/services/core/java/com/android/server/apphibernation/AppHibernationManagerInternal.java +++ b/services/core/java/com/android/server/apphibernation/AppHibernationManagerInternal.java @@ -43,9 +43,4 @@ public abstract class AppHibernationManagerInternal { * @see AppHibernationService#setHibernatingGlobally */ public abstract void setHibernatingGlobally(String packageName, boolean isHibernating); - - /** - * @see AppHibernationService#isOatArtifactDeletionEnabled - */ - public abstract boolean isOatArtifactDeletionEnabled(); } diff --git a/services/core/java/com/android/server/apphibernation/AppHibernationService.java b/services/core/java/com/android/server/apphibernation/AppHibernationService.java index 4d025c981ce94..bd066ff848c19 100644 --- a/services/core/java/com/android/server/apphibernation/AppHibernationService.java +++ b/services/core/java/com/android/server/apphibernation/AppHibernationService.java @@ -199,14 +199,6 @@ public final class AppHibernationService extends SystemService { } } - /** - * Whether global hibernation should delete ART ahead-of-time compilation artifacts and prevent - * package manager from re-optimizing the APK. - */ - private boolean isOatArtifactDeletionEnabled() { - return mOatArtifactDeletionEnabled; - } - /** * Whether a package is hibernating for a given user. * @@ -738,11 +730,6 @@ public final class AppHibernationService extends SystemService { public boolean isHibernatingGlobally(String packageName) { return mService.isHibernatingGlobally(packageName); } - - @Override - public boolean isOatArtifactDeletionEnabled() { - return mService.isOatArtifactDeletionEnabled(); - } } private final AppHibernationServiceStub mServiceStub = new AppHibernationServiceStub(this); diff --git a/services/core/java/com/android/server/pm/DexOptHelper.java b/services/core/java/com/android/server/pm/DexOptHelper.java index 5c3a991f70046..939028476cd6e 100644 --- a/services/core/java/com/android/server/pm/DexOptHelper.java +++ b/services/core/java/com/android/server/pm/DexOptHelper.java @@ -49,6 +49,8 @@ import android.util.Slog; import com.android.internal.R; import com.android.internal.annotations.GuardedBy; import com.android.internal.logging.MetricsLogger; +import com.android.server.apphibernation.AppHibernationManagerInternal; +import com.android.server.apphibernation.AppHibernationService; import com.android.server.pm.dex.DexManager; import com.android.server.pm.dex.DexoptOptions; import com.android.server.pm.parsing.pkg.AndroidPackage; @@ -169,7 +171,7 @@ final class DexOptHelper { } } - if (!mPm.mPackageDexOptimizer.canOptimizePackage(pkg)) { + if (!PackageDexOptimizer.canOptimizePackage(pkg)) { if (DEBUG_DEXOPT) { Log.i(TAG, "Skipping update of non-optimizable app " + pkg.getPackageName()); } @@ -289,11 +291,16 @@ final class DexOptHelper { ArraySet pkgs = new ArraySet<>(); synchronized (mPm.mLock) { for (AndroidPackage p : mPm.mPackages.values()) { - if (mPm.mPackageDexOptimizer.canOptimizePackage(p)) { + if (PackageDexOptimizer.canOptimizePackage(p)) { pkgs.add(p.getPackageName()); } } } + if (AppHibernationService.isAppHibernationEnabled()) { + AppHibernationManagerInternal appHibernationManager = + mPm.mInjector.getLocalService(AppHibernationManagerInternal.class); + pkgs.removeIf(pkgName -> appHibernationManager.isHibernatingGlobally(pkgName)); + } return pkgs; } diff --git a/services/core/java/com/android/server/pm/OtaDexoptService.java b/services/core/java/com/android/server/pm/OtaDexoptService.java index 9122221f726ce..68801d67359fe 100644 --- a/services/core/java/com/android/server/pm/OtaDexoptService.java +++ b/services/core/java/com/android/server/pm/OtaDexoptService.java @@ -387,7 +387,7 @@ public class OtaDexoptService extends IOtaDexopt.Stub { } // Does the package have code? If not, there won't be any artifacts. - if (!mPackageManagerService.mPackageDexOptimizer.canOptimizePackage(pkg)) { + if (!PackageDexOptimizer.canOptimizePackage(pkg)) { continue; } if (pkg.getPath() == null) { diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java index cac1978a83ec8..7739f2ff6e5b1 100644 --- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java +++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java @@ -64,10 +64,7 @@ import android.util.Slog; import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; -import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.IndentingPrintWriter; -import com.android.server.LocalServices; -import com.android.server.apphibernation.AppHibernationManagerInternal; import com.android.server.pm.Installer.InstallerException; import com.android.server.pm.dex.ArtManagerService; import com.android.server.pm.dex.ArtStatsLogUtils; @@ -137,24 +134,16 @@ public class PackageDexOptimizer { private volatile boolean mSystemReady; private final ArtStatsLogger mArtStatsLogger = new ArtStatsLogger(); - private final Injector mInjector; - private static final Random sRandom = new Random(); PackageDexOptimizer(Installer installer, Object installLock, Context context, String wakeLockTag) { - this(new Injector() { - @Override - public AppHibernationManagerInternal getAppHibernationManagerInternal() { - return LocalServices.getService(AppHibernationManagerInternal.class); - } + this.mInstaller = installer; + this.mInstallLock = installLock; - @Override - public PowerManager getPowerManager(Context context) { - return context.getSystemService(PowerManager.class); - } - }, installer, installLock, context, wakeLockTag); + PowerManager powerManager = (PowerManager)context.getSystemService(Context.POWER_SERVICE); + mDexoptWakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, wakeLockTag); } protected PackageDexOptimizer(PackageDexOptimizer from) { @@ -162,21 +151,9 @@ public class PackageDexOptimizer { this.mInstallLock = from.mInstallLock; this.mDexoptWakeLock = from.mDexoptWakeLock; this.mSystemReady = from.mSystemReady; - this.mInjector = from.mInjector; } - @VisibleForTesting - PackageDexOptimizer(@NonNull Injector injector, Installer installer, Object installLock, - Context context, String wakeLockTag) { - this.mInstaller = installer; - this.mInstallLock = installLock; - - PowerManager powerManager = injector.getPowerManager(context); - mDexoptWakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, wakeLockTag); - mInjector = injector; - } - - boolean canOptimizePackage(AndroidPackage pkg) { + static boolean canOptimizePackage(AndroidPackage pkg) { // We do not dexopt a package with no code. // Note that the system package is marked as having no code, however we can // still optimize it via dexoptSystemServerPath. @@ -184,13 +161,6 @@ public class PackageDexOptimizer { return false; } - // We do not dexopt unused packages. - AppHibernationManagerInternal ahm = mInjector.getAppHibernationManagerInternal(); - if (ahm.isHibernatingGlobally(pkg.getPackageName()) - && ahm.isOatArtifactDeletionEnabled()) { - return false; - } - return true; } @@ -1030,13 +1000,4 @@ public class PackageDexOptimizer { private Installer getInstallerWithoutLock() { return mInstaller; } - - /** - * Injector for {@link PackageDexOptimizer} dependencies - */ - interface Injector { - AppHibernationManagerInternal getAppHibernationManagerInternal(); - - PowerManager getPowerManager(Context context); - } } diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt b/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt index 15acdac592e9a..3ee23480dfc0b 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt +++ b/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt @@ -16,10 +16,8 @@ package com.android.server.pm -import android.content.Context import android.os.Build import android.os.Handler -import android.os.PowerManager import android.provider.DeviceConfig import android.provider.DeviceConfig.NAMESPACE_APP_HIBERNATION import android.testing.AndroidTestingRunner @@ -57,8 +55,6 @@ class PackageManagerServiceHibernationTests { @Mock lateinit var appHibernationManager: AppHibernationManagerInternal - @Mock - lateinit var powerManager: PowerManager @Before @Throws(Exception::class) @@ -72,24 +68,6 @@ class PackageManagerServiceHibernationTests { .thenReturn(appHibernationManager) whenever(rule.mocks().injector.handler) .thenReturn(Handler(TestableLooper.get(this).looper)) - val injector = object : PackageDexOptimizer.Injector { - override fun getAppHibernationManagerInternal(): AppHibernationManagerInternal { - return appHibernationManager - } - - override fun getPowerManager(context: Context?): PowerManager { - return powerManager - } - } - val packageDexOptimizer = PackageDexOptimizer( - injector, - rule.mocks().installer, - rule.mocks().installLock, - rule.mocks().context, - "*dexopt*") - whenever(rule.mocks().injector.packageDexOptimizer) - .thenReturn(packageDexOptimizer) - whenever(appHibernationManager.isOatArtifactDeletionEnabled).thenReturn(true) } @Test