Process method flags uniformly in the profile
We used to have two main ways to add methods in profiles: one which would
assume methods are hot and another one where we had to specify the method
flags explicitly. This created some implicit assumptions when adding new
methods that made it hard to follow which methods sets what.
This CL unify the way flags are processed removing any assumptions of flags.
Bug: 139884006
Test: m test-art-host
Change-Id: Ib1dcfdd4a5220507b89e3e0947252cef4fa2eb4c
diff --git a/libprofile/profile/profile_compilation_info.cc b/libprofile/profile/profile_compilation_info.cc
index 34b2fed..574fd25 100644
--- a/libprofile/profile/profile_compilation_info.cc
+++ b/libprofile/profile/profile_compilation_info.cc
@@ -696,15 +696,20 @@
// The data is null if there is a mismatch in the checksum or number of method ids.
return false;
}
-
- // Add the method.
- InlineCacheMap* inline_cache = data->FindOrAddMethod(method_index);
- if (inline_cache == nullptr) {
+ if (!data->AddMethod(flags, method_index)) {
// Happens if the method index is outside the range (i.e. is greater then the number
// of methods in the dex file). This should not happen during normal execution,
// But tools (e.g. boot image aggregation tools) and tests stress this behaviour.
return false;
}
+ if ((flags & MethodHotness::kFlagHot) == 0) {
+ // The method is not hot, do not add inline caches.
+ return true;
+ }
+
+ // Add inline caches.
+ InlineCacheMap* inline_cache = data->FindOrAddMethod(method_index);
+ DCHECK(inline_cache != nullptr);
data->SetMethodHotness(method_index, flags);
@@ -750,11 +755,17 @@
if (data == nullptr) { // checksum mismatch
return false;
}
- InlineCacheMap* inline_cache = data->FindOrAddMethod(pmi.ref.index);
- if (inline_cache == nullptr) {
+ if (!data->AddMethod(flags, pmi.ref.index)) {
return false;
}
- data->SetMethodHotness(pmi.ref.index, flags);
+ if ((flags & MethodHotness::kFlagHot) == 0) {
+ // The method is not hot, do not add inline caches.
+ return true;
+ }
+
+ // Add inline caches.
+ InlineCacheMap* inline_cache = data->FindOrAddMethod(pmi.ref.index);
+ DCHECK(inline_cache != nullptr);
for (const ProfileMethodInfo::ProfileInlineCache& cache : pmi.inline_caches) {
if (cache.is_missing_types) {
diff --git a/libprofile/profile/profile_compilation_info_test.cc b/libprofile/profile/profile_compilation_info_test.cc
index bae1903..4ef4b98 100644
--- a/libprofile/profile/profile_compilation_info_test.cc
+++ b/libprofile/profile/profile_compilation_info_test.cc
@@ -61,8 +61,10 @@
uint16_t method_idx,
const ProfileCompilationInfo::OfflineProfileMethodInfo& pmi,
ProfileCompilationInfo* info) {
+ Hotness::Flag flags = static_cast<Hotness::Flag>(
+ Hotness::kFlagHot | Hotness::kFlagPostStartup);
return info->AddMethod(
- dex_location, checksum, method_idx, kMaxMethodIds, pmi, Hotness::kFlagPostStartup);
+ dex_location, checksum, method_idx, kMaxMethodIds, pmi, flags);
}
bool AddClass(const std::string& dex_location,
@@ -1351,4 +1353,73 @@
TEST_F(ProfileCompilationInfoTest, SizeStressTestAllInRandom) {
SizeStressTest(/*random=*/ true);
}
+
+// Verifies that we correctly add methods to the profile according to their flags.
+TEST_F(ProfileCompilationInfoTest, AddMethodsProfileMethodInfoBasic) {
+ std::unique_ptr<const DexFile> dex(OpenTestDexFile("ManyMethods"));
+
+ ProfileCompilationInfo info;
+
+ MethodReference hot(dex.get(), 0);
+ MethodReference hot_startup(dex.get(), 1);
+ MethodReference startup(dex.get(), 2);
+
+ // Add methods
+ ASSERT_TRUE(info.AddMethod(ProfileMethodInfo(hot), Hotness::kFlagHot));
+ ASSERT_TRUE(info.AddMethod(
+ ProfileMethodInfo(hot_startup),
+ static_cast<Hotness::Flag>(Hotness::kFlagHot | Hotness::kFlagStartup)));
+ ASSERT_TRUE(info.AddMethod(ProfileMethodInfo(startup), Hotness::kFlagStartup));
+
+ // Verify the profile recorded them correctly.
+ EXPECT_TRUE(info.GetMethodHotness(hot).IsInProfile());
+ EXPECT_EQ(info.GetMethodHotness(hot).GetFlags(), Hotness::kFlagHot);
+
+ EXPECT_TRUE(info.GetMethodHotness(hot_startup).IsInProfile());
+ EXPECT_EQ(info.GetMethodHotness(hot_startup).GetFlags(),
+ static_cast<uint32_t>(Hotness::kFlagHot | Hotness::kFlagStartup));
+
+ EXPECT_TRUE(info.GetMethodHotness(startup).IsInProfile());
+ EXPECT_EQ(info.GetMethodHotness(startup).GetFlags(), Hotness::kFlagStartup);
+}
+
+// Verifies that we correctly add inline caches to the profile only for hot methods.
+TEST_F(ProfileCompilationInfoTest, AddMethodsProfileMethodInfoInlineCaches) {
+ std::unique_ptr<const DexFile> dex(OpenTestDexFile("ManyMethods"));
+
+ ProfileCompilationInfo info;
+ MethodReference hot(dex.get(), 0);
+ MethodReference startup(dex.get(), 2);
+
+ // Add inline caches with the methods. The profile should record only the one for the hot method.
+ std::vector<TypeReference> types = {};
+ ProfileMethodInfo::ProfileInlineCache ic(/*dex_pc*/ 0, /*missing_types*/true, types);
+ std::vector<ProfileMethodInfo::ProfileInlineCache> inline_caches = {ic};
+ info.AddMethod(ProfileMethodInfo(hot, inline_caches), Hotness::kFlagHot);
+ info.AddMethod(ProfileMethodInfo(startup, inline_caches), Hotness::kFlagStartup);
+
+ // Check the hot method's inline cache.
+ std::unique_ptr<ProfileCompilationInfo::OfflineProfileMethodInfo> hot_pmi =
+ info.GetMethod(dex->GetLocation(), dex->GetLocationChecksum(), hot.index);
+ ASSERT_TRUE(hot_pmi != nullptr);
+ ASSERT_EQ(hot_pmi->inline_caches->size(), 1u);
+ ASSERT_TRUE(hot_pmi->inline_caches->Get(0).is_missing_types);
+
+ // Check there's no inline caches for the startup method.
+ ASSERT_TRUE(
+ info.GetMethod(dex->GetLocation(), dex->GetLocationChecksum(), startup.index) == nullptr);
+}
+
+// Verifies that we correctly add methods to the profile according to their flags.
+TEST_F(ProfileCompilationInfoTest, AddMethodsProfileMethodInfoFail) {
+ std::unique_ptr<const DexFile> dex(OpenTestDexFile("ManyMethods"));
+
+ ProfileCompilationInfo info;
+
+ MethodReference hot(dex.get(), 0);
+ MethodReference bad_ref(dex.get(), kMaxMethodIds);
+
+ std::vector<ProfileMethodInfo> pmis = {ProfileMethodInfo(hot), ProfileMethodInfo(bad_ref)};
+ ASSERT_FALSE(info.AddMethods(pmis, Hotness::kFlagHot));
+}
} // namespace art
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index a5a895d..03d08a8 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -45,6 +45,8 @@
namespace art {
+using Hotness = ProfileCompilationInfo::MethodHotness;
+
ProfileSaver* ProfileSaver::instance_ = nullptr;
pthread_t ProfileSaver::profiler_pthread_ = 0U;
@@ -438,7 +440,6 @@
&sampled_methods);
MutexLock mu(self, *Locks::profiler_lock_);
uint64_t total_number_of_profile_entries_cached = 0;
- using Hotness = ProfileCompilationInfo::MethodHotness;
for (const auto& it : tracked_dex_base_locations_) {
std::set<DexCacheResolvedClasses> resolved_classes_for_location;
@@ -568,8 +569,9 @@
// Try to add the method data. Note this may fail is the profile loaded from disk contains
// outdated data (e.g. the previous profiled dex files might have been updated).
// If this happens we clear the profile data and for the save to ensure the file is cleared.
- if (!info.AddMethods(profile_methods,
- ProfileCompilationInfo::MethodHotness::kFlagPostStartup)) {
+ if (!info.AddMethods(
+ profile_methods,
+ static_cast<Hotness::Flag>(Hotness::kFlagHot | Hotness::kFlagPostStartup))) {
LOG(WARNING) << "Could not add methods to the existing profiler. "
<< "Clearing the profile data.";
info.ClearData();