Ensure that we always set the method hotness in the profile
The method hotness were not recorded for methods extracted from the JIT
code cache.
Test: gtest & run-test
Bug: 71588770
Change-Id: Ifdf6340caa9faf5adb6f3b3b5b4046f31f34189c
diff --git a/profman/profile_assistant_test.cc b/profman/profile_assistant_test.cc
index 79310ac..188d0b0 100644
--- a/profman/profile_assistant_test.cc
+++ b/profman/profile_assistant_test.cc
@@ -31,6 +31,8 @@
namespace art {
+using Hotness = ProfileCompilationInfo::MethodHotness;
+
static constexpr size_t kMaxMethodIds = 65535;
class ProfileAssistantTest : public CommonRuntimeTest {
@@ -80,12 +82,17 @@
ProfileCompilationInfo::OfflineProfileMethodInfo pmi =
GetOfflineProfileMethodInfo(dex_location1, dex_location_checksum1,
dex_location2, dex_location_checksum2);
+ Hotness::Flag flags = Hotness::kFlagPostStartup;
if (reverse_dex_write_order) {
- ASSERT_TRUE(info->AddMethod(dex_location2, dex_location_checksum2, i, kMaxMethodIds, pmi));
- ASSERT_TRUE(info->AddMethod(dex_location1, dex_location_checksum1, i, kMaxMethodIds, pmi));
+ ASSERT_TRUE(info->AddMethod(
+ dex_location2, dex_location_checksum2, i, kMaxMethodIds, pmi, flags));
+ ASSERT_TRUE(info->AddMethod(
+ dex_location1, dex_location_checksum1, i, kMaxMethodIds, pmi, flags));
} else {
- ASSERT_TRUE(info->AddMethod(dex_location1, dex_location_checksum1, i, kMaxMethodIds, pmi));
- ASSERT_TRUE(info->AddMethod(dex_location2, dex_location_checksum2, i, kMaxMethodIds, pmi));
+ ASSERT_TRUE(info->AddMethod(
+ dex_location1, dex_location_checksum1, i, kMaxMethodIds, pmi, flags));
+ ASSERT_TRUE(info->AddMethod(
+ dex_location2, dex_location_checksum2, i, kMaxMethodIds, pmi, flags));
}
}
for (uint16_t i = 0; i < number_of_classes; i++) {
@@ -109,7 +116,6 @@
const ScratchFile& profile,
ProfileCompilationInfo* info) {
std::string dex_location = "location1" + id;
- using Hotness = ProfileCompilationInfo::MethodHotness;
for (uint32_t idx : hot_methods) {
info->AddMethodIndex(Hotness::kFlagHot, dex_location, checksum, idx, number_of_methods);
}
@@ -1086,10 +1092,10 @@
ASSERT_EQ(1u, classes.size());
ASSERT_TRUE(classes.find(invalid_class_index) != classes.end());
- // Verify that the invalid method is in the profile.
- ASSERT_EQ(2u, hot_methods.size());
+ // Verify that the invalid method did not get in the profile.
+ ASSERT_EQ(1u, hot_methods.size());
uint16_t invalid_method_index = std::numeric_limits<uint16_t>::max() - 1;
- ASSERT_TRUE(hot_methods.find(invalid_method_index) != hot_methods.end());
+ ASSERT_FALSE(hot_methods.find(invalid_method_index) != hot_methods.end());
}
TEST_F(ProfileAssistantTest, DumpOnly) {
diff --git a/profman/profman.cc b/profman/profman.cc
index 387ce8d..efb7fcf 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -895,6 +895,17 @@
method_str = line.substr(method_sep_index + kMethodSep.size());
}
+ uint32_t flags = 0;
+ if (is_hot) {
+ flags |= ProfileCompilationInfo::MethodHotness::kFlagHot;
+ }
+ if (is_startup) {
+ flags |= ProfileCompilationInfo::MethodHotness::kFlagStartup;
+ }
+ if (is_post_startup) {
+ flags |= ProfileCompilationInfo::MethodHotness::kFlagPostStartup;
+ }
+
TypeReference class_ref(/* dex_file */ nullptr, dex::TypeIndex());
if (!FindClass(dex_files, klass, &class_ref)) {
LOG(WARNING) << "Could not find class: " << klass;
@@ -930,7 +941,7 @@
}
}
// TODO: Check return values?
- profile->AddMethods(methods);
+ profile->AddMethods(methods, static_cast<ProfileCompilationInfo::MethodHotness::Flag>(flags));
profile->AddClasses(resolved_class_set);
return true;
}
@@ -982,18 +993,12 @@
}
MethodReference ref(class_ref.dex_file, method_index);
if (is_hot) {
- profile->AddMethod(ProfileMethodInfo(ref, inline_caches));
- }
- uint32_t flags = 0;
- using Hotness = ProfileCompilationInfo::MethodHotness;
- if (is_startup) {
- flags |= Hotness::kFlagStartup;
- }
- if (is_post_startup) {
- flags |= Hotness::kFlagPostStartup;
+ profile->AddMethod(ProfileMethodInfo(ref, inline_caches),
+ static_cast<ProfileCompilationInfo::MethodHotness::Flag>(flags));
}
if (flags != 0) {
- if (!profile->AddMethodIndex(static_cast<Hotness::Flag>(flags), ref)) {
+ if (!profile->AddMethodIndex(
+ static_cast<ProfileCompilationInfo::MethodHotness::Flag>(flags), ref)) {
return false;
}
DCHECK(profile->GetMethodHotness(ref).IsInProfile());
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index de4d02e..dcb4a20 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -168,9 +168,10 @@
return data->AddMethod(flags, method_idx);
}
-bool ProfileCompilationInfo::AddMethods(const std::vector<ProfileMethodInfo>& methods) {
+bool ProfileCompilationInfo::AddMethods(const std::vector<ProfileMethodInfo>& methods,
+ MethodHotness::Flag flags) {
for (const ProfileMethodInfo& method : methods) {
- if (!AddMethod(method)) {
+ if (!AddMethod(method, flags)) {
return false;
}
}
@@ -644,15 +645,26 @@
uint32_t dex_checksum,
uint16_t method_index,
uint32_t num_method_ids,
- const OfflineProfileMethodInfo& pmi) {
+ const OfflineProfileMethodInfo& pmi,
+ MethodHotness::Flag flags) {
DexFileData* const data = GetOrAddDexFileData(GetProfileDexFileKey(dex_location),
dex_checksum,
num_method_ids);
- if (data == nullptr) { // checksum mismatch
+ if (data == nullptr) {
+ // 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) {
+ // 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;
+ }
+
+ data->SetMethodHotness(method_index, flags);
if (pmi.inline_caches == nullptr) {
// If we don't have inline caches return success right away.
@@ -691,12 +703,16 @@
return true;
}
-bool ProfileCompilationInfo::AddMethod(const ProfileMethodInfo& pmi) {
+bool ProfileCompilationInfo::AddMethod(const ProfileMethodInfo& pmi, MethodHotness::Flag flags) {
DexFileData* const data = GetOrAddDexFileData(pmi.ref.dex_file);
if (data == nullptr) { // checksum mismatch
return false;
}
InlineCacheMap* inline_cache = data->FindOrAddMethod(pmi.ref.index);
+ if (inline_cache == nullptr) {
+ return false;
+ }
+ data->SetMethodHotness(pmi.ref.index, flags);
for (const ProfileMethodInfo::ProfileInlineCache& cache : pmi.inline_caches) {
if (cache.is_missing_types) {
@@ -811,6 +827,9 @@
uint16_t method_index = last_method_index + diff_with_last_method_index;
last_method_index = method_index;
InlineCacheMap* inline_cache = data->FindOrAddMethod(method_index);
+ if (inline_cache == nullptr) {
+ return false;
+ }
if (!ReadInlineCache(buffer,
number_of_dex_files,
dex_profile_index_remap,
@@ -1521,6 +1540,9 @@
for (const auto& other_method_it : other_dex_data->method_map) {
uint16_t other_method_index = other_method_it.first;
InlineCacheMap* inline_cache = dex_data->FindOrAddMethod(other_method_index);
+ if (inline_cache == nullptr) {
+ return false;
+ }
const auto& other_inline_cache = other_method_it.second;
for (const auto& other_ic_it : other_inline_cache) {
uint16_t other_dex_pc = other_ic_it.first;
@@ -1955,6 +1977,10 @@
ProfileCompilationInfo::InlineCacheMap*
ProfileCompilationInfo::DexFileData::FindOrAddMethod(uint16_t method_index) {
+ if (method_index >= num_method_ids) {
+ LOG(ERROR) << "Invalid method index " << method_index << ". num_method_ids=" << num_method_ids;
+ return nullptr;
+ }
return &(method_map.FindOrAdd(
method_index,
InlineCacheMap(std::less<uint16_t>(), allocator_->Adapter(kArenaAllocProfile)))->second);
@@ -1967,12 +1993,8 @@
return false;
}
- if ((flags & MethodHotness::kFlagStartup) != 0) {
- method_bitmap.StoreBit(MethodBitIndex(/*startup*/ true, index), /*value*/ true);
- }
- if ((flags & MethodHotness::kFlagPostStartup) != 0) {
- method_bitmap.StoreBit(MethodBitIndex(/*startup*/ false, index), /*value*/ true);
- }
+ SetMethodHotness(index, flags);
+
if ((flags & MethodHotness::kFlagHot) != 0) {
method_map.FindOrAdd(
index,
@@ -1981,6 +2003,17 @@
return true;
}
+void ProfileCompilationInfo::DexFileData::SetMethodHotness(size_t index,
+ MethodHotness::Flag flags) {
+ DCHECK_LT(index, num_method_ids);
+ if ((flags & MethodHotness::kFlagStartup) != 0) {
+ method_bitmap.StoreBit(MethodBitIndex(/*startup*/ true, index), /*value*/ true);
+ }
+ if ((flags & MethodHotness::kFlagPostStartup) != 0) {
+ method_bitmap.StoreBit(MethodBitIndex(/*startup*/ false, index), /*value*/ true);
+ }
+}
+
ProfileCompilationInfo::MethodHotness ProfileCompilationInfo::DexFileData::GetHotnessInfo(
uint32_t dex_method_index) const {
MethodHotness ret;
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index 1973f3f..5488a9e 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -241,7 +241,7 @@
~ProfileCompilationInfo();
// Add the given methods to the current profile object.
- bool AddMethods(const std::vector<ProfileMethodInfo>& methods);
+ bool AddMethods(const std::vector<ProfileMethodInfo>& methods, MethodHotness::Flag flags);
// Add the given classes to the current profile object.
bool AddClasses(const std::set<DexCacheResolvedClasses>& resolved_classes);
@@ -278,7 +278,7 @@
bool AddMethodIndex(MethodHotness::Flag flags, const MethodReference& ref);
// Add a method to the profile using its online representation (containing runtime structures).
- bool AddMethod(const ProfileMethodInfo& pmi);
+ bool AddMethod(const ProfileMethodInfo& pmi, MethodHotness::Flag flags);
// Bulk add sampled methods and/or hot methods for a single dex, fast since it only has one
// GetOrAddDexFileData call.
@@ -500,6 +500,7 @@
}
}
+ void SetMethodHotness(size_t index, MethodHotness::Flag flags);
MethodHotness GetHotnessInfo(uint32_t dex_method_index) const;
// The allocator used to allocate new inline cache maps.
@@ -559,7 +560,8 @@
uint32_t dex_checksum,
uint16_t method_index,
uint32_t num_method_ids,
- const OfflineProfileMethodInfo& pmi);
+ const OfflineProfileMethodInfo& pmi,
+ MethodHotness::Flag flags);
// Add a class index to the profile.
bool AddClassIndex(const std::string& dex_location,
diff --git a/runtime/jit/profile_compilation_info_test.cc b/runtime/jit/profile_compilation_info_test.cc
index 4ac11ee..e691795 100644
--- a/runtime/jit/profile_compilation_info_test.cc
+++ b/runtime/jit/profile_compilation_info_test.cc
@@ -80,7 +80,8 @@
uint16_t method_index,
const ProfileCompilationInfo::OfflineProfileMethodInfo& pmi,
ProfileCompilationInfo* info) {
- return info->AddMethod(dex_location, checksum, method_index, kMaxMethodIds, pmi);
+ return info->AddMethod(
+ dex_location, checksum, method_index, kMaxMethodIds, pmi, Hotness::kFlagPostStartup);
}
bool AddClass(const std::string& dex_location,
@@ -99,7 +100,8 @@
bool SaveProfilingInfo(
const std::string& filename,
const std::vector<ArtMethod*>& methods,
- const std::set<DexCacheResolvedClasses>& resolved_classes) {
+ const std::set<DexCacheResolvedClasses>& resolved_classes,
+ Hotness::Flag flags) {
ProfileCompilationInfo info;
std::vector<ProfileMethodInfo> profile_methods;
ScopedObjectAccess soa(Thread::Current());
@@ -107,7 +109,7 @@
profile_methods.emplace_back(
MethodReference(method->GetDexFile(), method->GetDexMethodIndex()));
}
- if (!info.AddMethods(profile_methods) || !info.AddClasses(resolved_classes)) {
+ if (!info.AddMethods(profile_methods, flags) || !info.AddClasses(resolved_classes)) {
return false;
}
if (info.GetNumberOfMethods() != profile_methods.size()) {
@@ -130,6 +132,7 @@
bool SaveProfilingInfoWithFakeInlineCaches(
const std::string& filename,
const std::vector<ArtMethod*>& methods,
+ Hotness::Flag flags,
/*out*/ SafeMap<ArtMethod*, ProfileMethodInfo>* profile_methods_map) {
ProfileCompilationInfo info;
std::vector<ProfileMethodInfo> profile_methods;
@@ -170,7 +173,8 @@
profile_methods_map->Put(method, pmi);
}
- if (!info.AddMethods(profile_methods) || info.GetNumberOfMethods() != profile_methods.size()) {
+ if (!info.AddMethods(profile_methods, flags)
+ || info.GetNumberOfMethods() != profile_methods.size()) {
return false;
}
return info.Save(filename, nullptr);
@@ -345,7 +349,8 @@
// Save virtual methods from Main.
std::set<DexCacheResolvedClasses> resolved_classes;
std::vector<ArtMethod*> main_methods = GetVirtualMethods(class_loader, "LMain;");
- ASSERT_TRUE(SaveProfilingInfo(profile.GetFilename(), main_methods, resolved_classes));
+ ASSERT_TRUE(SaveProfilingInfo(
+ profile.GetFilename(), main_methods, resolved_classes, Hotness::kFlagPostStartup));
// Check that what we saved is in the profile.
ProfileCompilationInfo info1;
@@ -354,14 +359,16 @@
{
ScopedObjectAccess soa(self);
for (ArtMethod* m : main_methods) {
- ASSERT_TRUE(info1.GetMethodHotness(
- MethodReference(m->GetDexFile(), m->GetDexMethodIndex())).IsHot());
+ Hotness h = info1.GetMethodHotness(MethodReference(m->GetDexFile(), m->GetDexMethodIndex()));
+ ASSERT_TRUE(h.IsHot());
+ ASSERT_TRUE(h.IsPostStartup());
}
}
// Save virtual methods from Second.
std::vector<ArtMethod*> second_methods = GetVirtualMethods(class_loader, "LSecond;");
- ASSERT_TRUE(SaveProfilingInfo(profile.GetFilename(), second_methods, resolved_classes));
+ ASSERT_TRUE(SaveProfilingInfo(
+ profile.GetFilename(), second_methods, resolved_classes, Hotness::kFlagStartup));
// Check that what we saved is in the profile (methods form Main and Second).
ProfileCompilationInfo info2;
@@ -371,12 +378,14 @@
{
ScopedObjectAccess soa(self);
for (ArtMethod* m : main_methods) {
- ASSERT_TRUE(
- info2.GetMethodHotness(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())).IsHot());
+ Hotness h = info2.GetMethodHotness(MethodReference(m->GetDexFile(), m->GetDexMethodIndex()));
+ ASSERT_TRUE(h.IsHot());
+ ASSERT_TRUE(h.IsPostStartup());
}
for (ArtMethod* m : second_methods) {
- ASSERT_TRUE(
- info2.GetMethodHotness(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())).IsHot());
+ Hotness h = info2.GetMethodHotness(MethodReference(m->GetDexFile(), m->GetDexMethodIndex()));
+ ASSERT_TRUE(h.IsHot());
+ ASSERT_TRUE(h.IsStartup());
}
}
}
@@ -730,7 +739,7 @@
SafeMap<ArtMethod*, ProfileMethodInfo> profile_methods_map;
ASSERT_TRUE(SaveProfilingInfoWithFakeInlineCaches(
- profile.GetFilename(), main_methods, &profile_methods_map));
+ profile.GetFilename(), main_methods, Hotness::kFlagStartup, &profile_methods_map));
// Check that what we saved is in the profile.
ProfileCompilationInfo info;
@@ -739,8 +748,9 @@
{
ScopedObjectAccess soa(self);
for (ArtMethod* m : main_methods) {
- ASSERT_TRUE(
- info.GetMethodHotness(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())).IsHot());
+ Hotness h = info.GetMethodHotness(MethodReference(m->GetDexFile(), m->GetDexMethodIndex()));
+ ASSERT_TRUE(h.IsHot());
+ ASSERT_TRUE(h.IsStartup());
const ProfileMethodInfo& pmi = profile_methods_map.find(m)->second;
std::unique_ptr<ProfileCompilationInfo::OfflineProfileMethodInfo> offline_pmi =
info.GetMethod(m->GetDexFile()->GetLocation(),
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index 8f0ac33..53f4864 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -511,7 +511,7 @@
uint64_t last_save_number_of_methods = info.GetNumberOfMethods();
uint64_t last_save_number_of_classes = info.GetNumberOfResolvedClasses();
- info.AddMethods(profile_methods);
+ info.AddMethods(profile_methods, ProfileCompilationInfo::MethodHotness::kFlagPostStartup);
auto profile_cache_it = profile_cache_.find(filename);
if (profile_cache_it != profile_cache_.end()) {
info.MergeWith(*(profile_cache_it->second));