Merge "Address some review comments for aog/415919"

6 files changed, 54 insertions, 44 deletions

diff --git a/dexlayout/dexlayout.cc b/dexlayout/dexlayout.cc
index 22f0cb042e..f886de24b6 100644
--- a/dexlayout/dexlayout.cc
+++ b/dexlayout/dexlayout.cc
@@ -1557,7 +1557,7 @@ void DexLayout::LayoutStringData(const DexFile* dex_file) {
             (method->GetAccessFlags() & kAccConstructor) != 0 &&
             (method->GetAccessFlags() & kAccStatic) != 0;
         const bool method_executed = is_clinit ||
-            info_->GetMethodHotness(MethodReference(dex_file, method_id->GetIndex())).HasAnyFlags();
+            info_->GetMethodHotness(MethodReference(dex_file, method_id->GetIndex())).IsInProfile();
         if (!method_executed) {
           continue;
         }
@@ -1712,7 +1712,7 @@ int32_t DexLayout::LayoutCodeItems(const DexFile* dex_file,
           state = kCodeItemStateExecStartupOnly;
         } else if (is_clinit) {
           state = kCodeItemStateClinit;
-        } else if (hotness.HasAnyFlags()) {
+        } else if (hotness.IsInProfile()) {
           state = kCodeItemStateExec;
         }
         code_items[state].insert(code_item);
diff --git a/profman/profman.cc b/profman/profman.cc
index d8b5dafffe..f763b8ea05 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -849,7 +849,7 @@ class ProfMan FINAL {
       if (!profile->AddMethodIndex(static_cast<Hotness::Flag>(flags), ref)) {
         return false;
       }
-      DCHECK(profile->GetMethodHotness(ref).HasAnyFlags());
+      DCHECK(profile->GetMethodHotness(ref).IsInProfile());
     }
     return true;
   }
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index 175563ab9c..960030d577 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -1154,6 +1154,8 @@ bool ProfileCompilationInfo::MergeWith(const ProfileCompilationInfo& other) {
   // Note that the number of elements should be very small, so this should not
   // be a performance issue.
   for (const DexFileData* other_dex_data : other.info_) {
+    // verify_checksum is false because we want to differentiate between a missing dex data and
+    // a mismatched checksum.
     const DexFileData* dex_data = FindDexData(other_dex_data->profile_key,
                                               0u,
                                               /* verify_checksum */ false);
@@ -1251,11 +1253,11 @@ std::unique_ptr<ProfileCompilationInfo::OfflineProfileMethodInfo> ProfileCompila
     uint32_t dex_checksum,
     uint16_t dex_method_index) const {
   MethodHotness hotness(GetMethodHotness(dex_location, dex_checksum, dex_method_index));
-  const InlineCacheMap* inline_caches = hotness.GetInlineCacheMap();
-  if (inline_caches == nullptr) {
+  if (!hotness.IsHot()) {
     return nullptr;
   }
-
+  const InlineCacheMap* inline_caches = hotness.GetInlineCacheMap();
+  DCHECK(inline_caches != nullptr);
   std::unique_ptr<OfflineProfileMethodInfo> pmi(new OfflineProfileMethodInfo(inline_caches));
 
   pmi->dex_references.resize(info_.size());
@@ -1596,6 +1598,38 @@ ProfileCompilationInfo::DexFileData::FindOrAddMethod(uint16_t method_index) {
       InlineCacheMap(std::less<uint16_t>(), arena_->Adapter(kArenaAllocProfile)))->second);
 }
 
+// Mark a method as executed at least once.
+void ProfileCompilationInfo::DexFileData::AddMethod(MethodHotness::Flag flags, size_t index) {
+  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);
+  }
+  if ((flags & MethodHotness::kFlagHot) != 0) {
+    method_map.FindOrAdd(
+        index,
+        InlineCacheMap(std::less<uint16_t>(), arena_->Adapter(kArenaAllocProfile)));
+  }
+}
+
+ProfileCompilationInfo::MethodHotness ProfileCompilationInfo::DexFileData::GetHotnessInfo(
+    uint32_t dex_method_index) const {
+  MethodHotness ret;
+  if (method_bitmap.LoadBit(MethodBitIndex(/*startup*/ true, dex_method_index))) {
+    ret.AddFlag(MethodHotness::kFlagStartup);
+  }
+  if (method_bitmap.LoadBit(MethodBitIndex(/*startup*/ false, dex_method_index))) {
+    ret.AddFlag(MethodHotness::kFlagPostStartup);
+  }
+  auto it = method_map.find(dex_method_index);
+  if (it != method_map.end()) {
+    ret.SetInlineCacheMap(&it->second);
+    ret.AddFlag(MethodHotness::kFlagHot);
+  }
+  return ret;
+}
+
 ProfileCompilationInfo::DexPcData*
 ProfileCompilationInfo::FindOrAddDexPc(InlineCacheMap* inline_cache, uint32_t dex_pc) {
   return &(inline_cache->FindOrAdd(dex_pc, DexPcData(&arena_))->second);
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index b2d541f896..8d1e578875 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -198,10 +198,14 @@ class ProfileCompilationInfo {
       return flags_;
     }
 
-    bool HasAnyFlags() const {
+    bool IsInProfile() const {
       return flags_ != 0;
     }
 
+   private:
+    const InlineCacheMap* inline_cache_map_ = nullptr;
+    uint8_t flags_ = 0;
+
     const InlineCacheMap* GetInlineCacheMap() const {
       return inline_cache_map_;
     }
@@ -210,9 +214,7 @@ class ProfileCompilationInfo {
       inline_cache_map_ = info;
     }
 
-   private:
-    const InlineCacheMap* inline_cache_map_ = nullptr;
-    uint8_t flags_ = 0;
+    friend class ProfileCompilationInfo;
   };
 
   // Encodes the full set of inline caches for a given method.
@@ -421,19 +423,7 @@ class ProfileCompilationInfo {
     }
 
     // Mark a method as executed at least once.
-    void AddMethod(MethodHotness::Flag flags, size_t index) {
-      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);
-      }
-      if ((flags & MethodHotness::kFlagHot) != 0) {
-        method_map.FindOrAdd(
-            index,
-            InlineCacheMap(std::less<uint16_t>(), arena_->Adapter(kArenaAllocProfile)));
-      }
-    }
+    void AddMethod(MethodHotness::Flag flags, size_t index);
 
     void MergeBitmap(const DexFileData& other) {
       DCHECK_EQ(bitmap_storage.size(), other.bitmap_storage.size());
@@ -442,21 +432,7 @@ class ProfileCompilationInfo {
       }
     }
 
-    MethodHotness GetHotnessInfo(uint32_t dex_method_index) const {
-      MethodHotness ret;
-      if (method_bitmap.LoadBit(MethodBitIndex(/*startup*/ true, dex_method_index))) {
-        ret.AddFlag(MethodHotness::kFlagStartup);
-      }
-      if (method_bitmap.LoadBit(MethodBitIndex(/*startup*/ false, dex_method_index))) {
-        ret.AddFlag(MethodHotness::kFlagPostStartup);
-      }
-      auto it = method_map.find(dex_method_index);
-      if (it != method_map.end()) {
-        ret.SetInlineCacheMap(&it->second);
-        ret.AddFlag(MethodHotness::kFlagHot);
-      }
-      return ret;
-    }
+    MethodHotness GetHotnessInfo(uint32_t dex_method_index) const;
 
     // The arena used to allocate new inline cache maps.
     ArenaAllocator* arena_;
diff --git a/runtime/jit/profile_compilation_info_test.cc b/runtime/jit/profile_compilation_info_test.cc
index c3a34156ad..1ba98acab8 100644
--- a/runtime/jit/profile_compilation_info_test.cc
+++ b/runtime/jit/profile_compilation_info_test.cc
@@ -868,8 +868,8 @@ TEST_F(ProfileCompilationInfoTest, SampledMethodsTest) {
   test_info.AddMethodIndex(Hotness::kFlagStartup, kDex2, kChecksum2, 2, kNumMethods);
   test_info.AddMethodIndex(Hotness::kFlagPostStartup, kDex2, kChecksum2, 4, kNumMethods);
   auto run_test = [](const ProfileCompilationInfo& info) {
-    EXPECT_FALSE(info.GetMethodHotness(kDex1, kChecksum1, 2).HasAnyFlags());
-    EXPECT_FALSE(info.GetMethodHotness(kDex1, kChecksum1, 4).HasAnyFlags());
+    EXPECT_FALSE(info.GetMethodHotness(kDex1, kChecksum1, 2).IsInProfile());
+    EXPECT_FALSE(info.GetMethodHotness(kDex1, kChecksum1, 4).IsInProfile());
     EXPECT_TRUE(info.GetMethodHotness(kDex1, kChecksum1, 1).IsStartup());
     EXPECT_FALSE(info.GetMethodHotness(kDex1, kChecksum1, 3).IsStartup());
     EXPECT_TRUE(info.GetMethodHotness(kDex1, kChecksum1, 5).IsPostStartup());
@@ -931,9 +931,9 @@ TEST_F(ProfileCompilationInfoTest, SampledMethodsTest) {
     }
     EXPECT_TRUE(info.GetMethodHotness(MethodReference(dex.get(), 6)).IsPostStartup());
     // Check that methods that shouldn't have been touched are OK.
-    EXPECT_TRUE(info.GetMethodHotness(MethodReference(dex.get(), 0)).HasAnyFlags());
-    EXPECT_FALSE(info.GetMethodHotness(MethodReference(dex.get(), 4)).HasAnyFlags());
-    EXPECT_FALSE(info.GetMethodHotness(MethodReference(dex.get(), 7)).HasAnyFlags());
+    EXPECT_TRUE(info.GetMethodHotness(MethodReference(dex.get(), 0)).IsInProfile());
+    EXPECT_FALSE(info.GetMethodHotness(MethodReference(dex.get(), 4)).IsInProfile());
+    EXPECT_FALSE(info.GetMethodHotness(MethodReference(dex.get(), 7)).IsInProfile());
     EXPECT_FALSE(info.GetMethodHotness(MethodReference(dex.get(), 1)).IsPostStartup());
     EXPECT_FALSE(info.GetMethodHotness(MethodReference(dex.get(), 4)).IsStartup());
     EXPECT_FALSE(info.GetMethodHotness(MethodReference(dex.get(), 6)).IsStartup());
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index 94363c6f2d..b41bc78170 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -681,7 +681,7 @@ bool ProfileSaver::HasSeenMethod(const std::string& profile,
     if (!info.Load(profile, /*clear_if_invalid*/false)) {
       return false;
     }
-    return info.GetMethodHotness(MethodReference(dex_file, method_idx)).HasAnyFlags();
+    return info.GetMethodHotness(MethodReference(dex_file, method_idx)).IsInProfile();
   }
   return false;
 }