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

6 files changed, 105 insertions, 49 deletions

diff --git a/profman/profile_assistant_test.cc b/profman/profile_assistant_test.cc
index 79310ac166..188d0b0a24 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 @@ class ProfileAssistantTest : public CommonRuntimeTest {
       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 @@ class ProfileAssistantTest : public CommonRuntimeTest {
                          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 @@ TEST_F(ProfileAssistantTest, TestProfileCreateWithInvalidData) {
   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 387ce8dfae..efb7fcfec6 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -895,6 +895,17 @@ class ProfMan FINAL {
       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 @@ class ProfMan FINAL {
         }
       }
       // 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 @@ class ProfMan FINAL {
     }
     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 de4d02edaf..dcb4a20b5f 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -168,9 +168,10 @@ bool ProfileCompilationInfo::AddMethodIndex(MethodHotness::Flag flags,
   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 @@ bool ProfileCompilationInfo::AddMethod(const std::string& dex_location,
                                        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 @@ bool ProfileCompilationInfo::AddMethod(const std::string& dex_location,
   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 @@ bool ProfileCompilationInfo::ReadMethods(SafeBuffer& buffer,
     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 @@ bool ProfileCompilationInfo::MergeWith(const ProfileCompilationInfo& other,
     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 @@ bool ProfileCompilationInfo::IsEmpty() const {
 
 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 @@ bool ProfileCompilationInfo::DexFileData::AddMethod(MethodHotness::Flag flags, s
     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 @@ bool ProfileCompilationInfo::DexFileData::AddMethod(MethodHotness::Flag flags, s
   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 1973f3f09e..5488a9e81e 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -241,7 +241,7 @@ class ProfileCompilationInfo {
   ~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 @@ class ProfileCompilationInfo {
   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 @@ class ProfileCompilationInfo {
       }
     }
 
+    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 @@ class ProfileCompilationInfo {
                  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 4ac11ee422..e6917956ae 100644
--- a/runtime/jit/profile_compilation_info_test.cc
+++ b/runtime/jit/profile_compilation_info_test.cc
@@ -80,7 +80,8 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest {
                  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 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest {
   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 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest {
       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 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest {
   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 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest {
       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 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethods) {
   // 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 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethods) {
   {
     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 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethods) {
   {
     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 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethodsWithInlineCaches) {
 
   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 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethodsWithInlineCaches) {
   {
     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 8f0ac33594..53f48644f2 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -511,7 +511,7 @@ bool ProfileSaver::ProcessProfilingInfo(bool force_save, /*out*/uint16_t* number
       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));