Merge changes from topics "delete_odex", "verify-prof"
* changes:
Return the freed bytes from deleteOdex API
Extend profile analysis with proper return codes
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index c55fc6a..a176df9 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -2354,7 +2354,7 @@
// 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::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 @@
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 9819327..3127be6 100644
--- a/cmds/installd/InstalldNativeService.h
+++ b/cmds/installd/InstalldNativeService.h
@@ -122,7 +122,7 @@
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 @@
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 4ac70a4..816e508 100644
--- a/cmds/installd/binder/android/os/IInstalld.aidl
+++ b/cmds/installd/binder/android/os/IInstalld.aidl
@@ -71,7 +71,7 @@
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 @@
@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 cc0434d..15f0c5b 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 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 @@
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 @@
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 @@
/* 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,11 +601,17 @@
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;
case PROFMAN_BIN_RETURN_CODE_BAD_PROFILES:
LOG(WARNING) << "Bad profiles for location " << location;
need_to_compile = false;
@@ -653,16 +654,29 @@
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 @@
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 @@
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 @@
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 @@
}
// 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 @@
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 @@
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) {
- PLOG(ERROR) << "Could not unlink " << path;
- return false;
+ 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;
}
- PLOG(WARNING) << "Could not unlink " << path;
+ return 0;
}
- return true;
+
+ if (unlink(path) != 0) {
+ if (errno != ENOENT) {
+ PLOG(ERROR) << "Could not unlink " << path;
+ return -1;
+ }
+ }
+ 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 success.
- return return_value_oat && return_value_art && return_value_vdex;
+ // Report result
+ if (return_value_oat == -1
+ || return_value_art == -1
+ || return_value_vdex == -1) {
+ return -1;
+ }
+
+ 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 d35953c..5a637b1 100644
--- a/cmds/installd/dexopt.h
+++ b/cmds/installd/dexopt.h
@@ -54,15 +54,20 @@
// 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 @@
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 b5ee481..00d8441 100644
--- a/cmds/installd/installd_constants.h
+++ b/cmds/installd/installd_constants.h
@@ -77,6 +77,12 @@
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 216347e..7e7e513 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 @@
// 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 @@
}
}
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 @@
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 @@
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 @@
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 @@
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 @@
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) {