Address review comments for aog/411660
Added test for bulk adding method apis.
Test: test-art-host
Change-Id: Ib5b8c73e572110bccbbab031c11f030c23545fba
diff --git a/runtime/common_runtime_test.h b/runtime/common_runtime_test.h
index 1274a36..0197703 100644
--- a/runtime/common_runtime_test.h
+++ b/runtime/common_runtime_test.h
@@ -132,8 +132,7 @@
std::vector<std::unique_ptr<const DexFile>> OpenTestDexFiles(const char* name);
- std::unique_ptr<const DexFile> OpenTestDexFile(const char* name)
- REQUIRES_SHARED(Locks::mutator_lock_);
+ std::unique_ptr<const DexFile> OpenTestDexFile(const char* name);
jobject LoadDex(const char* dex_name) REQUIRES_SHARED(Locks::mutator_lock_);
jobject LoadMultiDex(const char* first_dex_name, const char* second_dex_name)
diff --git a/runtime/dex_reference_collection.h b/runtime/dex_reference_collection.h
index 76355d6..01b9b97 100644
--- a/runtime/dex_reference_collection.h
+++ b/runtime/dex_reference_collection.h
@@ -63,12 +63,13 @@
private:
DexFileMap map_;
- // Optimize for adding to same vector in succession.
const DexFile* current_dex_file_ = nullptr;
IndexVector* current_vector_ = nullptr;
VectorAllocator vector_allocator_;
ALWAYS_INLINE IndexVector* GetOrInsertVector(const DexFile* dex) {
+ // Optimize for adding to same vector in succession, the cached dex file and vector aims to
+ // prevent map lookups.
if (UNLIKELY(current_dex_file_ != dex)) {
// There is an assumption that constructing an empty vector wont do any allocations. If this
// incorrect, this might leak for the arena case.
diff --git a/runtime/jit/profile_compilation_info-inl.h b/runtime/jit/profile_compilation_info-inl.h
index 8a067a5..b71a95e 100644
--- a/runtime/jit/profile_compilation_info-inl.h
+++ b/runtime/jit/profile_compilation_info-inl.h
@@ -22,10 +22,11 @@
namespace art {
template <class Iterator>
-inline bool ProfileCompilationInfo::AddSampledMethodsForDex(bool startup,
- const DexFile* dex_file,
- Iterator index_begin,
- Iterator index_end) {
+inline bool ProfileCompilationInfo::AddMethodsForDex(bool startup,
+ bool hot,
+ const DexFile* dex_file,
+ Iterator index_begin,
+ Iterator index_end) {
DexFileData* data = GetOrAddDexFileData(dex_file);
if (data == nullptr) {
return false;
@@ -33,21 +34,9 @@
for (auto it = index_begin; it != index_end; ++it) {
DCHECK_LT(*it, data->num_method_ids);
data->AddSampledMethod(startup, *it);
- }
- return true;
-}
-
-template <class Iterator>
-inline bool ProfileCompilationInfo::AddHotMethodsForDex(const DexFile* dex_file,
- Iterator index_begin,
- Iterator index_end) {
- DexFileData* data = GetOrAddDexFileData(dex_file);
- if (data == nullptr) {
- return false;
- }
- for (auto it = index_begin; it != index_end; ++it) {
- DCHECK_LT(*it, data->num_method_ids);
- data->FindOrAddMethod(*it);
+ if (hot) {
+ data->FindOrAddMethod(*it);
+ }
}
return true;
}
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index ea27d3b..a67fb38 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -1223,6 +1223,15 @@
method_ref.dex_method_index);
}
+const ProfileCompilationInfo::DexFileData* ProfileCompilationInfo::FindDexData(
+ const DexFile* dex_file) const {
+ const DexFileData* dex_data = FindDexData(GetProfileDexFileKey(dex_file->GetLocation()));
+ if (dex_data == nullptr || !ChecksumMatch(*dex_file, dex_data->checksum)) {
+ return nullptr;
+ }
+ return dex_data;
+}
+
bool ProfileCompilationInfo::IsStartupOrHotMethod(const std::string& dex_location,
uint32_t dex_checksum,
uint16_t dex_method_index) const {
@@ -1238,6 +1247,15 @@
return method_it != methods.end();
}
+bool ProfileCompilationInfo::ContainsSampledMethod(bool startup,
+ const MethodReference& method_ref) const {
+ const DexFileData* dex_data = FindDexData(method_ref.dex_file);
+ if (dex_data == nullptr) {
+ return false;
+ }
+ return dex_data->HasSampledMethod(startup, method_ref.dex_method_index);
+}
+
bool ProfileCompilationInfo::ContainsHotMethod(const MethodReference& method_ref) const {
return FindMethod(method_ref.dex_file->GetLocation(),
method_ref.dex_file->GetLocationChecksum(),
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index bd1b9d6..7bcaffb 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -211,24 +211,22 @@
bool AddMethod(const ProfileMethodInfo& pmi);
// Add methods that have samples but are are not necessarily hot. These are partitioned into two
- // possibly intersecting sets startup and post startup.
+ // possibly intersecting sets startup and post startup. Sampled methods are used for layout but
+ // not necessarily determining what gets compiled.
bool AddSampledMethod(bool startup,
const std::string& dex_location,
uint32_t checksum,
uint16_t method_idx,
uint32_t num_method_ids);
- // Bulk add sampled methods for a single dex, fast since it only has one GetOrAddDexFileData call.
- template <class Iterator>
- bool AddSampledMethodsForDex(bool startup,
- const DexFile* dex_file,
- Iterator index_begin,
- Iterator index_end);
- // Bulk add hot methods for a single dex, fast since it only has one GetOrAddDexFileData call.
+ // Bulk add sampled methods and/or hot methods for a single dex, fast since it only has one
+ // GetOrAddDexFileData call.
template <class Iterator>
- bool AddHotMethodsForDex(const DexFile* dex_file,
- Iterator index_begin,
- Iterator index_end);
+ ALWAYS_INLINE bool AddMethodsForDex(bool startup,
+ bool hot,
+ const DexFile* dex_file,
+ Iterator index_begin,
+ Iterator index_end);
// Load profile information from the given file descriptor.
// If the current profile is non-empty the load will fail.
@@ -264,6 +262,10 @@
// Return true if the method reference iS present and hot in the profiling info.
bool ContainsHotMethod(const MethodReference& method_ref) const;
+
+ // Return true if the profile contains a startup or post startup method.
+ bool ContainsSampledMethod(bool startup, const MethodReference& method_ref) const;
+
// Return true if the class's type is present in the profiling info.
bool ContainsClass(const DexFile& dex_file, dex::TypeIndex type_idx) const;
@@ -358,7 +360,7 @@
class_set(std::less<dex::TypeIndex>(), arena->Adapter(kArenaAllocProfile)),
num_method_ids(num_methods),
bitmap_storage(arena->Adapter(kArenaAllocProfile)) {
- const size_t num_bits = num_method_ids * kBitmapCount;
+ const size_t num_bits = num_method_ids * kBitmapIndexCount;
bitmap_storage.resize(RoundUp(num_bits, kBitsPerByte) / kBitsPerByte);
if (!bitmap_storage.empty()) {
method_bitmap =
@@ -409,9 +411,9 @@
private:
enum BitmapIndex {
- kBitmapStartup,
- kBitmapPostStartup,
- kBitmapCount,
+ kBitmapIndexStartup,
+ kBitmapIndexPostStartup,
+ kBitmapIndexCount,
};
size_t MethodBitIndex(bool startup, size_t index) const {
@@ -420,8 +422,8 @@
// This compresses better than ([startup bit][post statup bit])*
return index + (startup
- ? kBitmapStartup * num_method_ids
- : kBitmapPostStartup * num_method_ids);
+ ? kBitmapIndexStartup * num_method_ids
+ : kBitmapIndexPostStartup * num_method_ids);
}
};
@@ -468,6 +470,10 @@
// doesn't contain the key.
const DexFileData* FindDexData(const std::string& profile_key) const;
+ // Return the dex data associated with the given dex file or null if the profile doesn't contain
+ // the key or the checksum mismatches.
+ const DexFileData* FindDexData(const DexFile* dex_file) const;
+
// Checks if the profile is empty.
bool IsEmpty() const;
diff --git a/runtime/jit/profile_compilation_info_test.cc b/runtime/jit/profile_compilation_info_test.cc
index 39670af..5528366 100644
--- a/runtime/jit/profile_compilation_info_test.cc
+++ b/runtime/jit/profile_compilation_info_test.cc
@@ -25,7 +25,7 @@
#include "mirror/class-inl.h"
#include "mirror/class_loader.h"
#include "handle_scope-inl.h"
-#include "jit/profile_compilation_info.h"
+#include "jit/profile_compilation_info-inl.h"
#include "linear_alloc.h"
#include "scoped_thread_state_change-inl.h"
#include "type_reference.h"
@@ -891,6 +891,49 @@
test_info.MergeWith(merge_info);
}
EXPECT_TRUE(test_info.IsStartupOrHotMethod(kDex1, kChecksum1, 11));
+
+ // Test bulk adding.
+ {
+ std::unique_ptr<const DexFile> dex(OpenTestDexFile("ManyMethods"));
+ ProfileCompilationInfo info;
+ std::vector<uint16_t> hot_methods = {1, 3, 5};
+ std::vector<uint16_t> startup_methods = {1, 2};
+ std::vector<uint16_t> post_methods = {0, 2, 6};
+ ASSERT_GE(dex->NumMethodIds(), 7u);
+ info.AddMethodsForDex(/*startup*/true,
+ /*hot*/true,
+ dex.get(),
+ hot_methods.begin(),
+ hot_methods.end());
+ info.AddMethodsForDex(/*startup*/true,
+ /*hot*/false,
+ dex.get(),
+ startup_methods.begin(),
+ startup_methods.end());
+ info.AddMethodsForDex(/*startup*/false,
+ /*hot*/false,
+ dex.get(),
+ post_methods.begin(),
+ post_methods.end());
+ for (uint16_t id : hot_methods) {
+ EXPECT_TRUE(info.ContainsHotMethod(MethodReference(dex.get(), id)));
+ EXPECT_TRUE(info.ContainsSampledMethod(/*startup*/true, MethodReference(dex.get(), id)));
+ }
+ for (uint16_t id : startup_methods) {
+ EXPECT_TRUE(info.ContainsSampledMethod(/*startup*/true, MethodReference(dex.get(), id)));
+ }
+ for (uint16_t id : post_methods) {
+ EXPECT_TRUE(info.ContainsSampledMethod(/*startup*/false, MethodReference(dex.get(), id)));
+ }
+ EXPECT_TRUE(info.ContainsSampledMethod(/*startup*/false, MethodReference(dex.get(), 6)));
+ // Check that methods that shouldn't have been touched are OK.
+ EXPECT_FALSE(info.ContainsHotMethod(MethodReference(dex.get(), 0)));
+ EXPECT_FALSE(info.ContainsHotMethod(MethodReference(dex.get(), 2)));
+ EXPECT_FALSE(info.ContainsHotMethod(MethodReference(dex.get(), 4)));
+ EXPECT_FALSE(info.ContainsSampledMethod(/*startup*/false, MethodReference(dex.get(), 1)));
+ EXPECT_FALSE(info.ContainsSampledMethod(/*startup*/true, MethodReference(dex.get(), 4)));
+ EXPECT_FALSE(info.ContainsSampledMethod(/*startup*/true, MethodReference(dex.get(), 6)));
+ }
}
} // namespace art
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index edce9cd..cf1bfe1 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -283,21 +283,23 @@
for (const auto& pair : hot_methods.GetMap()) {
const DexFile* const dex_file = pair.first;
if (locations.find(dex_file->GetBaseLocation()) != locations.end()) {
- cached_info->AddSampledMethodsForDex(/*startup*/ true,
- dex_file,
- pair.second.begin(),
- pair.second.end());
- // Adding hot methods is a bit slow, TODO: optimize.
- cached_info->AddHotMethodsForDex(dex_file, pair.second.begin(), pair.second.end());
+ const MethodReferenceCollection::IndexVector& indices = pair.second;
+ cached_info->AddMethodsForDex(/*startup*/ true,
+ /*hot*/ true,
+ dex_file,
+ indices.begin(),
+ indices.end());
}
}
for (const auto& pair : startup_methods.GetMap()) {
const DexFile* const dex_file = pair.first;
if (locations.find(dex_file->GetBaseLocation()) != locations.end()) {
- cached_info->AddSampledMethodsForDex(/*startup*/ true,
- dex_file,
- pair.second.begin(),
- pair.second.end());
+ const MethodReferenceCollection::IndexVector& indices = pair.second;
+ cached_info->AddMethodsForDex(/*startup*/ true,
+ /*hot*/ false,
+ dex_file,
+ indices.begin(),
+ indices.end());
}
}
for (const auto& pair : resolved_classes.GetMap()) {