Merge "Ensure we select the right execution binary for all dexopt commands"

3 files changed, 115 insertions, 14 deletions

diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp
index f523725e7b..ffd1191501 100644
--- a/cmds/installd/dexopt.cpp
+++ b/cmds/installd/dexopt.cpp
@@ -261,6 +261,40 @@ static std::string MapPropertyToArg(const std::string& property,
   return "";
 }
 
+// Determines which binary we should use for execution (the debug or non-debug version).
+// e.g. dex2oatd vs dex2oat
+static const char* select_execution_binary(const char* binary, const char* debug_binary,
+        bool background_job_compile) {
+    return select_execution_binary(
+        binary,
+        debug_binary,
+        background_job_compile,
+        is_debug_runtime(),
+        (android::base::GetProperty("ro.build.version.codename", "") == "REL"),
+        is_debuggable_build());
+}
+
+// Determines which binary we should use for execution (the debug or non-debug version).
+// e.g. dex2oatd vs dex2oat
+// This is convenient method which is much easier to test because it doesn't read
+// system properties.
+const char* select_execution_binary(
+        const char* binary,
+        const char* debug_binary,
+        bool background_job_compile,
+        bool is_debug_runtime,
+        bool is_release,
+        bool is_debuggable_build) {
+    // Do not use debug binaries for release candidates (to give more soak time).
+    bool is_debug_bg_job = background_job_compile && is_debuggable_build && !is_release;
+
+    // If the runtime was requested to use libartd.so, we'll run the debug version - assuming
+    // the file is present (it may not be on images with very little space available).
+    bool useDebug = (is_debug_runtime || is_debug_bg_job) && (access(debug_binary, X_OK) == 0);
+
+    return useDebug ? debug_binary : binary;
+}
+
 // Namespace for Android Runtime flags applied during boot time.
 static const char* RUNTIME_NATIVE_BOOT_NAMESPACE = "runtime_native_boot";
 // Feature flag name for running the JIT in Zygote experiment, b/119800099.
@@ -354,16 +388,9 @@ class RunDex2Oat : public ExecVHelper {
         std::string dex2oat_large_app_threshold_arg =
             MapPropertyToArg("dalvik.vm.dex2oat-very-large", "--very-large-app-threshold=%s");
 
-        // If the runtime was requested to use libartd.so, we'll run dex2oatd, otherwise dex2oat.
-        const char* dex2oat_bin = kDex2oatPath;
-        // Do not use dex2oatd for release candidates (give dex2oat more soak time).
-        bool is_release = android::base::GetProperty("ro.build.version.codename", "") == "REL";
-        if (is_debug_runtime() ||
-                (background_job_compile && is_debuggable_build() && !is_release)) {
-            if (access(kDex2oatDebugPath, X_OK) == 0) {
-                dex2oat_bin = kDex2oatDebugPath;
-            }
-        }
+
+        const char* dex2oat_bin = select_execution_binary(
+            kDex2oatPath, kDex2oatDebugPath, background_job_compile);
 
         bool generate_minidebug_info = kEnableMinidebugInfo &&
                 GetBoolProperty(kMinidebugInfoSystemProperty, kMinidebugInfoSystemPropertyDefault);
@@ -676,7 +703,12 @@ class RunProfman : public ExecVHelper {
                   const std::vector<std::string>& dex_locations,
                   bool copy_and_update,
                   bool store_aggregation_counters) {
-        const char* profman_bin = is_debug_runtime() ? kProfmanDebugPath: kProfmanPath;
+
+        // TODO(calin): Assume for now we run in the bg compile job (which is in
+        // most of the invocation). With the current data flow, is not very easy or
+        // clean to discover this in RunProfman (it will require quite a messy refactoring).
+        const char* profman_bin = select_execution_binary(
+            kProfmanPath, kProfmanDebugPath, /*background_job_compile=*/ true);
 
         if (copy_and_update) {
             CHECK_EQ(1u, profile_fds.size());
@@ -1491,8 +1523,10 @@ class RunDexoptAnalyzer : public ExecVHelper {
                     bool downgrade,
                     const char* class_loader_context) {
         CHECK_GE(zip_fd, 0);
-        const char* dexoptanalyzer_bin =
-            is_debug_runtime() ? kDexoptanalyzerDebugPath : kDexoptanalyzerPath;
+
+        // We always run the analyzer in the background job.
+        const char* dexoptanalyzer_bin = select_execution_binary(
+             kDexoptanalyzerPath, kDexoptanalyzerDebugPath, /*background_job_compile=*/ true);
 
         std::string dex_file_arg = "--dex-file=" + dex_file;
         std::string oat_fd_arg = "--oat-fd=" + std::to_string(oat_fd);
diff --git a/cmds/installd/dexopt.h b/cmds/installd/dexopt.h
index 5902659e8b..a8c48c564e 100644
--- a/cmds/installd/dexopt.h
+++ b/cmds/installd/dexopt.h
@@ -36,7 +36,7 @@ static constexpr int DEX2OAT_FOR_FILTER          = 3;
 // Location of binaries in the Android Runtime APEX.
 static constexpr const char* kDex2oatPath = ANDROID_RUNTIME_APEX_BIN "/dex2oat";
 static constexpr const char* kDex2oatDebugPath = ANDROID_RUNTIME_APEX_BIN "/dex2oatd";
-static constexpr const char* kProfmanPath = ANDROID_RUNTIME_APEX_BIN "/profmand";
+static constexpr const char* kProfmanPath = ANDROID_RUNTIME_APEX_BIN "/profman";
 static constexpr const char* kProfmanDebugPath = ANDROID_RUNTIME_APEX_BIN "/profmand";
 static constexpr const char* kDexoptanalyzerPath = ANDROID_RUNTIME_APEX_BIN "/dexoptanalyzer";
 static constexpr const char* kDexoptanalyzerDebugPath = ANDROID_RUNTIME_APEX_BIN "/dexoptanalyzerd";
@@ -128,6 +128,14 @@ bool create_cache_path_default(char path[PKG_PATH_MAX], const char *src,
 
 bool move_ab(const char* apk_path, const char* instruction_set, const char* output_path);
 
+const char* select_execution_binary(
+        const char* binary,
+        const char* debug_binary,
+        bool background_job_compile,
+        bool is_debug_runtime,
+        bool is_release,
+        bool is_debuggable_build);
+
 }  // namespace installd
 }  // namespace android
 
diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp
index 78edce08fa..71b710b0d3 100644
--- a/cmds/installd/tests/installd_dexopt_test.cpp
+++ b/cmds/installd/tests/installd_dexopt_test.cpp
@@ -1175,5 +1175,64 @@ TEST_F(BootProfileTest, CollectProfiles) {
     ASSERT_TRUE(std::find(profiles.begin(), profiles.end(), ref_prof) != profiles.end());
 }
 
+TEST_F(DexoptTest, select_execution_binary) {
+    LOG(INFO) << "DexoptTestselect_execution_binary";
+
+    std::string release_str = app_private_dir_ce_  + "/release";
+    std::string debug_str = app_private_dir_ce_  + "/debug";
+
+    // Setup the binaries. Note that we only need executable files to actually
+    // test the execution binary selection
+    run_cmd("touch " + release_str);
+    run_cmd("touch " + debug_str);
+    run_cmd("chmod 777 " + release_str);
+    run_cmd("chmod 777 " + debug_str);
+
+    const char* release = release_str.c_str();
+    const char* debug = debug_str.c_str();
+
+    ASSERT_STREQ(release, select_execution_binary(
+        release,
+        debug,
+        /*background_job_compile=*/ false,
+        /*is_debug_runtime=*/ false,
+        /*is_release=*/ false,
+        /*is_debuggable_build=*/ false));
+
+    ASSERT_STREQ(release, select_execution_binary(
+        release,
+        debug,
+        /*background_job_compile=*/ true,
+        /*is_debug_runtime=*/ false,
+        /*is_release=*/ true,
+        /*is_debuggable_build=*/ true));
+
+    ASSERT_STREQ(debug, select_execution_binary(
+        release,
+        debug,
+        /*background_job_compile=*/ false,
+        /*is_debug_runtime=*/ true,
+        /*is_release=*/ false,
+        /*is_debuggable_build=*/ false));
+
+    ASSERT_STREQ(debug, select_execution_binary(
+        release,
+        debug,
+        /*background_job_compile=*/ true,
+        /*is_debug_runtime=*/ false,
+        /*is_release=*/ false,
+        /*is_debuggable_build=*/ true));
+
+
+    // Select the release when the debug file is not there.
+    ASSERT_STREQ(release, select_execution_binary(
+        release,
+        "does_not_exist",
+        /*background_job_compile=*/ false,
+        /*is_debug_runtime=*/ true,
+        /*is_release=*/ false,
+        /*is_debuggable_build=*/ false));
+}
+
 }  // namespace installd
 }  // namespace android