Merge changes from topic "boot-prof-fix"
* changes:
Improve the handling of profile snapshot in installd
Fix BootImageProfile aggregation
diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp
index 838d11d..768d900 100644
--- a/cmds/installd/dexopt.cpp
+++ b/cmds/installd/dexopt.cpp
@@ -713,6 +713,7 @@
static constexpr int PROFMAN_BIN_RETURN_CODE_BAD_PROFILES = 2;
static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_IO = 3;
static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_LOCKING = 4;
+static constexpr int PROFMAN_BIN_RETURN_CODE_SUCCESS = 5;
class RunProfman : public ExecVHelper {
public:
@@ -720,7 +721,9 @@
const unique_fd& reference_profile_fd,
const std::vector<unique_fd>& apk_fds,
const std::vector<std::string>& dex_locations,
- bool copy_and_update) {
+ bool copy_and_update,
+ bool for_snapshot,
+ bool for_boot_image) {
// 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
@@ -752,6 +755,14 @@
AddArg("--copy-and-update-profile-key");
}
+ if (for_snapshot) {
+ AddArg("--force-merge");
+ }
+
+ if (for_boot_image) {
+ AddArg("--boot-image-merge");
+ }
+
// Do not add after dex2oat_flags, they should override others for debugging.
PrepareArgs(profman_bin);
}
@@ -759,12 +770,16 @@
void SetupMerge(const std::vector<unique_fd>& profiles_fd,
const unique_fd& reference_profile_fd,
const std::vector<unique_fd>& apk_fds = std::vector<unique_fd>(),
- const std::vector<std::string>& dex_locations = std::vector<std::string>()) {
+ const std::vector<std::string>& dex_locations = std::vector<std::string>(),
+ bool for_snapshot = false,
+ bool for_boot_image = false) {
SetupArgs(profiles_fd,
reference_profile_fd,
apk_fds,
dex_locations,
- /*copy_and_update=*/false);
+ /*copy_and_update=*/ false,
+ for_snapshot,
+ for_boot_image);
}
void SetupCopyAndUpdate(unique_fd&& profile_fd,
@@ -781,7 +796,9 @@
reference_profile_fd_,
apk_fds_,
dex_locations,
- /*copy_and_update=*/true);
+ /*copy_and_update=*/true,
+ /*for_snapshot*/false,
+ /*for_boot_image*/false);
}
void SetupDump(const std::vector<unique_fd>& profiles_fd,
@@ -795,7 +812,9 @@
reference_profile_fd,
apk_fds,
dex_locations,
- /*copy_and_update=*/false);
+ /*copy_and_update=*/false,
+ /*for_snapshot*/false,
+ /*for_boot_image*/false);
}
void Exec() {
@@ -872,7 +891,7 @@
break;
default:
// Unknown return code or error. Unlink profiles.
- LOG(WARNING) << "Unknown error code while processing profiles for location "
+ LOG(WARNING) << "Unexpected error code while processing profiles for location "
<< location << ": " << return_code;
need_to_compile = false;
should_clear_current_profiles = true;
@@ -2741,7 +2760,7 @@
}
RunProfman args;
- args.SetupMerge(profiles_fd, snapshot_fd, apk_fds, dex_locations);
+ args.SetupMerge(profiles_fd, snapshot_fd, apk_fds, dex_locations, /*for_snapshot=*/true);
pid_t pid = fork();
if (pid == 0) {
/* child -- drop privileges before continuing */
@@ -2756,6 +2775,13 @@
return false;
}
+ // Verify that profman finished successfully.
+ int profman_code = WEXITSTATUS(return_code);
+ if (profman_code != PROFMAN_BIN_RETURN_CODE_SUCCESS) {
+ LOG(WARNING) << "profman error for " << package_name << ":" << profile_name
+ << ":" << profman_code;
+ return false;
+ }
return true;
}
@@ -2818,19 +2844,29 @@
// We do this to avoid opening a huge a amount of files.
static constexpr size_t kAggregationBatchSize = 10;
- std::vector<unique_fd> profiles_fd;
for (size_t i = 0; i < profiles.size(); ) {
+ std::vector<unique_fd> profiles_fd;
for (size_t k = 0; k < kAggregationBatchSize && i < profiles.size(); k++, i++) {
unique_fd fd = open_profile(AID_SYSTEM, profiles[i], O_RDONLY);
if (fd.get() >= 0) {
profiles_fd.push_back(std::move(fd));
}
}
+
+ // We aggregate (read & write) into the same fd multiple times in a row.
+ // We need to reset the cursor every time to ensure we read the whole file every time.
+ if (TEMP_FAILURE_RETRY(lseek(snapshot_fd, 0, SEEK_SET)) == static_cast<off_t>(-1)) {
+ PLOG(ERROR) << "Cannot reset position for snapshot profile";
+ return false;
+ }
+
RunProfman args;
args.SetupMerge(profiles_fd,
snapshot_fd,
apk_fds,
- dex_locations);
+ dex_locations,
+ /*for_snapshot=*/true,
+ /*for_boot_image=*/true);
pid_t pid = fork();
if (pid == 0) {
/* child -- drop privileges before continuing */
@@ -2843,12 +2879,21 @@
/* parent */
int return_code = wait_child(pid);
+
if (!WIFEXITED(return_code)) {
PLOG(WARNING) << "profman failed for " << package_name << ":" << profile_name;
return false;
}
- return true;
+
+ // Verify that profman finished successfully.
+ int profman_code = WEXITSTATUS(return_code);
+ if (profman_code != PROFMAN_BIN_RETURN_CODE_SUCCESS) {
+ LOG(WARNING) << "profman error for " << package_name << ":" << profile_name
+ << ":" << profman_code;
+ return false;
+ }
}
+
return true;
}
diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp
index 0212bc5..69fefa1 100644
--- a/cmds/installd/tests/installd_dexopt_test.cpp
+++ b/cmds/installd/tests/installd_dexopt_test.cpp
@@ -897,7 +897,9 @@
std::string expected_profile_content = snap_profile_ + ".expected";
run_cmd("rm -f " + expected_profile_content);
run_cmd("touch " + expected_profile_content);
- run_cmd("profman --profile-file=" + cur_profile_ +
+ // We force merging when creating the expected profile to make sure
+ // that the random profiles do not affect the output.
+ run_cmd("profman --force-merge --profile-file=" + cur_profile_ +
" --profile-file=" + ref_profile_ +
" --reference-profile-file=" + expected_profile_content +
" --apk=" + apk_path_);
@@ -1130,16 +1132,60 @@
class BootProfileTest : public ProfileTest {
public:
- virtual void setup() {
+ std::vector<const std::string> extra_apps_;
+ std::vector<int64_t> extra_ce_data_inodes_;
+
+ virtual void SetUp() {
+
ProfileTest::SetUp();
intial_android_profiles_dir = android_profiles_dir;
+ // Generate profiles for some extra apps.
+ // When merging boot profile we split profiles into small groups to avoid
+ // opening a lot of file descriptors at the same time.
+ // (Currently the group size for aggregation is 10)
+ //
+ // To stress test that works fine, create profile for more apps.
+ createAppProfilesForBootMerge(21);
}
virtual void TearDown() {
android_profiles_dir = intial_android_profiles_dir;
+ deleteAppProfilesForBootMerge();
ProfileTest::TearDown();
}
+ void createAppProfilesForBootMerge(size_t number_of_profiles) {
+ for (size_t i = 0; i < number_of_profiles; i++) {
+ int64_t ce_data_inode;
+ std::string package_name = "dummy_test_pkg" + std::to_string(i);
+ LOG(INFO) << package_name;
+ ASSERT_BINDER_SUCCESS(service_->createAppData(
+ volume_uuid_,
+ package_name,
+ kTestUserId,
+ kAppDataFlags,
+ kTestAppUid,
+ se_info_,
+ kOSdkVersion,
+ &ce_data_inode));
+ extra_apps_.push_back(package_name);
+ extra_ce_data_inodes_.push_back(ce_data_inode);
+ std::string profile = create_current_profile_path(
+ kTestUserId, package_name, kPrimaryProfile, /*is_secondary_dex*/ false);
+ SetupProfile(profile, kTestAppUid, kTestAppGid, 0600, 1);
+ }
+ }
+
+ void deleteAppProfilesForBootMerge() {
+ if (kDebug) {
+ return;
+ }
+ for (size_t i = 0; i < extra_apps_.size(); i++) {
+ service_->destroyAppData(
+ volume_uuid_, extra_apps_[i], kTestUserId, kAppDataFlags, extra_ce_data_inodes_[i]);
+ }
+ }
+
void UpdateAndroidProfilesDir(const std::string& profile_dir) {
android_profiles_dir = profile_dir;
// We need to create the reference profile directory in the new profile dir.