Add a new profman flag --force-merge-and-analyze.
When the flag is set, profman merges profiles even if the difference
between before and after the merge doesn't meet the thresholds, and
returns `kCompile` if there is any difference or
`kSkipCompilationSmallDelta` if there is no difference.
The behavior of this flag is the same as --force-merge, except for that
it returns different codes to indicate whether there is any difference
or not. We can't change the return code of --force-merge because
installd on S and T relies on it to be `kSuccess`.
Bug: 242170869
Test: m test-art-host-gtest-art_profman_tests
Change-Id: I59fdc64a66284ec66c4b7a7c355df9e902dfee00
diff --git a/artd/artd.cc b/artd/artd.cc
index af02d51..97faeb0 100644
--- a/artd/artd.cc
+++ b/artd/artd.cc
@@ -833,10 +833,9 @@
OR_RETURN_NON_FATAL(NewFile::Create(output_profile_path, in_outputProfile->fsPermission));
if (in_referenceProfile.has_value()) {
- if (in_options.forceMerge || in_options.dumpOnly || in_options.dumpClassesAndMethods) {
+ if (in_options.dumpOnly || in_options.dumpClassesAndMethods) {
return Fatal(
- "Reference profile must not be set when 'forceMerge', 'dumpOnly', or "
- "'dumpClassesAndMethods' is set");
+ "Reference profile must not be set when 'dumpOnly' or 'dumpClassesAndMethods' is set");
}
std::string reference_profile_path =
OR_RETURN_FATAL(BuildProfileOrDmPath(*in_referenceProfile));
@@ -869,7 +868,7 @@
props_->GetOrEmpty("dalvik.vm.bgdexopt.new-classes-percent"))
.AddIfNonEmpty("--min-new-methods-percent-change=%s",
props_->GetOrEmpty("dalvik.vm.bgdexopt.new-methods-percent"))
- .AddIf(in_options.forceMerge, "--force-merge")
+ .AddIf(in_options.forceMerge, "--force-merge-and-analyze")
.AddIf(in_options.forBootImage, "--boot-image-merge");
}
@@ -892,9 +891,8 @@
}
ProfmanResult::ProcessingResult expected_result =
- (in_options.forceMerge || in_options.dumpOnly || in_options.dumpClassesAndMethods) ?
- ProfmanResult::kSuccess :
- ProfmanResult::kCompile;
+ (in_options.dumpOnly || in_options.dumpClassesAndMethods) ? ProfmanResult::kSuccess :
+ ProfmanResult::kCompile;
if (result.value() != expected_result) {
return NonFatal(ART_FORMAT("profman returned an unexpected code: {}", result.value()));
}
diff --git a/artd/artd_test.cc b/artd/artd_test.cc
index ec957e0..6660bcc 100644
--- a/artd/artd_test.cc
+++ b/artd/artd_test.cc
@@ -1863,7 +1863,7 @@
Contains(Flag("--reference-profile-file-fd=", FdHasContent("abc"))),
Contains(Flag("--apk-fd=", FdOf(dex_file_1))),
Contains(Flag("--apk-fd=", FdOf(dex_file_2))),
- Not(Contains("--force-merge")),
+ Not(Contains("--force-merge-and-analyze")),
Not(Contains("--boot-image-merge")))),
HasKeepFdsFor("--profile-file-fd=", "--reference-profile-file-fd=", "--apk-fd=")),
_,
@@ -1979,13 +1979,14 @@
CreateFile(dex_file_);
- EXPECT_CALL(
- *mock_exec_utils_,
- DoExecAndReturnCode(
- WhenSplitBy("--", _, AllOf(Contains("--force-merge"), Contains("--boot-image-merge"))),
- _,
- _))
- .WillOnce(Return(ProfmanResult::kSuccess));
+ EXPECT_CALL(*mock_exec_utils_,
+ DoExecAndReturnCode(WhenSplitBy("--",
+ _,
+ AllOf(Contains("--force-merge-and-analyze"),
+ Contains("--boot-image-merge"))),
+ _,
+ _))
+ .WillOnce(Return(ProfmanResult::kCompile));
bool result;
EXPECT_TRUE(artd_
diff --git a/artd/binder/com/android/server/art/MergeProfileOptions.aidl b/artd/binder/com/android/server/art/MergeProfileOptions.aidl
index 2d007f9..5d3591c 100644
--- a/artd/binder/com/android/server/art/MergeProfileOptions.aidl
+++ b/artd/binder/com/android/server/art/MergeProfileOptions.aidl
@@ -28,7 +28,7 @@
* @hide
*/
parcelable MergeProfileOptions {
- /** --force-merge */
+ /** --force-merge-and-analyze */
boolean forceMerge;
/** --boot-image-merge */
boolean forBootImage;
diff --git a/profman/include/profman/profman_result.h b/profman/include/profman/profman_result.h
index 9c9aca9..c4ca988 100644
--- a/profman/include/profman/profman_result.h
+++ b/profman/include/profman/profman_result.h
@@ -40,9 +40,13 @@
kSuccess = 0,
// A merge has been performed, meaning the reference profile has been changed.
kCompile = 1,
- // `--profile-file(-fd)` is not specified, or the specified profiles are outdated (i.e., APK
- // filename or checksum mismatch), empty, or don't contain enough number of new classes and
- // methods that meets the threshold to trigger a merge.
+ // One of the following conditions is met:
+ // - `--profile-file(-fd)` is not specified.
+ // - The specified profiles are outdated (i.e., APK filename or checksum mismatch).
+ // - The specified profiles are empty.
+ // - The specified profiles don't contain any new class or method.
+ // - The specified profiles don't contain enough number of new classes and methods that meets
+ // the threshold to trigger a merge, and `--force-merge-and-analyze` is not set.
kSkipCompilationSmallDelta = 2,
// All the input profiles (including the reference profile) are either outdated (i.e., APK
// filename or checksum mismatch) or empty.
diff --git a/profman/profile_assistant.cc b/profman/profile_assistant.cc
index abbde2d..4d98e49 100644
--- a/profman/profile_assistant.cc
+++ b/profman/profile_assistant.cc
@@ -54,7 +54,7 @@
ProfileCompilationInfo cur_info(options.IsBootImageMerge());
if (!cur_info.Load(profile_files[i]->Fd(), /*merge_classes=*/ true, filter_fn)) {
LOG(WARNING) << "Could not load profile file at index " << i;
- if (options.IsForceMerge()) {
+ if (options.IsForceMerge() || options.IsForceMergeAndAnalyze()) {
// If we have to merge forcefully, ignore load failures.
// This is useful for boot image profiles to ignore stale profiles which are
// cleared lazily.
@@ -79,17 +79,30 @@
if (info.IsEmpty()) {
return ProfmanResult::kSkipCompilationEmptyProfiles;
}
- uint32_t min_change_in_methods_for_compilation = std::max(
- (options.GetMinNewMethodsPercentChangeForCompilation() * number_of_methods) / 100,
- kMinNewMethodsForCompilation);
- uint32_t min_change_in_classes_for_compilation = std::max(
- (options.GetMinNewClassesPercentChangeForCompilation() * number_of_classes) / 100,
- kMinNewClassesForCompilation);
- // Check if there is enough new information added by the current profiles.
- if (((info.GetNumberOfMethods() - number_of_methods) < min_change_in_methods_for_compilation) &&
- ((info.GetNumberOfResolvedClasses() - number_of_classes)
- < min_change_in_classes_for_compilation)) {
- return ProfmanResult::kSkipCompilationSmallDelta;
+
+ if (options.IsForceMergeAndAnalyze()) {
+ // When we force merge and analyze, we want to always recompile unless there is absolutely no
+ // difference between before and after the merge (i.e., the classes and methods in the
+ // reference profile were already a superset of those in all current profiles before the
+ // merge.)
+ if (info.GetNumberOfMethods() == number_of_methods &&
+ info.GetNumberOfResolvedClasses() == number_of_classes) {
+ return ProfmanResult::kSkipCompilationSmallDelta;
+ }
+ } else {
+ uint32_t min_change_in_methods_for_compilation = std::max(
+ (options.GetMinNewMethodsPercentChangeForCompilation() * number_of_methods) / 100,
+ kMinNewMethodsForCompilation);
+ uint32_t min_change_in_classes_for_compilation = std::max(
+ (options.GetMinNewClassesPercentChangeForCompilation() * number_of_classes) / 100,
+ kMinNewClassesForCompilation);
+ // Check if there is enough new information added by the current profiles.
+ if (((info.GetNumberOfMethods() - number_of_methods) <
+ min_change_in_methods_for_compilation) &&
+ ((info.GetNumberOfResolvedClasses() - number_of_classes) <
+ min_change_in_classes_for_compilation)) {
+ return ProfmanResult::kSkipCompilationSmallDelta;
+ }
}
}
diff --git a/profman/profile_assistant.h b/profman/profile_assistant.h
index 84f140f..5ef37b4 100644
--- a/profman/profile_assistant.h
+++ b/profman/profile_assistant.h
@@ -44,7 +44,9 @@
kMinNewClassesPercentChangeForCompilation) {
}
+ // Only for S and T uses. U+ should use `IsForceMergeAndAnalyze`.
bool IsForceMerge() const { return force_merge_; }
+ bool IsForceMergeAndAnalyze() const { return force_merge_and_analyze_; }
bool IsBootImageMerge() const { return boot_image_merge_; }
uint32_t GetMinNewMethodsPercentChangeForCompilation() const {
return min_new_methods_percent_change_for_compilation_;
@@ -54,6 +56,7 @@
}
void SetForceMerge(bool value) { force_merge_ = value; }
+ void SetForceMergeAndAnalyze(bool value) { force_merge_and_analyze_ = value; }
void SetBootImageMerge(bool value) { boot_image_merge_ = value; }
void SetMinNewMethodsPercentChangeForCompilation(uint32_t value) {
min_new_methods_percent_change_for_compilation_ = value;
@@ -63,10 +66,15 @@
}
private:
- // If true, performs a forced merge, without analyzing if there is a
- // significant difference between the current profile and the reference profile.
+ // If true, performs a forced merge, without analyzing if there is a significant difference
+ // between before and after the merge.
// See ProfileAssistant#ProcessProfile.
+ // Only for S and T uses. U+ should use `force_merge_and_analyze_`.
bool force_merge_;
+ // If true, performs a forced merge and analyzes if there is any difference between before and
+ // after the merge.
+ // See ProfileAssistant#ProcessProfile.
+ bool force_merge_and_analyze_;
// Signals that the merge is for boot image profiles. It will ignore differences
// in profile versions (instead of aborting).
bool boot_image_merge_;
diff --git a/profman/profile_assistant_test.cc b/profman/profile_assistant_test.cc
index 7d0868f..d31df20 100644
--- a/profman/profile_assistant_test.cc
+++ b/profman/profile_assistant_test.cc
@@ -2055,38 +2055,60 @@
}
TEST_F(ProfileAssistantTest, BootImageMerge) {
- ScratchFile profile;
- ScratchFile reference_profile;
- std::vector<int> profile_fds({GetFd(profile)});
- int reference_profile_fd = GetFd(reference_profile);
- std::vector<uint32_t> hot_methods_cur;
- std::vector<uint32_t> hot_methods_ref;
- std::vector<uint32_t> empty_vector;
- size_t num_methods = 100;
- for (size_t i = 0; i < num_methods; ++i) {
- hot_methods_cur.push_back(i);
+ ScratchFile profile1;
+ ScratchFile profile2;
+ ScratchFile profile3;
+ ScratchFile output_profile;
+ std::vector<uint32_t> hot_methods_1;
+ std::vector<uint32_t> hot_methods_2;
+ std::vector<uint32_t> hot_methods_3;
+ for (size_t i = 0; i < 100; ++i) {
+ hot_methods_1.push_back(i);
}
- for (size_t i = 0; i < num_methods; ++i) {
- hot_methods_ref.push_back(i);
+ for (size_t i = 50; i < 150; ++i) {
+ hot_methods_2.push_back(i);
}
- ProfileCompilationInfo info1(/*for_boot_image=*/ true);
- SetupBasicProfile(dex1, hot_methods_cur, empty_vector, empty_vector,
- profile, &info1);
+ for (size_t i = 100; i < 200; ++i) {
+ hot_methods_3.push_back(i);
+ }
+ ProfileCompilationInfo info1(/*for_boot_image=*/false);
+ SetupBasicProfile(
+ dex1, hot_methods_1, /*startup_methods=*/{}, /*post_startup_methods=*/{}, profile1, &info1);
ProfileCompilationInfo info2(/*for_boot_image=*/true);
- SetupBasicProfile(dex1, hot_methods_ref, empty_vector, empty_vector,
- reference_profile, &info2);
+ SetupBasicProfile(
+ dex1, hot_methods_2, /*startup_methods=*/{}, /*post_startup_methods=*/{}, profile2, &info2);
+ ProfileCompilationInfo info3(/*for_boot_image=*/true);
+ SetupBasicProfile(
+ dex1, hot_methods_3, /*startup_methods=*/{}, /*post_startup_methods=*/{}, profile3, &info3);
- std::vector<const std::string> extra_args({"--force-merge", "--boot-image-merge"});
+ {
+ int return_code = ProcessProfiles({profile1.GetFd(), profile2.GetFd(), profile3.GetFd()},
+ output_profile.GetFd(),
+ {"--force-merge-and-analyze", "--boot-image-merge"});
+ ASSERT_EQ(return_code, ProfmanResult::kCompile);
- int return_code = ProcessProfiles(profile_fds, reference_profile_fd, extra_args);
+ // Verify the result: it should be equal to info2 union info3 since info1 is a regular profile
+ // and should be ignored.
+ ProfileCompilationInfo result(/*for_boot_image=*/true);
+ ASSERT_TRUE(result.Load(output_profile.GetFd()));
+ ASSERT_TRUE(info2.MergeWith(info3));
+ ASSERT_TRUE(result.Equals(info2));
+ }
- ASSERT_EQ(return_code, ProfmanResult::kSuccess);
+ // Same for the legacy force merge mode.
+ {
+ int return_code = ProcessProfiles({profile1.GetFd(), profile2.GetFd(), profile3.GetFd()},
+ output_profile.GetFd(),
+ {"--force-merge", "--boot-image-merge"});
+ ASSERT_EQ(return_code, ProfmanResult::kSuccess);
- // Verify the result: it should be equal to info2 since info1 is a regular profile
- // and should be ignored.
- ProfileCompilationInfo result(/*for_boot_image=*/ true);
- ASSERT_TRUE(result.Load(reference_profile.GetFd()));
- ASSERT_TRUE(result.Equals(info2));
+ // Verify the result: it should be equal to info2 union info3 since info1 is a regular profile
+ // and should be ignored.
+ ProfileCompilationInfo result(/*for_boot_image=*/true);
+ ASSERT_TRUE(result.Load(output_profile.GetFd()));
+ ASSERT_TRUE(info2.MergeWith(info3));
+ ASSERT_TRUE(result.Equals(info2));
+ }
}
// Under default behaviour we should not advice compilation
@@ -2131,6 +2153,82 @@
ASSERT_TRUE(result.Equals(info1));
}
+TEST_F(ProfileAssistantTest, ForceMergeAndAnalyze) {
+ const uint16_t kNumberOfMethodsInRefProfile = 600;
+ const uint16_t kNumberOfMethodsInCurProfile = 601;
+
+ ScratchFile ref_profile;
+ ScratchFile cur_profile;
+
+ ProfileCompilationInfo ref_info;
+ SetupProfile(
+ dex1, dex2, kNumberOfMethodsInRefProfile, /*number_of_classes=*/0, ref_profile, &ref_info);
+ ProfileCompilationInfo cur_info;
+ SetupProfile(
+ dex1, dex2, kNumberOfMethodsInCurProfile, /*number_of_classes=*/0, cur_profile, &cur_info);
+
+ std::vector<const std::string> extra_args({"--force-merge-and-analyze"});
+ int return_code = ProcessProfiles({cur_profile.GetFd()}, ref_profile.GetFd(), extra_args);
+
+ ASSERT_EQ(return_code, ProfmanResult::kCompile);
+
+ // Check that the result is the aggregation.
+ ProfileCompilationInfo result;
+ ASSERT_TRUE(result.Load(ref_profile.GetFd()));
+ ASSERT_TRUE(ref_info.MergeWith(cur_info));
+ ASSERT_TRUE(result.Equals(ref_info));
+}
+
+TEST_F(ProfileAssistantTest, ForceMergeAndAnalyzeNoDelta) {
+ const uint16_t kNumberOfMethodsInRefProfile = 600;
+ const uint16_t kNumberOfMethodsInCurProfile = 600;
+
+ ScratchFile ref_profile;
+ ScratchFile cur_profile;
+
+ ProfileCompilationInfo ref_info;
+ SetupProfile(
+ dex1, dex2, kNumberOfMethodsInRefProfile, /*number_of_classes=*/0, ref_profile, &ref_info);
+ ProfileCompilationInfo cur_info;
+ SetupProfile(
+ dex1, dex2, kNumberOfMethodsInCurProfile, /*number_of_classes=*/0, cur_profile, &cur_info);
+
+ std::vector<const std::string> extra_args({"--force-merge-and-analyze"});
+ int return_code = ProcessProfiles({cur_profile.GetFd()}, ref_profile.GetFd(), extra_args);
+
+ ASSERT_EQ(return_code, ProfmanResult::kSkipCompilationSmallDelta);
+
+ // Check that the reference profile is unchanged.
+ ProfileCompilationInfo result;
+ ASSERT_TRUE(result.Load(ref_profile.GetFd()));
+ ASSERT_TRUE(result.Equals(ref_info));
+}
+
+TEST_F(ProfileAssistantTest, ForceMergeAndAnalyzeEmptyProfiles) {
+ const uint16_t kNumberOfMethodsInRefProfile = 0;
+ const uint16_t kNumberOfMethodsInCurProfile = 0;
+
+ ScratchFile ref_profile;
+ ScratchFile cur_profile;
+
+ ProfileCompilationInfo ref_info;
+ SetupProfile(
+ dex1, dex2, kNumberOfMethodsInRefProfile, /*number_of_classes=*/0, ref_profile, &ref_info);
+ ProfileCompilationInfo cur_info;
+ SetupProfile(
+ dex1, dex2, kNumberOfMethodsInCurProfile, /*number_of_classes=*/0, cur_profile, &cur_info);
+
+ std::vector<const std::string> extra_args({"--force-merge-and-analyze"});
+ int return_code = ProcessProfiles({cur_profile.GetFd()}, ref_profile.GetFd(), extra_args);
+
+ ASSERT_EQ(return_code, ProfmanResult::kSkipCompilationEmptyProfiles);
+
+ // Check that the reference profile is unchanged.
+ ProfileCompilationInfo result;
+ ASSERT_TRUE(result.Load(ref_profile.GetFd()));
+ ASSERT_TRUE(result.Equals(ref_info));
+}
+
// Test that we consider the annations when we merge boot image profiles.
TEST_F(ProfileAssistantTest, BootImageMergeWithAnnotations) {
ScratchFile profile;
@@ -2213,25 +2311,40 @@
std::string content = "giberish";
ASSERT_TRUE(profile1.GetFile()->WriteFully(content.c_str(), content.length()));
- ProfileCompilationInfo info2(/*for_boot_image=*/ true);
+ ProfileCompilationInfo info2(/*for_boot_image=*/true);
info2.Save(profile2.GetFd());
- std::vector<int> profile_fds({ GetFd(profile1)});
+ std::vector<int> profile_fds({GetFd(profile1)});
int reference_profile_fd = GetFd(profile2);
// With force-merge we should merge successfully.
- std::vector<const std::string> extra_args({"--force-merge", "--boot-image-merge"});
- ASSERT_EQ(ProcessProfiles(profile_fds, reference_profile_fd, extra_args),
- ProfmanResult::kSuccess);
+ {
+ ASSERT_EQ(
+ ProcessProfiles(
+ profile_fds, reference_profile_fd, {"--force-merge-and-analyze", "--boot-image-merge"}),
+ ProfmanResult::kSkipCompilationEmptyProfiles);
- ProfileCompilationInfo result(/*for_boot_image=*/ true);
- ASSERT_TRUE(result.Load(reference_profile_fd));
- ASSERT_TRUE(info2.Equals(result));
+ ProfileCompilationInfo result(/*for_boot_image=*/true);
+ ASSERT_TRUE(result.Load(reference_profile_fd));
+ ASSERT_TRUE(info2.Equals(result));
+ }
+
+ // Same for the legacy force merge mode.
+ {
+ ASSERT_EQ(
+ ProcessProfiles(profile_fds, reference_profile_fd, {"--force-merge", "--boot-image-merge"}),
+ ProfmanResult::kSuccess);
+
+ ProfileCompilationInfo result(/*for_boot_image=*/true);
+ ASSERT_TRUE(result.Load(reference_profile_fd));
+ ASSERT_TRUE(info2.Equals(result));
+ }
// Without force-merge we should fail.
- std::vector<const std::string> extra_args2({"--boot-image-merge"});
- ASSERT_EQ(ProcessProfiles(profile_fds, reference_profile_fd, extra_args2),
- ProfmanResult::kErrorBadProfiles);
+ {
+ ASSERT_EQ(ProcessProfiles(profile_fds, reference_profile_fd, {"--boot-image-merge"}),
+ ProfmanResult::kErrorBadProfiles);
+ }
}
} // namespace art
diff --git a/profman/profman.cc b/profman/profman.cc
index a39d6c5..adcf7ac 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -190,7 +190,10 @@
UsageError(" --boot-image-merge: indicates that this merge is for a boot image profile.");
UsageError(" In this case, the reference profile must have a boot profile version.");
UsageError(" --force-merge: performs a forced merge, without analyzing if there is a");
- UsageError(" significant difference between the current profile and the reference profile.");
+ UsageError(" significant difference between before and after the merge.");
+ UsageError(" Deprecated. Use --force-merge-and-analyze instead.");
+ UsageError(" --force-merge-and-analyze: performs a forced merge and analyzes if there is any");
+ UsageError(" difference between before and after the merge.");
UsageError(" --min-new-methods-percent-change=percentage between 0 and 100 (default 2)");
UsageError(" the min percent of new methods to trigger a compilation.");
UsageError(" --min-new-classes-percent-change=percentage between 0 and 100 (default 2)");
@@ -469,7 +472,11 @@
} else if (option == "--boot-image-merge") {
profile_assistant_options_.SetBootImageMerge(true);
} else if (option == "--force-merge") {
+ // For backward compatibility only.
+ // TODO(jiakaiz): Remove this when S and T are no longer supported.
profile_assistant_options_.SetForceMerge(true);
+ } else if (option == "--force-merge-and-analyze") {
+ profile_assistant_options_.SetForceMergeAndAnalyze(true);
} else {
Usage("Unknown argument '%s'", raw_option);
}