From 9754ebd64166fd584bd285f8fb3035ed64439a28 Mon Sep 17 00:00:00 2001 From: Martin Stjernholm Date: Tue, 18 Oct 2022 22:07:22 +0100 Subject: [PATCH] Turn profile updates on unconditionally for profile guided compilations. Align with ART Service behaviour, which will not provide an option to skip the check on profile files. Test: m Test: adb shell cmd package compile --check-prof false \ -m speed com.google.android.dialer Bug: 251903639 Change-Id: If93aab863e80e2285f03266641c9e38965591f45 --- .../server/pm/BackgroundDexOptService.java | 42 +++++++++++---- .../com/android/server/pm/DexOptHelper.java | 51 ++++++++++++------- .../server/pm/IPackageManagerBase.java | 9 +++- .../server/pm/PackageManagerShellCommand.java | 27 +++------- .../android/server/pm/dex/DexoptOptions.java | 15 +++--- 5 files changed, 88 insertions(+), 56 deletions(-) diff --git a/services/core/java/com/android/server/pm/BackgroundDexOptService.java b/services/core/java/com/android/server/pm/BackgroundDexOptService.java index d72aacc0db740..dd41830243149 100644 --- a/services/core/java/com/android/server/pm/BackgroundDexOptService.java +++ b/services/core/java/com/android/server/pm/BackgroundDexOptService.java @@ -17,8 +17,11 @@ package com.android.server.pm; import static com.android.server.pm.PackageManagerService.PLATFORM_PACKAGE_NAME; +import static com.android.server.pm.PackageManagerServiceCompilerMapping.getCompilerFilterForReason; import static com.android.server.pm.dex.ArtStatsLogUtils.BackgroundDexoptJobStatsLogger; +import static dalvik.system.DexFile.isProfileGuidedCompilerFilter; + import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; @@ -748,10 +751,21 @@ public final class BackgroundDexOptService { return PackageDexOptimizer.DEX_OPT_CANCELLED; } int reason = PackageManagerService.REASON_INACTIVE_PACKAGE_DOWNGRADE; + String filter = getCompilerFilterForReason(reason); int dexoptFlags = DexoptOptions.DEXOPT_BOOT_COMPLETE | DexoptOptions.DEXOPT_DOWNGRADE; + + if (isProfileGuidedCompilerFilter(filter)) { + // We don't expect updates in current profiles to be significant here, but + // DEXOPT_CHECK_FOR_PROFILES_UPDATES is set to replicate behaviour that will be + // unconditionally enabled for profile guided filters when ART Service is called instead + // of the legacy PackageDexOptimizer implementation. + dexoptFlags |= DexoptOptions.DEXOPT_CHECK_FOR_PROFILES_UPDATES; + } + if (!isPostBootUpdate) { dexoptFlags |= DexoptOptions.DEXOPT_IDLE_BACKGROUND_JOB; } + long package_size_before = getPackageSize(snapshot, pkg); int result = PackageDexOptimizer.DEX_OPT_SKIPPED; if (isForPrimaryDex || PLATFORM_PACKAGE_NAME.equals(pkg)) { @@ -762,10 +776,10 @@ public final class BackgroundDexOptService { // remove their compiler artifacts from dalvik cache. pm.deleteOatArtifactsOfPackage(snapshot, pkg); } else { - result = performDexOptPrimary(pkg, reason, dexoptFlags); + result = performDexOptPrimary(pkg, reason, filter, dexoptFlags); } } else { - result = performDexOptSecondary(pkg, reason, dexoptFlags); + result = performDexOptSecondary(pkg, reason, filter, dexoptFlags); } if (result == PackageDexOptimizer.DEX_OPT_PERFORMED) { @@ -801,32 +815,42 @@ public final class BackgroundDexOptService { private int optimizePackage(String pkg, boolean isForPrimaryDex, boolean isPostBootUpdate) { int reason = isPostBootUpdate ? PackageManagerService.REASON_POST_BOOT : PackageManagerService.REASON_BACKGROUND_DEXOPT; + String filter = getCompilerFilterForReason(reason); + int dexoptFlags = DexoptOptions.DEXOPT_BOOT_COMPLETE; if (!isPostBootUpdate) { dexoptFlags |= DexoptOptions.DEXOPT_CHECK_FOR_PROFILES_UPDATES | DexoptOptions.DEXOPT_IDLE_BACKGROUND_JOB; } + if (isProfileGuidedCompilerFilter(filter)) { + // Ensure DEXOPT_CHECK_FOR_PROFILES_UPDATES is enabled if the filter is profile guided, + // to replicate behaviour that will be unconditionally enabled when ART Service is + // called instead of the legacy PackageDexOptimizer implementation. + dexoptFlags |= DexoptOptions.DEXOPT_CHECK_FOR_PROFILES_UPDATES; + } + // System server share the same code path as primary dex files. // PackageManagerService will select the right optimization path for it. if (isForPrimaryDex || PLATFORM_PACKAGE_NAME.equals(pkg)) { - return performDexOptPrimary(pkg, reason, dexoptFlags); + return performDexOptPrimary(pkg, reason, filter, dexoptFlags); } else { - return performDexOptSecondary(pkg, reason, dexoptFlags); + return performDexOptSecondary(pkg, reason, filter, dexoptFlags); } } @DexOptResult - private int performDexOptPrimary(String pkg, int reason, int dexoptFlags) { - DexoptOptions dexoptOptions = new DexoptOptions(pkg, reason, dexoptFlags); + private int performDexOptPrimary(String pkg, int reason, String filter, int dexoptFlags) { + DexoptOptions dexoptOptions = + new DexoptOptions(pkg, reason, filter, /*splitName=*/null, dexoptFlags); return trackPerformDexOpt(pkg, /*isForPrimaryDex=*/true, () -> mDexOptHelper.performDexOptWithStatus(dexoptOptions)); } @DexOptResult - private int performDexOptSecondary(String pkg, int reason, int dexoptFlags) { - DexoptOptions dexoptOptions = new DexoptOptions( - pkg, reason, dexoptFlags | DexoptOptions.DEXOPT_ONLY_SECONDARY_DEX); + private int performDexOptSecondary(String pkg, int reason, String filter, int dexoptFlags) { + DexoptOptions dexoptOptions = new DexoptOptions(pkg, reason, filter, /*splitName=*/null, + dexoptFlags | DexoptOptions.DEXOPT_ONLY_SECONDARY_DEX); return trackPerformDexOpt(pkg, /*isForPrimaryDex=*/false, () -> mDexOptHelper.performDexOpt(dexoptOptions) diff --git a/services/core/java/com/android/server/pm/DexOptHelper.java b/services/core/java/com/android/server/pm/DexOptHelper.java index c4f6836eba7ba..427cc852da42b 100644 --- a/services/core/java/com/android/server/pm/DexOptHelper.java +++ b/services/core/java/com/android/server/pm/DexOptHelper.java @@ -33,6 +33,8 @@ import static com.android.server.pm.PackageManagerServiceCompilerMapping.getDefa import static com.android.server.pm.PackageManagerServiceUtils.REMOVE_IF_APEX_PKG; import static com.android.server.pm.PackageManagerServiceUtils.REMOVE_IF_NULL_PKG; +import static dalvik.system.DexFile.isProfileGuidedCompilerFilter; + import android.Manifest; import android.annotation.NonNull; import android.annotation.Nullable; @@ -70,8 +72,6 @@ import com.android.server.pm.pkg.AndroidPackage; import com.android.server.pm.pkg.PackageState; import com.android.server.pm.pkg.PackageStateInternal; -import dalvik.system.DexFile; - import java.io.File; import java.nio.file.Path; import java.nio.file.Paths; @@ -236,20 +236,24 @@ public final class DexOptHelper { mPm.mArtManagerService.compileLayouts(pkg); } - // checkProfiles is false to avoid merging profiles during boot which - // might interfere with background compilation (b/28612421). - // Unfortunately this will also means that "pm.dexopt.boot=speed-profile" will - // behave differently than "pm.dexopt.bg-dexopt=speed-profile" but that's a - // trade-off worth doing to save boot time work. int dexoptFlags = bootComplete ? DexoptOptions.DEXOPT_BOOT_COMPLETE : 0; + + String filter = getCompilerFilterForReason(pkgCompilationReason); + if (isProfileGuidedCompilerFilter(filter)) { + // DEXOPT_CHECK_FOR_PROFILES_UPDATES used to be false to avoid merging profiles + // during boot which might interfere with background compilation (b/28612421). + // However those problems were related to the verify-profile compiler filter which + // doesn't exist any more, so enable it again. + dexoptFlags |= DexoptOptions.DEXOPT_CHECK_FOR_PROFILES_UPDATES; + } + if (compilationReason == REASON_FIRST_BOOT) { // TODO: This doesn't cover the upgrade case, we should check for this too. dexoptFlags |= DexoptOptions.DEXOPT_INSTALL_WITH_DEX_METADATA_FILE; } - int primaryDexOptStatus = performDexOptTraced(new DexoptOptions( - pkg.getPackageName(), - pkgCompilationReason, - dexoptFlags)); + int primaryDexOptStatus = performDexOptTraced( + new DexoptOptions(pkg.getPackageName(), pkgCompilationReason, filter, + /*splitName*/ null, dexoptFlags)); switch (primaryDexOptStatus) { case PackageDexOptimizer.DEX_OPT_PERFORMED: @@ -297,7 +301,7 @@ public final class DexOptHelper { SystemProperties.get("dalvik.vm.systemuicompilerfilter", defaultCompilerFilter); String compilerFilter; - if (DexFile.isProfileGuidedCompilerFilter(targetCompilerFilter)) { + if (isProfileGuidedCompilerFilter(targetCompilerFilter)) { compilerFilter = defaultCompilerFilter; File profileFile = new File(getPrebuildProfilePath(pkg)); @@ -322,8 +326,16 @@ public final class DexOptHelper { compilerFilter = targetCompilerFilter; } + // We don't expect updates in current profiles to be significant here, but + // DEXOPT_CHECK_FOR_PROFILES_UPDATES is set to replicate behaviour that will be + // unconditionally enabled for profile guided filters when ART Service is called instead of + // the legacy PackageDexOptimizer implementation. + int dexoptFlags = isProfileGuidedCompilerFilter(compilerFilter) + ? DexoptOptions.DEXOPT_CHECK_FOR_PROFILES_UPDATES + : 0; + performDexOptTraced(new DexoptOptions(pkg.getPackageName(), REASON_BOOT_AFTER_OTA, - compilerFilter, null /* splitName */, 0 /* dexoptFlags */)); + compilerFilter, null /* splitName */, dexoptFlags)); } @RequiresPermission(Manifest.permission.READ_DEVICE_CONFIG) @@ -622,16 +634,21 @@ public final class DexOptHelper { } public boolean performDexOptMode(@NonNull Computer snapshot, String packageName, - boolean checkProfiles, String targetCompilerFilter, boolean force, - boolean bootComplete, String splitName) { + String targetCompilerFilter, boolean force, boolean bootComplete, String splitName) { if (!PackageManagerServiceUtils.isSystemOrRootOrShell() && !isCallerInstallerForPackage(snapshot, packageName)) { throw new SecurityException("performDexOptMode"); } - int flags = (checkProfiles ? DexoptOptions.DEXOPT_CHECK_FOR_PROFILES_UPDATES : 0) - | (force ? DexoptOptions.DEXOPT_FORCE : 0) + int flags = (force ? DexoptOptions.DEXOPT_FORCE : 0) | (bootComplete ? DexoptOptions.DEXOPT_BOOT_COMPLETE : 0); + + if (isProfileGuidedCompilerFilter(targetCompilerFilter)) { + // Set this flag whenever the filter is profile guided, to align with ART Service + // behavior. + flags |= DexoptOptions.DEXOPT_CHECK_FOR_PROFILES_UPDATES; + } + return performDexOpt(new DexoptOptions(packageName, REASON_CMDLINE, targetCompilerFilter, splitName, flags)); } diff --git a/services/core/java/com/android/server/pm/IPackageManagerBase.java b/services/core/java/com/android/server/pm/IPackageManagerBase.java index 05a0adcf0a6d2..5c3890c192dd9 100644 --- a/services/core/java/com/android/server/pm/IPackageManagerBase.java +++ b/services/core/java/com/android/server/pm/IPackageManagerBase.java @@ -57,6 +57,7 @@ import android.os.RemoteException; import android.os.Trace; import android.os.UserHandle; import android.permission.PermissionManager; +import android.util.Log; import com.android.internal.R; import com.android.internal.content.InstallLocationUtils; @@ -964,8 +965,12 @@ public abstract class IPackageManagerBase extends IPackageManager.Stub { boolean checkProfiles, String targetCompilerFilter, boolean force, boolean bootComplete, String splitName) { final Computer snapshot = snapshot(); - return mDexOptHelper.performDexOptMode(snapshot, packageName, checkProfiles, - targetCompilerFilter, force, bootComplete, splitName); + if (!checkProfiles) { + // There is no longer a flag to skip profile checking. + Log.w(PackageManagerService.TAG, "Ignored checkProfiles=false flag"); + } + return mDexOptHelper.performDexOptMode( + snapshot, packageName, targetCompilerFilter, force, bootComplete, splitName); } /** diff --git a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java index 9f21f114560e5..cc1306dbc2b36 100644 --- a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java +++ b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java @@ -88,7 +88,6 @@ import android.os.ServiceManager; import android.os.ServiceSpecificException; import android.os.ShellCommand; import android.os.SystemClock; -import android.os.SystemProperties; import android.os.Trace; import android.os.UserHandle; import android.os.UserManager; @@ -1787,13 +1786,11 @@ class PackageManagerShellCommand extends ShellCommand { private int runCompile() throws RemoteException { final PrintWriter pw = getOutPrintWriter(); - boolean checkProfiles = SystemProperties.getBoolean("dalvik.vm.usejitprofiles", false); boolean forceCompilation = false; boolean allPackages = false; boolean clearProfileData = false; String compilerFilter = null; String compilationReason = null; - String checkProfilesRaw = null; boolean secondaryDex = false; String split = null; @@ -1816,7 +1813,9 @@ class PackageManagerShellCommand extends ShellCommand { compilationReason = getNextArgRequired(); break; case "--check-prof": - checkProfilesRaw = getNextArgRequired(); + getNextArgRequired(); + pw.println("Warning: Ignoring obsolete flag --check-prof " + + "- it is unconditionally enabled now"); break; case "--reset": forceCompilation = true; @@ -1835,17 +1834,6 @@ class PackageManagerShellCommand extends ShellCommand { } } - if (checkProfilesRaw != null) { - if ("true".equals(checkProfilesRaw)) { - checkProfiles = true; - } else if ("false".equals(checkProfilesRaw)) { - checkProfiles = false; - } else { - pw.println("Invalid value for \"--check-prof\". Expected \"true\" or \"false\"."); - return 1; - } - } - final boolean compilerFilterGiven = compilerFilter != null; final boolean compilationReasonGiven = compilationReason != null; // Make sure exactly one of -m, or -r is given. @@ -1922,11 +1910,10 @@ class PackageManagerShellCommand extends ShellCommand { } final boolean result = secondaryDex - ? mInterface.performDexOptSecondary(packageName, - targetCompilerFilter, forceCompilation) - : mInterface.performDexOptMode(packageName, - checkProfiles, targetCompilerFilter, forceCompilation, - true /* bootComplete */, split); + ? mInterface.performDexOptSecondary( + packageName, targetCompilerFilter, forceCompilation) + : mInterface.performDexOptMode(packageName, true /* checkProfiles */, + targetCompilerFilter, forceCompilation, true /* bootComplete */, split); if (!result) { failedPackages.add(packageName); } diff --git a/services/core/java/com/android/server/pm/dex/DexoptOptions.java b/services/core/java/com/android/server/pm/dex/DexoptOptions.java index f5557c417f1b3..411c19ff73103 100644 --- a/services/core/java/com/android/server/pm/dex/DexoptOptions.java +++ b/services/core/java/com/android/server/pm/dex/DexoptOptions.java @@ -18,6 +18,8 @@ package com.android.server.pm.dex; import static com.android.server.pm.PackageManagerServiceCompilerMapping.getCompilerFilterForReason; +import static dalvik.system.DexFile.isProfileGuidedCompilerFilter; + import android.annotation.Nullable; import com.android.server.art.ReasonMapping; @@ -26,8 +28,6 @@ import com.android.server.art.model.OptimizeParams; import com.android.server.pm.DexOptHelper; import com.android.server.pm.PackageManagerService; -import dalvik.system.DexFile; - /** * Options used for dexopt invocations. */ @@ -218,12 +218,11 @@ public final class DexoptOptions { /*@OptimizeFlags*/ int flags = extraFlags; if ((mFlags & DEXOPT_CHECK_FOR_PROFILES_UPDATES) == 0 - && DexFile.isProfileGuidedCompilerFilter(mCompilerFilter)) { - // ART Service doesn't support bypassing this, so not setting this flag is not - // supported. - DexOptHelper.reportArtManagerFallback(mPackageName, - "DEXOPT_CHECK_FOR_PROFILES_UPDATES not set with profile compiler filter"); - return null; + && isProfileGuidedCompilerFilter(mCompilerFilter)) { + // ART Service doesn't support bypassing the profile update check when profiles are + // used, so not setting this flag is not supported. + throw new IllegalArgumentException( + "DEXOPT_CHECK_FOR_PROFILES_UPDATES must be set with profile guided filter"); } if ((mFlags & DEXOPT_FORCE) != 0) { flags |= ArtFlags.FLAG_FORCE;