diff options
| -rw-r--r-- | cmds/installd/InstalldNativeService.cpp | 9 | ||||
| -rw-r--r-- | cmds/installd/InstalldNativeService.h | 4 | ||||
| -rw-r--r-- | cmds/installd/binder/android/os/IInstalld.aidl | 4 | ||||
| -rw-r--r-- | cmds/installd/dexopt.cpp | 109 | ||||
| -rw-r--r-- | cmds/installd/dexopt.h | 24 | ||||
| -rw-r--r-- | cmds/installd/installd_constants.h | 6 | ||||
| -rw-r--r-- | cmds/installd/tests/installd_dexopt_test.cpp | 78 |
7 files changed, 168 insertions, 66 deletions
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index c55fc6af83..a176df9789 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -2354,7 +2354,7 @@ binder::Status InstalldNativeService::copySystemProfile(const std::string& syste // TODO: Consider returning error codes. binder::Status InstalldNativeService::mergeProfiles(int32_t uid, const std::string& packageName, - const std::string& profileName, bool* _aidl_return) { + const std::string& profileName, int* _aidl_return) { ENFORCE_UID(AID_SYSTEM); CHECK_ARGUMENT_PACKAGE_NAME(packageName); std::lock_guard<std::recursive_mutex> lock(mLock); @@ -2654,7 +2654,8 @@ binder::Status InstalldNativeService::moveAb(const std::string& apkPath, } binder::Status InstalldNativeService::deleteOdex(const std::string& apkPath, - const std::string& instructionSet, const std::optional<std::string>& outputPath) { + const std::string& instructionSet, const std::optional<std::string>& outputPath, + int64_t* _aidl_return) { ENFORCE_UID(AID_SYSTEM); CHECK_ARGUMENT_PATH(apkPath); CHECK_ARGUMENT_PATH(outputPath); @@ -2664,8 +2665,8 @@ binder::Status InstalldNativeService::deleteOdex(const std::string& apkPath, const char* instruction_set = instructionSet.c_str(); const char* oat_dir = outputPath ? outputPath->c_str() : nullptr; - bool res = delete_odex(apk_path, instruction_set, oat_dir); - return res ? ok() : error(); + *_aidl_return = delete_odex(apk_path, instruction_set, oat_dir); + return *_aidl_return == -1 ? error() : ok(); } // This kernel feature is experimental. diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h index 9819327840..3127be6fd5 100644 --- a/cmds/installd/InstalldNativeService.h +++ b/cmds/installd/InstalldNativeService.h @@ -122,7 +122,7 @@ public: binder::Status rmdex(const std::string& codePath, const std::string& instructionSet); binder::Status mergeProfiles(int32_t uid, const std::string& packageName, - const std::string& profileName, bool* _aidl_return); + const std::string& profileName, int* _aidl_return); binder::Status dumpProfiles(int32_t uid, const std::string& packageName, const std::string& profileName, const std::string& codePath, bool* _aidl_return); binder::Status copySystemProfile(const std::string& systemProfile, @@ -147,7 +147,7 @@ public: binder::Status moveAb(const std::string& apkPath, const std::string& instructionSet, const std::string& outputPath); binder::Status deleteOdex(const std::string& apkPath, const std::string& instructionSet, - const std::optional<std::string>& outputPath); + const std::optional<std::string>& outputPath, int64_t* _aidl_return); binder::Status installApkVerity(const std::string& filePath, android::base::unique_fd verityInput, int32_t contentSize); binder::Status assertFsverityRootHashMatches(const std::string& filePath, diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl index 4ac70a4857..816e50826d 100644 --- a/cmds/installd/binder/android/os/IInstalld.aidl +++ b/cmds/installd/binder/android/os/IInstalld.aidl @@ -71,7 +71,7 @@ interface IInstalld { void rmdex(@utf8InCpp String codePath, @utf8InCpp String instructionSet); - boolean mergeProfiles(int uid, @utf8InCpp String packageName, @utf8InCpp String profileName); + int mergeProfiles(int uid, @utf8InCpp String packageName, @utf8InCpp String profileName); boolean dumpProfiles(int uid, @utf8InCpp String packageName, @utf8InCpp String profileName, @utf8InCpp String codePath); boolean copySystemProfile(@utf8InCpp String systemProfile, int uid, @@ -93,7 +93,7 @@ interface IInstalld { @utf8InCpp String toBase); void moveAb(@utf8InCpp String apkPath, @utf8InCpp String instructionSet, @utf8InCpp String outputPath); - void deleteOdex(@utf8InCpp String apkPath, @utf8InCpp String instructionSet, + long deleteOdex(@utf8InCpp String apkPath, @utf8InCpp String instructionSet, @nullable @utf8InCpp String outputPath); void installApkVerity(@utf8InCpp String filePath, in FileDescriptor verityInput, int contentSize); diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index cc0434d9e4..15f0c5b75c 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -53,6 +53,7 @@ #include "execv_helper.h" #include "globals.h" #include "installd_deps.h" +#include "installd_constants.h" #include "otapreopt_utils.h" #include "run_dex2oat.h" #include "unique_file.h" @@ -416,11 +417,12 @@ static void open_profile_files(uid_t uid, const std::string& package_name, static constexpr int PROFMAN_BIN_RETURN_CODE_SUCCESS = 0; static constexpr int PROFMAN_BIN_RETURN_CODE_COMPILE = 1; -static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION = 2; +static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_NOT_ENOUGH_DELTA = 2; static constexpr int PROFMAN_BIN_RETURN_CODE_BAD_PROFILES = 3; static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_IO = 4; static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_LOCKING = 5; static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_DIFFERENT_VERSIONS = 6; +static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_EMPTY_PROFILES = 7; class RunProfman : public ExecVHelper { public: @@ -555,15 +557,7 @@ class RunProfman : public ExecVHelper { std::vector<unique_fd> apk_fds_; }; - - -// Decides if profile guided compilation is needed or not based on existing profiles. -// The location is the package name for primary apks or the dex path for secondary dex files. -// Returns true if there is enough information in the current profiles that makes it -// worth to recompile the given location. -// If the return value is true all the current profiles would have been merged into -// the reference profiles accessible with open_reference_profile(). -static bool analyze_profiles(uid_t uid, const std::string& package_name, +static int analyze_profiles(uid_t uid, const std::string& package_name, const std::string& location, bool is_secondary_dex) { std::vector<unique_fd> profiles_fd; unique_fd reference_profile_fd; @@ -572,7 +566,7 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, if (profiles_fd.empty() || (reference_profile_fd.get() < 0)) { // Skip profile guided compilation because no profiles were found. // Or if the reference profile info couldn't be opened. - return false; + return PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES; } RunProfman profman_merge; @@ -594,6 +588,7 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, /* parent */ int return_code = wait_child(pid); bool need_to_compile = false; + bool empty_profiles = false; bool should_clear_current_profiles = false; bool should_clear_reference_profile = false; if (!WIFEXITED(return_code)) { @@ -606,8 +601,14 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, should_clear_current_profiles = true; should_clear_reference_profile = false; break; - case PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION: + case PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_NOT_ENOUGH_DELTA: + need_to_compile = false; + should_clear_current_profiles = false; + should_clear_reference_profile = false; + break; + case PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_EMPTY_PROFILES: need_to_compile = false; + empty_profiles = true; should_clear_current_profiles = false; should_clear_reference_profile = false; break; @@ -653,16 +654,29 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, if (should_clear_reference_profile) { clear_reference_profile(package_name, location, is_secondary_dex); } - return need_to_compile; + int result = 0; + if (need_to_compile) { + result = PROFILES_ANALYSIS_OPTIMIZE; + } else if (empty_profiles) { + result = PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES; + } else { + result = PROFILES_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA; + } + return result; } // Decides if profile guided compilation is needed or not based on existing profiles. -// The analysis is done for the primary apks of the given package. -// Returns true if there is enough information in the current profiles that makes it -// worth to recompile the package. -// If the return value is true all the current profiles would have been merged into -// the reference profiles accessible with open_reference_profile(). -bool analyze_primary_profiles(uid_t uid, const std::string& package_name, +// The analysis is done for a single profile name (which corresponds to a single code path). +// +// Returns PROFILES_ANALYSIS_OPTIMIZE if there is enough information in the current profiles +// that makes it worth to recompile the package. +// If the return value is PROFILES_ANALYSIS_OPTIMIZE all the current profiles would have been +// merged into the reference profiles accessible with open_reference_profile(). +// +// Return PROFILES_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA if the package should not optimize. +// As a special case returns PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES if all profiles are +// empty. +int analyze_primary_profiles(uid_t uid, const std::string& package_name, const std::string& profile_name) { return analyze_profiles(uid, package_name, profile_name, /*is_secondary_dex*/false); } @@ -1166,7 +1180,7 @@ class RunDexoptAnalyzer : public ExecVHelper { int zip_fd, const std::string& instruction_set, const std::string& compiler_filter, - bool profile_was_updated, + int profile_analysis_result, bool downgrade, const char* class_loader_context, const std::string& class_loader_context_fds) { @@ -1182,7 +1196,8 @@ class RunDexoptAnalyzer : public ExecVHelper { std::string zip_fd_arg = "--zip-fd=" + std::to_string(zip_fd); std::string isa_arg = "--isa=" + instruction_set; std::string compiler_filter_arg = "--compiler-filter=" + compiler_filter; - const char* assume_profile_changed = "--assume-profile-changed"; + std::string profile_analysis_arg = "--profile-analysis-result=" + + std::to_string(profile_analysis_result); const char* downgrade_flag = "--downgrade"; std::string class_loader_context_arg = "--class-loader-context="; if (class_loader_context != nullptr) { @@ -1204,9 +1219,8 @@ class RunDexoptAnalyzer : public ExecVHelper { AddArg(vdex_fd_arg); } AddArg(zip_fd_arg); - if (profile_was_updated) { - AddArg(assume_profile_changed); - } + AddArg(profile_analysis_arg); + if (downgrade) { AddArg(downgrade_flag); } @@ -1578,7 +1592,7 @@ static bool process_secondary_dex_dexopt(const std::string& dex_path, const char } // Analyze profiles. - bool profile_was_updated = analyze_profiles(uid, pkgname, dex_path, + int profile_analysis_result = analyze_profiles(uid, pkgname, dex_path, /*is_secondary_dex*/true); // Run dexoptanalyzer to get dexopt_needed code. This is not expected to return. @@ -1589,7 +1603,8 @@ static bool process_secondary_dex_dexopt(const std::string& dex_path, const char oat_file_fd.get(), zip_fd.get(), instruction_set, - compiler_filter, profile_was_updated, + compiler_filter, + profile_analysis_result, downgrade, class_loader_context, join_fds(context_zip_fds)); @@ -2237,38 +2252,52 @@ bool move_ab(const char* apk_path, const char* instruction_set, const char* oat_ return success; } -bool delete_odex(const char* apk_path, const char* instruction_set, const char* oat_dir) { +int64_t delete_odex(const char* apk_path, const char* instruction_set, const char* oat_dir) { // Delete the oat/odex file. char out_path[PKG_PATH_MAX]; if (!create_oat_out_path(apk_path, instruction_set, oat_dir, /*is_secondary_dex*/false, out_path)) { - return false; + LOG(ERROR) << "Cannot create apk path for " << apk_path; + return -1; } // In case of a permission failure report the issue. Otherwise just print a warning. - auto unlink_and_check = [](const char* path) -> bool { - int result = unlink(path); - if (result != 0) { - if (errno == EACCES || errno == EPERM) { + auto unlink_and_check = [](const char* path) -> int64_t { + struct stat file_stat; + if (stat(path, &file_stat) != 0) { + if (errno != ENOENT) { + PLOG(ERROR) << "Could not stat " << path; + return -1; + } + return 0; + } + + if (unlink(path) != 0) { + if (errno != ENOENT) { PLOG(ERROR) << "Could not unlink " << path; - return false; + return -1; } - PLOG(WARNING) << "Could not unlink " << path; } - return true; + return static_cast<int64_t>(file_stat.st_size); }; // Delete the oat/odex file. - bool return_value_oat = unlink_and_check(out_path); + int64_t return_value_oat = unlink_and_check(out_path); // Derive and delete the app image. - bool return_value_art = unlink_and_check(create_image_filename(out_path).c_str()); + int64_t return_value_art = unlink_and_check(create_image_filename(out_path).c_str()); // Derive and delete the vdex file. - bool return_value_vdex = unlink_and_check(create_vdex_filename(out_path).c_str()); + int64_t return_value_vdex = unlink_and_check(create_vdex_filename(out_path).c_str()); + + // Report result + if (return_value_oat == -1 + || return_value_art == -1 + || return_value_vdex == -1) { + return -1; + } - // Report success. - return return_value_oat && return_value_art && return_value_vdex; + return return_value_oat + return_value_art + return_value_vdex; } static bool is_absolute_path(const std::string& path) { diff --git a/cmds/installd/dexopt.h b/cmds/installd/dexopt.h index d35953c058..5a637b1ce7 100644 --- a/cmds/installd/dexopt.h +++ b/cmds/installd/dexopt.h @@ -54,15 +54,20 @@ bool clear_primary_current_profile(const std::string& pkgname, const std::string // Clear all current profiles identified by the given profile name (all users). bool clear_primary_current_profiles(const std::string& pkgname, const std::string& profile_name); -// Decide if profile guided compilation is needed or not based on existing profiles. +// Decides if profile guided compilation is needed or not based on existing profiles. // The analysis is done for a single profile name (which corresponds to a single code path). -// Returns true if there is enough information in the current profiles that makes it -// worth to recompile the package. -// If the return value is true all the current profiles would have been merged into -// the reference profiles accessible with open_reference_profile(). -bool analyze_primary_profiles(uid_t uid, - const std::string& pkgname, - const std::string& profile_name); +// +// Returns PROFILES_ANALYSIS_OPTIMIZE if there is enough information in the current profiles +// that makes it worth to recompile the package. +// If the return value is PROFILES_ANALYSIS_OPTIMIZE all the current profiles would have been +// merged into the reference profiles accessible with open_reference_profile(). +// +// Return PROFILES_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA if the package should not optimize. +// As a special case returns PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES if all profiles are +// empty. +int analyze_primary_profiles(uid_t uid, + const std::string& pkgname, + const std::string& profile_name); // Create a snapshot of the profile information for the given package profile. // If appId is -1, the method creates the profile snapshot for the boot image. @@ -104,7 +109,8 @@ bool prepare_app_profile(const std::string& package_name, const std::string& code_path, const std::optional<std::string>& dex_metadata); -bool delete_odex(const char* apk_path, const char* instruction_set, const char* output_path); +// Returns the total bytes that were freed, or -1 in case of errors. +int64_t delete_odex(const char* apk_path, const char* instruction_set, const char* output_path); bool reconcile_secondary_dex_file(const std::string& dex_path, const std::string& pkgname, int uid, const std::vector<std::string>& isas, diff --git a/cmds/installd/installd_constants.h b/cmds/installd/installd_constants.h index b5ee481645..00d8441607 100644 --- a/cmds/installd/installd_constants.h +++ b/cmds/installd/installd_constants.h @@ -77,6 +77,12 @@ constexpr int DEXOPT_MASK = constexpr int FLAG_STORAGE_DE = 1 << 0; constexpr int FLAG_STORAGE_CE = 1 << 1; +// TODO: import them from dexoptanalyzer.h +// NOTE: keep in sync with Installer.java +constexpr int PROFILES_ANALYSIS_OPTIMIZE = 1; +constexpr int PROFILES_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA = 2; +constexpr int PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES = 3; + #define ARRAY_SIZE(a) (sizeof(a) / sizeof(*(a))) } // namespace installd diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp index 216347e616..7e7e513568 100644 --- a/cmds/installd/tests/installd_dexopt_test.cpp +++ b/cmds/installd/tests/installd_dexopt_test.cpp @@ -38,6 +38,7 @@ #include "binder_test_utils.h" #include "dexopt.h" #include "InstalldNativeService.h" +#include "installd_constants.h" #include "globals.h" #include "tests/test_utils.h" #include "utils.h" @@ -517,7 +518,8 @@ protected: // Check the access to the compiler output. // - speed-profile artifacts are not world-wide readable. // - files are owned by the system uid. - std::string odex = GetPrimaryDexArtifact(oat_dir, apk_path_, "odex"); + std::string odex = GetPrimaryDexArtifact(oat_dir, apk_path_, + oat_dir == nullptr ? "dex" : "odex"); std::string vdex = GetPrimaryDexArtifact(oat_dir, apk_path_, "vdex"); std::string art = GetPrimaryDexArtifact(oat_dir, apk_path_, "art"); @@ -545,7 +547,7 @@ protected: } } return android_data_dir + DALVIK_CACHE + '/' + kRuntimeIsa + "/" + path - + "@classes.dex"; + + "@classes." + type; } else { std::string::size_type name_end = dex_path.rfind('.'); std::string::size_type name_start = dex_path.rfind('/'); @@ -553,6 +555,53 @@ protected: dex_path.substr(name_start + 1, name_end - name_start) + type; } } + + int64_t GetSize(const std::string& path) { + struct stat file_stat; + if (stat(path.c_str(), &file_stat) == 0) { + return static_cast<int64_t>(file_stat.st_size); + } + PLOG(ERROR) << "Cannot stat path: " << path; + return -1; + } + + void TestDeleteOdex(bool in_dalvik_cache) { + const char* oat_dir = in_dalvik_cache ? nullptr : app_oat_dir_.c_str(); + CompilePrimaryDexOk( + "speed-profile", + DEXOPT_BOOTCOMPLETE | DEXOPT_PROFILE_GUIDED | DEXOPT_PUBLIC + | DEXOPT_GENERATE_APP_IMAGE, + oat_dir, + kTestAppGid, + DEX2OAT_FROM_SCRATCH, + /*binder_result=*/nullptr, + empty_dm_file_.c_str()); + + + int64_t odex_size = GetSize(GetPrimaryDexArtifact(oat_dir, apk_path_, + in_dalvik_cache ? "dex" : "odex")); + int64_t vdex_size = GetSize(GetPrimaryDexArtifact(oat_dir, apk_path_, "vdex")); + int64_t art_size = GetSize(GetPrimaryDexArtifact(oat_dir, apk_path_, "art")); + + LOG(ERROR) << "test odex " << odex_size; + LOG(ERROR) << "test vdex_size " << vdex_size; + LOG(ERROR) << "test art_size " << art_size; + int64_t expected_bytes_freed = odex_size + vdex_size + art_size; + + int64_t bytes_freed; + binder::Status result = service_->deleteOdex( + apk_path_, + kRuntimeIsa, + in_dalvik_cache ? std::nullopt : std::make_optional<std::string>(app_oat_dir_.c_str()), + &bytes_freed); + ASSERT_TRUE(result.isOk()) << result.toString8().c_str(); + + ASSERT_GE(odex_size, 0); + ASSERT_GE(vdex_size, 0); + ASSERT_GE(art_size, 0); + + ASSERT_EQ(expected_bytes_freed, bytes_freed); + } }; @@ -701,6 +750,16 @@ TEST_F(DexoptTest, DexoptPrimaryBackgroundOk) { empty_dm_file_.c_str()); } +TEST_F(DexoptTest, DeleteDexoptArtifactsData) { + LOG(INFO) << "DeleteDexoptArtifactsData"; + TestDeleteOdex(/*in_dalvik_cache=*/ false); +} + +TEST_F(DexoptTest, DeleteDexoptArtifactsDalvikCache) { + LOG(INFO) << "DeleteDexoptArtifactsDalvikCache"; + TestDeleteOdex(/*in_dalvik_cache=*/ true); +} + TEST_F(DexoptTest, ResolveStartupConstStrings) { LOG(INFO) << "DexoptDex2oatResolveStartupStrings"; const std::string property = "persist.device_config.runtime.dex2oat_resolve_startup_strings"; @@ -951,14 +1010,14 @@ class ProfileTest : public DexoptTest { void mergePackageProfiles(const std::string& package_name, const std::string& code_path, - bool expected_result) { - bool result; + int expected_result) { + int result; ASSERT_BINDER_SUCCESS(service_->mergeProfiles( kTestAppUid, package_name, code_path, &result)); ASSERT_EQ(expected_result, result); - if (!expected_result) { - // Do not check the files if we expect to fail. + // There's nothing to check if the files are empty. + if (result == PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES) { return; } @@ -1077,7 +1136,7 @@ TEST_F(ProfileTest, ProfileMergeOk) { LOG(INFO) << "ProfileMergeOk"; SetupProfiles(/*setup_ref*/ true); - mergePackageProfiles(package_name_, "primary.prof", /*expected_result*/ true); + mergePackageProfiles(package_name_, "primary.prof", PROFILES_ANALYSIS_OPTIMIZE); } // The reference profile is created on the fly. We need to be able to @@ -1086,14 +1145,15 @@ TEST_F(ProfileTest, ProfileMergeOkNoReference) { LOG(INFO) << "ProfileMergeOkNoReference"; SetupProfiles(/*setup_ref*/ false); - mergePackageProfiles(package_name_, "primary.prof", /*expected_result*/ true); + mergePackageProfiles(package_name_, "primary.prof", PROFILES_ANALYSIS_OPTIMIZE); } TEST_F(ProfileTest, ProfileMergeFailWrongPackage) { LOG(INFO) << "ProfileMergeFailWrongPackage"; SetupProfiles(/*setup_ref*/ true); - mergePackageProfiles("not.there", "primary.prof", /*expected_result*/ false); + mergePackageProfiles("not.there", "primary.prof", + PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES); } TEST_F(ProfileTest, ProfileDirOk) { |