Better handling of various types of compilation in DexOptimizer

From the runtime perspective extract-only/profile-guide-compiled/fully-
compiled oat files are up-to-date and don't need dex2oat. However,
wihout knowing the exact "class" of the aot file we are not able to do a
full compilation of something previously only-extracted, or limit
profile-guide compilation to only previously profile-guide or extract-
only oat files.

GetDexOptNeeded now accepts a mask of desired types of compilation
that the runtime takes into account when advising what's needed.

This CL adds the necessary handling in DexOptimizer.

Also:
- removes the contraint to use extract-only oat files only when
profiles are enabled.
- removes mDexOptPerformed which interfere with the different types of
compilation and only saved a call to GetDexOptNeeded.

Bug: 27189430
Change-Id: Iced2bdcc3aa7866ff888a3bace43f4b71fffb353

5 files changed, 50 insertions, 94 deletions

diff --git a/core/java/android/content/pm/PackageParser.java b/core/java/android/content/pm/PackageParser.java
index ce6ddfd31d19..0588a9d6224e 100644
--- a/core/java/android/content/pm/PackageParser.java
+++ b/core/java/android/content/pm/PackageParser.java
@@ -4690,9 +4690,6 @@ public class PackageParser {
         // preferred up order.
         public int mPreferredOrder = 0;
 
-        // For use by package manager to keep track of where it needs to do dexopt.
-        public final ArraySet<String> mDexOptPerformed = new ArraySet<>(4);
-
         // For use by package manager to keep track of when a package was last used.
         public long mLastPackageUsageTimeInMills;
 
diff --git a/core/java/com/android/internal/os/ZygoteInit.java b/core/java/com/android/internal/os/ZygoteInit.java
index 6ad9e20360e0..92cee5e1a022 100644
--- a/core/java/com/android/internal/os/ZygoteInit.java
+++ b/core/java/com/android/internal/os/ZygoteInit.java
@@ -497,11 +497,11 @@ public class ZygoteInit {
 
         try {
             for (String classPathElement : classPathElements) {
+                // System server is fully AOTed and never profiled
+                // for profile guided compilation.
                 final int dexoptNeeded = DexFile.getDexOptNeeded(
-                        classPathElement, "*", instructionSet, false /* defer */);
+                        classPathElement, instructionSet, DexFile.COMPILATION_TYPE_FULL);
                 if (dexoptNeeded != DexFile.NO_DEXOPT_NEEDED) {
-                    // System server is fully AOTed and never profiled
-                    // for profile guided compilation.
                     installer.dexopt(classPathElement, Process.SYSTEM_UID, instructionSet,
                             dexoptNeeded, 0 /*dexFlags*/, null /*volumeUuid*/,
                             false /*useProfiles*/);
diff --git a/services/core/java/com/android/server/pm/OtaDexoptService.java b/services/core/java/com/android/server/pm/OtaDexoptService.java
index 94b3b2d613cf..3d2a3555ecd9 100644
--- a/services/core/java/com/android/server/pm/OtaDexoptService.java
+++ b/services/core/java/com/android/server/pm/OtaDexoptService.java
@@ -180,10 +180,5 @@ public class OtaDexoptService extends IOtaDexopt.Stub {
             return dexoptFlags | DEXOPT_OTA;
         }
 
-        @Override
-        protected void recordSuccessfulDexopt(Package pkg, String instructionSet) {
-            // Never record the dexopt, as it's in the B partition.
-        }
-
     }
 }
diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java
index a084d866ec64..c9613b4313c6 100644
--- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java
+++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java
@@ -113,15 +113,6 @@ class PackageDexOptimizer {
     }
 
     /**
-     * Determine whether the package should be skipped for the given instruction set. A return
-     * value of true means the package will be skipped. A return value of false means that the
-     * package will be further investigated, and potentially compiled.
-     */
-    protected boolean shouldSkipBasedOnISA(PackageParser.Package pkg, String instructionSet) {
-        return pkg.mDexOptPerformed.contains(instructionSet);
-    }
-
-    /**
      * Adjust the given dexopt-needed value. Can be overridden to influence the decision to
      * optimize or not (and in what way).
      */
@@ -136,13 +127,6 @@ class PackageDexOptimizer {
         return dexoptFlags;
     }
 
-    /**
-     * Update the package status after a successful compilation.
-     */
-    protected void recordSuccessfulDexopt(PackageParser.Package pkg, String instructionSet) {
-        pkg.mDexOptPerformed.add(instructionSet);
-    }
-
     private int performDexOptLI(PackageParser.Package pkg, String[] targetInstructionSets,
             boolean useProfiles, boolean extractOnly) {
         final String[] instructionSets = targetInstructionSets != null ?
@@ -159,11 +143,6 @@ class PackageDexOptimizer {
         boolean performedDexOpt = false;
         final String[] dexCodeInstructionSets = getDexCodeInstructionSets(instructionSets);
         for (String dexCodeInstructionSet : dexCodeInstructionSets) {
-            if (!useProfiles && shouldSkipBasedOnISA(pkg, dexCodeInstructionSet)) {
-                // Skip only if we do not use profiles since they might trigger a recompilation.
-                continue;
-            }
-
             for (String path : paths) {
                 if (useProfiles && isUsedByOtherApps(path)) {
                     // We cannot use profile guided compilation if the apk was used by another app.
@@ -172,35 +151,44 @@ class PackageDexOptimizer {
                 int dexoptNeeded;
 
                 try {
-                    dexoptNeeded = DexFile.getDexOptNeeded(path, pkg.packageName,
-                            dexCodeInstructionSet, /* defer */false);
+                    int compilationTypeMask = 0;
+                    if (extractOnly) {
+                        // For extract only, any type of compilation is good.
+                        compilationTypeMask = DexFile.COMPILATION_TYPE_FULL
+                            | DexFile.COMPILATION_TYPE_PROFILE_GUIDE
+                            | DexFile.COMPILATION_TYPE_EXTRACT_ONLY;
+                    } else {
+                        // Branch taken for profile guide and full compilation.
+                        // Profile guide compilation should only recompile a previous
+                        // profile compiled/extract only file and should not be attempted if the
+                        // apk is already fully compiled. So test against a full compilation type.
+                        compilationTypeMask = DexFile.COMPILATION_TYPE_FULL;
+                    }
+                    dexoptNeeded = DexFile.getDexOptNeeded(path,
+                            dexCodeInstructionSet, compilationTypeMask);
                 } catch (IOException ioe) {
                     Slog.w(TAG, "IOException reading apk: " + path, ioe);
                     return DEX_OPT_FAILED;
                 }
                 dexoptNeeded = adjustDexoptNeeded(dexoptNeeded);
 
-                if (dexoptNeeded == DexFile.NO_DEXOPT_NEEDED) {
-                    if (useProfiles) {
-                        // Profiles may trigger re-compilation. The final decision is taken in
-                        // installd.
-                        dexoptNeeded = DexFile.DEX2OAT_NEEDED;
-                    } else {
-                        // No dexopt needed and we don't use profiles. Nothing to do.
-                        continue;
-                    }
-                }
                 final String dexoptType;
                 String oatDir = null;
-                if (dexoptNeeded == DexFile.DEX2OAT_NEEDED) {
-                    dexoptType = "dex2oat";
-                    oatDir = createOatDirIfSupported(pkg, dexCodeInstructionSet);
-                } else if (dexoptNeeded == DexFile.PATCHOAT_NEEDED) {
-                    dexoptType = "patchoat";
-                } else if (dexoptNeeded == DexFile.SELF_PATCHOAT_NEEDED) {
-                    dexoptType = "self patchoat";
-                } else {
-                    throw new IllegalStateException("Invalid dexopt needed: " + dexoptNeeded);
+                switch (dexoptNeeded) {
+                    case DexFile.NO_DEXOPT_NEEDED:
+                        continue;
+                    case DexFile.DEX2OAT_NEEDED:
+                        dexoptType = "dex2oat";
+                        oatDir = createOatDirIfSupported(pkg, dexCodeInstructionSet);
+                        break;
+                    case DexFile.PATCHOAT_NEEDED:
+                        dexoptType = "patchoat";
+                        break;
+                    case DexFile.SELF_PATCHOAT_NEEDED:
+                        dexoptType = "self patchoat";
+                        break;
+                    default:
+                        throw new IllegalStateException("Invalid dexopt:" + dexoptNeeded);
                 }
 
 
@@ -226,15 +214,6 @@ class PackageDexOptimizer {
                     Slog.w(TAG, "Failed to dexopt", e);
                 }
             }
-
-            if (!extractOnly) {
-                // At this point we haven't failed dexopt and we haven't deferred dexopt. We must
-                // either have either succeeded dexopt, or have had getDexOptNeeded tell us
-                // it isn't required. We therefore mark that this package doesn't need dexopt unless
-                // it's forced. performedDexOpt will tell us whether we performed dex-opt or skipped
-                // it.
-                recordSuccessfulDexopt(pkg, dexCodeInstructionSet);
-            }
         }
 
         // If we've gotten here, we're sure that no error occurred and that we haven't
@@ -317,12 +296,6 @@ class PackageDexOptimizer {
         }
 
         @Override
-        protected boolean shouldSkipBasedOnISA(Package pkg, String instructionSet) {
-            // Forced compilation, never skip.
-            return false;
-        }
-
-        @Override
         protected int adjustDexoptNeeded(int dexoptNeeded) {
             // Ensure compilation, no matter the current state.
             // TODO: The return value is wrong when patchoat is needed.
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index f3937c812d01..b095c19de90f 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -637,9 +637,6 @@ public class PackageManagerService extends IPackageManager.Stub {
     // List of packages names to keep cached, even if they are uninstalled for all users
     private List<String> mKeepUninstalledPackages;
 
-    private boolean mUseJitProfiles =
-            SystemProperties.getBoolean("dalvik.vm.usejitprofiles", false);
-
     private static class IFVerificationParams {
         PackageParser.Package pkg;
         boolean replacing;
@@ -2159,10 +2156,12 @@ public class PackageManagerService extends IPackageManager.Stub {
                         }
 
                         try {
-                            int dexoptNeeded = DexFile.getDexOptNeeded(lib, null, dexCodeInstructionSet, false);
+                            // Shared libraries do not have profiles so we perform a full
+                            // AOT compilation (if needed).
+                            int dexoptNeeded = DexFile.getDexOptNeeded(
+                                    lib, dexCodeInstructionSet,
+                                    DexFile.COMPILATION_TYPE_FULL);
                             if (dexoptNeeded != DexFile.NO_DEXOPT_NEEDED) {
-                                // Shared libraries do not have profiles so we perform a full
-                                // AOT compilation.
                                 mInstaller.dexopt(lib, Process.SYSTEM_UID, dexCodeInstructionSet,
                                         dexoptNeeded, DEXOPT_PUBLIC /*dexFlags*/,
                                         StorageManager.UUID_PRIVATE_INTERNAL,
@@ -6828,7 +6827,7 @@ public class PackageManagerService extends IPackageManager.Stub {
 
         // Extract pacakges only if profile-guided compilation is enabled because
         // otherwise BackgroundDexOptService will not dexopt them later.
-        if (!mUseJitProfiles || !isUpgrade()) {
+        if (!isUpgrade()) {
             return;
         }
 
@@ -6910,10 +6909,6 @@ public class PackageManagerService extends IPackageManager.Stub {
 
             targetInstructionSet = instructionSet != null ? instructionSet :
                     getPrimaryInstructionSet(p.applicationInfo);
-            if (!force && !useProfiles && p.mDexOptPerformed.contains(targetInstructionSet)) {
-                // Skip only if we do not use profiles since they might trigger a recompilation.
-                return false;
-            }
         }
         long callingId = Binder.clearCallingIdentity();
         try {
@@ -13832,21 +13827,17 @@ public class PackageManagerService extends IPackageManager.Stub {
                 return;
             }
 
-            // Extract package to save the VM unzipping the APK in memory during
-            // launch. Only do this if profile-guided compilation is enabled because
-            // otherwise BackgroundDexOptService will not dexopt the package later.
-            if (mUseJitProfiles) {
-                Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "dexopt");
-                // Do not run PackageDexOptimizer through the local performDexOpt
-                // method because `pkg` is not in `mPackages` yet.
-                int result = mPackageDexOptimizer.performDexOpt(pkg, null /* instructionSets */,
-                        false /* useProfiles */, true /* extractOnly */);
-                Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER);
-                if (result == PackageDexOptimizer.DEX_OPT_FAILED) {
-                    String msg = "Extracking package failed for " + pkgName;
-                    res.setError(INSTALL_FAILED_DEXOPT, msg);
-                    return;
-                }
+
+            Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "dexopt");
+            // Do not run PackageDexOptimizer through the local performDexOpt
+            // method because `pkg` is not in `mPackages` yet.
+            int result = mPackageDexOptimizer.performDexOpt(pkg, null /* instructionSets */,
+                    false /* useProfiles */, true /* extractOnly */);
+            Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER);
+            if (result == PackageDexOptimizer.DEX_OPT_FAILED) {
+                String msg = "Extracking package failed for " + pkgName;
+                res.setError(INSTALL_FAILED_DEXOPT, msg);
+                return;
             }
         }