Merge changes from topics "delete_odex", "verify-prof"

* changes:
  Return the freed bytes from deleteOdex API
  Don't recompile apks with speed-profile is the profile is empty

4 files changed, 82 insertions, 22 deletions

diff --git a/services/core/java/com/android/server/pm/Installer.java b/services/core/java/com/android/server/pm/Installer.java
index 5ac86ca3f6da..f9dd826e4b99 100644
--- a/services/core/java/com/android/server/pm/Installer.java
+++ b/services/core/java/com/android/server/pm/Installer.java
@@ -75,6 +75,17 @@ public class Installer extends SystemService {
     /** Indicates that dexopt may be run with different performance / priority tuned for restore */
     public static final int DEXOPT_FOR_RESTORE = 1 << 13; // TODO(b/135202722): remove
 
+    /** The result of the profile analysis indicating that the app should be optimized. */
+    public static final int PROFILE_ANALYSIS_OPTIMIZE = 1;
+    /** The result of the profile analysis indicating that the app should not be optimized. */
+    public static final int PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA = 2;
+    /**
+     * The result of the profile analysis indicating that the app should not be optimized because
+     * the profiles are empty.
+     */
+    public static final int PROFILE_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES = 3;
+
+
     public static final int FLAG_STORAGE_DE = IInstalld.FLAG_STORAGE_DE;
     public static final int FLAG_STORAGE_CE = IInstalld.FLAG_STORAGE_CE;
     public static final int FLAG_STORAGE_EXTERNAL = IInstalld.FLAG_STORAGE_EXTERNAL;
@@ -388,9 +399,18 @@ public class Installer extends SystemService {
         }
     }
 
-    public boolean mergeProfiles(int uid, String packageName, String profileName)
+    /**
+     * Analyzes the ART profiles of the given package, possibly merging the information
+     * into the reference profile. Returns whether or not we should optimize the package
+     * based on how much information is in the profile.
+     *
+     * @return one of {@link #PROFILE_ANALYSIS_OPTIMIZE},
+     *         {@link #PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA},
+     *         {@link #PROFILE_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES}
+     */
+    public int mergeProfiles(int uid, String packageName, String profileName)
             throws InstallerException {
-        if (!checkBeforeRemote()) return false;
+        if (!checkBeforeRemote()) return PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA;
         try {
             return mInstalld.mergeProfiles(uid, packageName, profileName);
         } catch (Exception e) {
@@ -537,13 +557,17 @@ public class Installer extends SystemService {
         }
     }
 
-    public void deleteOdex(String apkPath, String instructionSet, String outputPath)
+    /**
+     * Deletes the optimized artifacts generated by ART and returns the number
+     * of freed bytes.
+     */
+    public long deleteOdex(String apkPath, String instructionSet, String outputPath)
             throws InstallerException {
-        if (!checkBeforeRemote()) return;
+        if (!checkBeforeRemote()) return -1;
         BlockGuard.getVmPolicy().onPathAccess(apkPath);
         BlockGuard.getVmPolicy().onPathAccess(outputPath);
         try {
-            mInstalld.deleteOdex(apkPath, instructionSet, outputPath);
+            return mInstalld.deleteOdex(apkPath, instructionSet, outputPath);
         } catch (Exception e) {
             throw InstallerException.from(e);
         }
diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java
index 5ef3d7629cdd..50be856e3070 100644
--- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java
+++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java
@@ -31,6 +31,9 @@ import static com.android.server.pm.Installer.DEXOPT_PUBLIC;
 import static com.android.server.pm.Installer.DEXOPT_SECONDARY_DEX;
 import static com.android.server.pm.Installer.DEXOPT_STORAGE_CE;
 import static com.android.server.pm.Installer.DEXOPT_STORAGE_DE;
+import static com.android.server.pm.Installer.PROFILE_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES;
+import static com.android.server.pm.Installer.PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA;
+import static com.android.server.pm.Installer.PROFILE_ANALYSIS_OPTIMIZE;
 import static com.android.server.pm.InstructionSets.getAppDexInstructionSets;
 import static com.android.server.pm.InstructionSets.getDexCodeInstructionSets;
 import static com.android.server.pm.PackageManagerService.PLATFORM_PACKAGE_NAME;
@@ -248,8 +251,12 @@ public class PackageDexOptimizer {
                     || packageUseInfo.isUsedByOtherApps(path);
             final String compilerFilter = getRealCompilerFilter(pkg,
                 options.getCompilerFilter(), isUsedByOtherApps);
-            final boolean profileUpdated = options.isCheckForProfileUpdates() &&
-                isProfileUpdated(pkg, sharedGid, profileName, compilerFilter);
+            // If we don't have to check for profiles updates assume
+            // PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA which will be a no-op with respect to
+            // profiles.
+            final int profileAnalysisResult = options.isCheckForProfileUpdates()
+                    ? analyseProfiles(pkg, sharedGid, profileName, compilerFilter)
+                    : PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA;
 
             // Get the dexopt flags after getRealCompilerFilter to make sure we get the correct
             // flags.
@@ -257,7 +264,7 @@ public class PackageDexOptimizer {
 
             for (String dexCodeIsa : dexCodeInstructionSets) {
                 int newResult = dexOptPath(pkg, pkgSetting, path, dexCodeIsa, compilerFilter,
-                        profileUpdated, classLoaderContexts[i], dexoptFlags, sharedGid,
+                        profileAnalysisResult, classLoaderContexts[i], dexoptFlags, sharedGid,
                         packageStats, options.isDowngrade(), profileName, dexMetadataPath,
                         options.getCompilationReason());
 
@@ -305,11 +312,11 @@ public class PackageDexOptimizer {
      */
     @GuardedBy("mInstallLock")
     private int dexOptPath(AndroidPackage pkg, @NonNull PackageSetting pkgSetting, String path,
-            String isa, String compilerFilter, boolean profileUpdated, String classLoaderContext,
+            String isa, String compilerFilter, int profileAnalysisResult, String classLoaderContext,
             int dexoptFlags, int uid, CompilerStats.PackageStats packageStats, boolean downgrade,
             String profileName, String dexMetadataPath, int compilationReason) {
         int dexoptNeeded = getDexoptNeeded(path, isa, compilerFilter, classLoaderContext,
-                profileUpdated, downgrade);
+                profileAnalysisResult, downgrade);
         if (Math.abs(dexoptNeeded) == DexFile.NO_DEXOPT_NEEDED) {
             return DEX_OPT_SKIPPED;
         }
@@ -363,7 +370,7 @@ public class PackageDexOptimizer {
                     isa,
                     options.getCompilerFilter(),
                     dexUseInfo.getClassLoaderContext(),
-                    /* newProfile= */false,
+                    PROFILE_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES,
                     /* downgrade= */ false);
 
             if (dexoptNeeded == DexFile.NO_DEXOPT_NEEDED) {
@@ -749,11 +756,25 @@ public class PackageDexOptimizer {
      * configuration (isa, compiler filter, profile).
      */
     private int getDexoptNeeded(String path, String isa, String compilerFilter,
-            String classLoaderContext, boolean newProfile, boolean downgrade) {
+            String classLoaderContext, int profileAnalysisResult, boolean downgrade) {
         int dexoptNeeded;
         try {
-            dexoptNeeded = DexFile.getDexOptNeeded(path, isa, compilerFilter, classLoaderContext,
-                    newProfile, downgrade);
+            // A profile guided optimizations with an empty profile is essentially 'verify' and
+            // dex2oat already makes this transformation. However DexFile.getDexOptNeeded() cannot
+            // check the profiles because system server does not have access to them.
+            // As such, we rely on the previous profile analysis (done with dexoptanalyzer) and
+            // manually adjust the actual filter before checking.
+            //
+            // TODO: ideally. we'd move this check in dexoptanalyzer, but that's a large change,
+            // and in the interim we can still improve things here.
+            String actualCompilerFilter = compilerFilter;
+            if (compilerFilterDependsOnProfiles(compilerFilter)
+                    && profileAnalysisResult == PROFILE_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES) {
+                actualCompilerFilter = "verify";
+            }
+            boolean newProfile = profileAnalysisResult == PROFILE_ANALYSIS_OPTIMIZE;
+            dexoptNeeded = DexFile.getDexOptNeeded(path, isa, actualCompilerFilter,
+                    classLoaderContext, newProfile, downgrade);
         } catch (IOException ioe) {
             Slog.w(TAG, "IOException reading apk: " + path, ioe);
             return DEX_OPT_FAILED;
@@ -764,27 +785,34 @@ public class PackageDexOptimizer {
         return adjustDexoptNeeded(dexoptNeeded);
     }
 
+    /** Returns true if the compiler filter depends on profiles (e.g speed-profile). */
+    private boolean compilerFilterDependsOnProfiles(String compilerFilter) {
+        return compilerFilter.endsWith("-profile");
+    }
+
     /**
      * Checks if there is an update on the profile information of the {@code pkg}.
-     * If the compiler filter is not profile guided the method returns false.
+     * If the compiler filter is not profile guided the method returns a safe default:
+     * PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA.
      *
      * Note that this is a "destructive" operation with side effects. Under the hood the
      * current profile and the reference profile will be merged and subsequent calls
      * may return a different result.
      */
-    private boolean isProfileUpdated(AndroidPackage pkg, int uid, String profileName,
+    private int analyseProfiles(AndroidPackage pkg, int uid, String profileName,
             String compilerFilter) {
         // Check if we are allowed to merge and if the compiler filter is profile guided.
         if (!isProfileGuidedCompilerFilter(compilerFilter)) {
-            return false;
+            return PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA;
         }
         // Merge profiles. It returns whether or not there was an updated in the profile info.
         try {
             return mInstaller.mergeProfiles(uid, pkg.getPackageName(), profileName);
         } catch (InstallerException e) {
             Slog.w(TAG, "Failed to merge profiles", e);
+            // We don't need to optimize if we failed to merge.
+            return PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA;
         }
-        return false;
     }
 
     /**
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 05ba9dafd928..a3df212c536a 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -25555,14 +25555,14 @@ public class PackageManagerService extends IPackageManager.Stub
         }
     }
 
-    void deleteOatArtifactsOfPackage(String packageName) {
+    long deleteOatArtifactsOfPackage(String packageName) {
         final AndroidPackage pkg;
         final PackageSetting pkgSetting;
         synchronized (mLock) {
             pkg = mPackages.get(packageName);
             pkgSetting = mSettings.getPackageLPr(packageName);
         }
-        mDexManager.deleteOptimizedFiles(ArtUtils.createArtPackageInfo(pkg, pkgSetting));
+        return mDexManager.deleteOptimizedFiles(ArtUtils.createArtPackageInfo(pkg, pkgSetting));
     }
 
     Set<String> getUnusedPackages(long downgradeTimeThresholdMillis) {
diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java
index 32ba26c2d5ed..58204891293c 100644
--- a/services/core/java/com/android/server/pm/dex/DexManager.java
+++ b/services/core/java/com/android/server/pm/dex/DexManager.java
@@ -1034,18 +1034,26 @@ public class DexManager {
 
     /**
      * Deletes all the optimizations files generated by ART.
+     * This is best effort, and the method will log but not throw errors
+     * for individual deletes
+     *
      * @param packageInfo the package information.
+     * @return the number of freed bytes or -1 if there was an error in the process.
      */
-    public void deleteOptimizedFiles(ArtPackageInfo packageInfo) {
+    public long deleteOptimizedFiles(ArtPackageInfo packageInfo) {
+        long freedBytes = 0;
+        boolean hadErrors = false;
         for (String codePath : packageInfo.getCodePaths()) {
             for (String isa : packageInfo.getInstructionSets()) {
                 try {
-                    mInstaller.deleteOdex(codePath, isa, packageInfo.getOatDir());
+                    freedBytes += mInstaller.deleteOdex(codePath, isa, packageInfo.getOatDir());
                 } catch (InstallerException e) {
                     Log.e(TAG, "Failed deleting oat files for " + codePath, e);
+                    hadErrors = true;
                 }
             }
         }
+        return hadErrors ? -1 : freedBytes;
     }
 
     public static class RegisterDexModuleResult {