Skip to content

Commit

Permalink
Merge changes from topic "boot-prof-fix" am: 4848455 am: 0d5f91b
Browse files Browse the repository at this point in the history
am: 81c0cc8

Change-Id: I3d05a010fa7eaf3948c597753dec9824518f5c05
  • Loading branch information
Calin Juravle authored and android-build-merger committed Nov 13, 2019
2 parents 9b32e4a + 81c0cc8 commit cf802c5
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 19 deletions.
58 changes: 40 additions & 18 deletions cmds/installd/dexopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -713,14 +713,17 @@ static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION = 1;
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:
void SetupArgs(const std::vector<unique_fd>& profile_fds,
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
Expand Down Expand Up @@ -752,19 +755,31 @@ class RunProfman : public ExecVHelper {
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);
}

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,
Expand All @@ -781,7 +796,9 @@ class RunProfman : public ExecVHelper {
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,
Expand All @@ -795,7 +812,9 @@ class RunProfman : public ExecVHelper {
reference_profile_fd,
apk_fds,
dex_locations,
/*copy_and_update=*/false);
/*copy_and_update=*/false,
/*for_snapshot*/false,
/*for_boot_image*/false);
}

void Exec() {
Expand Down Expand Up @@ -872,7 +891,7 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name,
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;
Expand Down Expand Up @@ -2741,7 +2760,7 @@ static bool create_app_profile_snapshot(int32_t app_id,
}

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 */
Expand All @@ -2756,6 +2775,13 @@ static bool create_app_profile_snapshot(int32_t app_id,
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;
}

Expand Down Expand Up @@ -2838,7 +2864,9 @@ static bool create_boot_image_profile_snapshot(const std::string& package_name,
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 */
Expand All @@ -2859,16 +2887,10 @@ static bool create_boot_image_profile_snapshot(const std::string& package_name,

// Verify that profman finished successfully.
int profman_code = WEXITSTATUS(return_code);
switch (profman_code) {
case PROFMAN_BIN_RETURN_CODE_BAD_PROFILES: // fall through
case PROFMAN_BIN_RETURN_CODE_ERROR_IO: // fall through
case PROFMAN_BIN_RETURN_CODE_ERROR_LOCKING:
LOG(WARNING) << "profman error for " << package_name << ":" << profile_name
<< ":" << profman_code;
return false;
default:
// Other return codes are ok.
continue;
if (profman_code != PROFMAN_BIN_RETURN_CODE_SUCCESS) {
LOG(WARNING) << "profman error for " << package_name << ":" << profile_name
<< ":" << profman_code;
return false;
}
}

Expand Down
4 changes: 3 additions & 1 deletion cmds/installd/tests/installd_dexopt_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,9 @@ class ProfileTest : public DexoptTest {
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_);
Expand Down

0 comments on commit cf802c5

Please sign in to comment.