Fix profile merges in profman
profman used ProfileCompilationInfo::Load() which was not preserving the
correct order of the dex files (in a multidex profile).
The CL fixes profman to use ProfileCompilationInfo::MergeWith which
guarantees the right dex order and redesigns profile storage to avoid
such mistakes in the future. Instead of keeping data in a map indexed by
the profile key, store it in a vector whose index match profile_index.
This way, any iteration over profile info becomes deterministic with
respect to the profile index of the dex files.
Test: m test-art-host-gtest-profile_assistant_test
m test-art-host-gtest-profile_compilation_info_test
profile YouTube.apk and compile it based on the reference profile
(failing before)
Bug: 36371709
Change-Id: Ideda1336e5aff59a7c5560429da645fe02c804c9
diff --git a/profman/profile_assistant.cc b/profman/profile_assistant.cc
index a25460e..b9a85bc 100644
--- a/profman/profile_assistant.cc
+++ b/profman/profile_assistant.cc
@@ -44,10 +44,15 @@
// Merge all current profiles.
for (size_t i = 0; i < profile_files.size(); i++) {
- if (!info.Load(profile_files[i].GetFile()->Fd())) {
+ ProfileCompilationInfo cur_info;
+ if (!cur_info.Load(profile_files[i].GetFile()->Fd())) {
LOG(WARNING) << "Could not load profile file at index " << i;
return kErrorBadProfiles;
}
+ if (!info.MergeWith(cur_info)) {
+ LOG(WARNING) << "Could not merge profile file at index " << i;
+ return kErrorBadProfiles;
+ }
}
// Check if there is enough new information added by the current profiles.
diff --git a/profman/profile_assistant_test.cc b/profman/profile_assistant_test.cc
index e2f622d..94f6e70 100644
--- a/profman/profile_assistant_test.cc
+++ b/profman/profile_assistant_test.cc
@@ -37,14 +37,25 @@
uint16_t number_of_classes,
const ScratchFile& profile,
ProfileCompilationInfo* info,
- uint16_t start_method_index = 0) {
+ uint16_t start_method_index = 0,
+ bool reverse_dex_write_order = false) {
std::string dex_location1 = "location1" + id;
uint32_t dex_location_checksum1 = checksum;
std::string dex_location2 = "location2" + id;
uint32_t dex_location_checksum2 = 10 * checksum;
for (uint16_t i = start_method_index; i < start_method_index + number_of_methods; i++) {
- ASSERT_TRUE(info->AddMethodIndex(dex_location1, dex_location_checksum1, i));
- ASSERT_TRUE(info->AddMethodIndex(dex_location2, dex_location_checksum2, i));
+ // reverse_dex_write_order controls the order in which the dex files will be added to
+ // the profile and thus written to disk.
+ ProfileCompilationInfo::OfflineProfileMethodInfo pmi =
+ GetOfflineProfileMethodInfo(dex_location1, dex_location_checksum1,
+ dex_location2, dex_location_checksum2);
+ if (reverse_dex_write_order) {
+ ASSERT_TRUE(info->AddMethod(dex_location2, dex_location_checksum2, i, pmi));
+ ASSERT_TRUE(info->AddMethod(dex_location1, dex_location_checksum1, i, pmi));
+ } else {
+ ASSERT_TRUE(info->AddMethod(dex_location1, dex_location_checksum1, i, pmi));
+ ASSERT_TRUE(info->AddMethod(dex_location2, dex_location_checksum2, i, pmi));
+ }
}
for (uint16_t i = 0; i < number_of_classes; i++) {
ASSERT_TRUE(info->AddClassIndex(dex_location1, dex_location_checksum1, dex::TypeIndex(i)));
@@ -55,6 +66,43 @@
ASSERT_TRUE(profile.GetFile()->ResetOffset());
}
+ ProfileCompilationInfo::OfflineProfileMethodInfo GetOfflineProfileMethodInfo(
+ const std::string& dex_location1, uint32_t dex_checksum1,
+ const std::string& dex_location2, uint32_t dex_checksum2) {
+ ProfileCompilationInfo::OfflineProfileMethodInfo pmi;
+ pmi.dex_references.emplace_back(dex_location1, dex_checksum1);
+ pmi.dex_references.emplace_back(dex_location2, dex_checksum2);
+
+ // Monomorphic
+ for (uint16_t dex_pc = 0; dex_pc < 11; dex_pc++) {
+ ProfileCompilationInfo::DexPcData dex_pc_data;
+ dex_pc_data.AddClass(0, dex::TypeIndex(0));
+ pmi.inline_caches.Put(dex_pc, dex_pc_data);
+ }
+ // Polymorphic
+ for (uint16_t dex_pc = 11; dex_pc < 22; dex_pc++) {
+ ProfileCompilationInfo::DexPcData dex_pc_data;
+ dex_pc_data.AddClass(0, dex::TypeIndex(0));
+ dex_pc_data.AddClass(1, dex::TypeIndex(1));
+
+ pmi.inline_caches.Put(dex_pc, dex_pc_data);
+ }
+ // Megamorphic
+ for (uint16_t dex_pc = 22; dex_pc < 33; dex_pc++) {
+ ProfileCompilationInfo::DexPcData dex_pc_data;
+ dex_pc_data.SetIsMegamorphic();
+ pmi.inline_caches.Put(dex_pc, dex_pc_data);
+ }
+ // Missing types
+ for (uint16_t dex_pc = 33; dex_pc < 44; dex_pc++) {
+ ProfileCompilationInfo::DexPcData dex_pc_data;
+ dex_pc_data.SetIsMissingTypes();
+ pmi.inline_caches.Put(dex_pc, dex_pc_data);
+ }
+
+ return pmi;
+ }
+
int GetFd(const ScratchFile& file) const {
return static_cast<int>(file.GetFd());
}
@@ -652,4 +700,44 @@
}
}
+TEST_F(ProfileAssistantTest, MergeProfilesWithDifferentDexOrder) {
+ ScratchFile profile1;
+ ScratchFile reference_profile;
+
+ std::vector<int> profile_fds({GetFd(profile1)});
+ int reference_profile_fd = GetFd(reference_profile);
+
+ // The new profile info will contain the methods with indices 0-100.
+ const uint16_t kNumberOfMethodsToEnableCompilation = 100;
+ ProfileCompilationInfo info1;
+ SetupProfile("p1", 1, kNumberOfMethodsToEnableCompilation, 0, profile1, &info1,
+ /*start_method_index*/0, /*reverse_dex_write_order*/false);
+
+ // The reference profile info will contain the methods with indices 50-150.
+ // When setting up the profile reverse the order in which the dex files
+ // are added to the profile. This will verify that profman merges profiles
+ // with a different dex order correctly.
+ const uint16_t kNumberOfMethodsAlreadyCompiled = 100;
+ ProfileCompilationInfo reference_info;
+ SetupProfile("p1", 1, kNumberOfMethodsAlreadyCompiled, 0, reference_profile,
+ &reference_info, kNumberOfMethodsToEnableCompilation / 2, /*reverse_dex_write_order*/true);
+
+ // We should advise compilation.
+ ASSERT_EQ(ProfileAssistant::kCompile,
+ ProcessProfiles(profile_fds, reference_profile_fd));
+
+ // The resulting compilation info must be equal to the merge of the inputs.
+ ProfileCompilationInfo result;
+ ASSERT_TRUE(reference_profile.GetFile()->ResetOffset());
+ ASSERT_TRUE(result.Load(reference_profile_fd));
+
+ ProfileCompilationInfo expected;
+ ASSERT_TRUE(expected.MergeWith(reference_info));
+ ASSERT_TRUE(expected.MergeWith(info1));
+ ASSERT_TRUE(expected.Equals(result));
+
+ // The information from profile must remain the same.
+ CheckProfileInfo(profile1, info1);
+}
+
} // namespace art
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index f951065..13dbc3f 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -57,6 +57,14 @@
static_assert(InlineCache::kIndividualCacheSize < kIsMissingTypesEncoding,
"InlineCache::kIndividualCacheSize is larger than expected");
+ProfileCompilationInfo::ProfileCompilationInfo(const ProfileCompilationInfo& pci) {
+ MergeWith(pci);
+}
+
+ProfileCompilationInfo::~ProfileCompilationInfo() {
+ ClearProfile();
+}
+
void ProfileCompilationInfo::DexPcData::AddClass(uint16_t dex_profile_idx,
const dex::TypeIndex& type_idx) {
if (is_megamorphic || is_missing_types) {
@@ -227,28 +235,21 @@
DCHECK_LE(info_.size(), std::numeric_limits<uint8_t>::max());
AddUintToBuffer(&buffer, static_cast<uint8_t>(info_.size()));
- // Make sure we write the dex files in order of their profile index. This
+ // Dex files must be written in the order of their profile index. This
// avoids writing the index in the output file and simplifies the parsing logic.
- std::vector<const std::string*> ordered_info_location(info_.size());
- std::vector<const DexFileData*> ordered_info_data(info_.size());
- for (const auto& it : info_) {
- ordered_info_location[it.second.profile_index] = &(it.first);
- ordered_info_data[it.second.profile_index] = &(it.second);
- }
- for (size_t i = 0; i < info_.size(); i++) {
+ for (const DexFileData* dex_data_ptr : info_) {
+ const DexFileData& dex_data = *dex_data_ptr;
if (buffer.size() > kMaxSizeToKeepBeforeWriting) {
if (!WriteBuffer(fd, buffer.data(), buffer.size())) {
return false;
}
buffer.clear();
}
- const std::string& dex_location = *ordered_info_location[i];
- const DexFileData& dex_data = *ordered_info_data[i];
// Note that we allow dex files without any methods or classes, so that
// inline caches can refer valid dex files.
- if (dex_location.size() >= kMaxDexFileKeyLength) {
+ if (dex_data.profile_key.size() >= kMaxDexFileKeyLength) {
LOG(WARNING) << "DexFileKey exceeds allocated limit";
return false;
}
@@ -258,19 +259,19 @@
uint32_t methods_region_size = GetMethodsRegionSize(dex_data);
size_t required_capacity = buffer.size() +
kLineHeaderSize +
- dex_location.size() +
+ dex_data.profile_key.size() +
sizeof(uint16_t) * dex_data.class_set.size() +
methods_region_size;
buffer.reserve(required_capacity);
- DCHECK_LE(dex_location.size(), std::numeric_limits<uint16_t>::max());
+ DCHECK_LE(dex_data.profile_key.size(), std::numeric_limits<uint16_t>::max());
DCHECK_LE(dex_data.class_set.size(), std::numeric_limits<uint16_t>::max());
- AddUintToBuffer(&buffer, static_cast<uint16_t>(dex_location.size()));
+ AddUintToBuffer(&buffer, static_cast<uint16_t>(dex_data.profile_key.size()));
AddUintToBuffer(&buffer, static_cast<uint16_t>(dex_data.class_set.size()));
AddUintToBuffer(&buffer, methods_region_size); // uint32_t
AddUintToBuffer(&buffer, dex_data.checksum); // uint32_t
- AddStringToBuffer(&buffer, dex_location);
+ AddStringToBuffer(&buffer, dex_data.profile_key);
for (const auto& method_it : dex_data.method_map) {
AddUintToBuffer(&buffer, method_it.first);
@@ -375,23 +376,52 @@
}
ProfileCompilationInfo::DexFileData* ProfileCompilationInfo::GetOrAddDexFileData(
- const std::string& dex_location,
+ const std::string& profile_key,
uint32_t checksum) {
- auto info_it = info_.FindOrAdd(dex_location, DexFileData(checksum, info_.size()));
- if (info_.size() > std::numeric_limits<uint8_t>::max()) {
+ const auto& profile_index_it = profile_key_map_.FindOrAdd(profile_key, profile_key_map_.size());
+ if (profile_key_map_.size() > std::numeric_limits<uint8_t>::max()) {
// Allow only 255 dex files to be profiled. This allows us to save bytes
// when encoding. The number is well above what we expect for normal applications.
if (kIsDebugBuild) {
- LOG(WARNING) << "Exceeded the maximum number of dex files (255). Something went wrong";
+ LOG(ERROR) << "Exceeded the maximum number of dex files (255). Something went wrong";
}
- info_.erase(dex_location);
+ profile_key_map_.erase(profile_key);
return nullptr;
}
- if (info_it->second.checksum != checksum) {
- LOG(WARNING) << "Checksum mismatch for dex " << dex_location;
+
+ uint8_t profile_index = profile_index_it->second;
+ if (info_.size() <= profile_index) {
+ // This is a new addition. Add it to the info_ array.
+ info_.emplace_back(new DexFileData(profile_key, checksum, profile_index));
+ }
+ DexFileData* result = info_[profile_index];
+ // DCHECK that profile info map key is consistent with the one stored in the dex file data.
+ // This should always be the case since since the cache map is managed by ProfileCompilationInfo.
+ DCHECK_EQ(profile_key, result->profile_key);
+ DCHECK_EQ(profile_index, result->profile_index);
+
+ // Check that the checksum matches.
+ // This may different if for example the dex file was updated and
+ // we had a record of the old one.
+ if (result->checksum != checksum) {
+ LOG(WARNING) << "Checksum mismatch for dex " << profile_key;
return nullptr;
}
- return &info_it->second;
+ return result;
+}
+
+const ProfileCompilationInfo::DexFileData* ProfileCompilationInfo::FindDexData(
+ const std::string& profile_key) const {
+ const auto& profile_index_it = profile_key_map_.find(profile_key);
+ if (profile_index_it == profile_key_map_.end()) {
+ return nullptr;
+ }
+
+ uint8_t profile_index = profile_index_it->second;
+ const DexFileData* result = info_[profile_index];
+ DCHECK_EQ(profile_key, result->profile_key);
+ DCHECK_EQ(profile_index, result->profile_index);
+ return result;
}
bool ProfileCompilationInfo::AddResolvedClasses(const DexCacheResolvedClasses& classes) {
@@ -415,9 +445,7 @@
uint32_t dex_checksum,
uint16_t method_index,
const OfflineProfileMethodInfo& pmi) {
- DexFileData* const data = GetOrAddDexFileData(
- GetProfileDexFileKey(dex_location),
- dex_checksum);
+ DexFileData* const data = GetOrAddDexFileData(GetProfileDexFileKey(dex_location), dex_checksum);
if (data == nullptr) { // checksum mismatch
return false;
}
@@ -753,6 +781,8 @@
return kProfileLoadSuccess;
}
+// TODO(calin): Fix this API. ProfileCompilationInfo::Load should be static and
+// return a unique pointer to a ProfileCompilationInfo upon success.
bool ProfileCompilationInfo::Load(int fd) {
std::string error;
ProfileLoadSatus status = LoadInternal(fd, &error);
@@ -770,6 +800,10 @@
ScopedTrace trace(__PRETTY_FUNCTION__);
DCHECK_GE(fd, 0);
+ if (!IsEmpty()) {
+ return kProfileLoadWouldOverwiteData;
+ }
+
struct stat stat_buffer;
if (fstat(fd, &stat_buffer) != 0) {
return kProfileLoadIOError;
@@ -820,10 +854,10 @@
// the current profile info.
// Note that the number of elements should be very small, so this should not
// be a performance issue.
- for (const auto& other_it : other.info_) {
- auto info_it = info_.find(other_it.first);
- if ((info_it != info_.end()) && (info_it->second.checksum != other_it.second.checksum)) {
- LOG(WARNING) << "Checksum mismatch for dex " << other_it.first;
+ for (const DexFileData* other_dex_data : other.info_) {
+ const DexFileData* dex_data = FindDexData(other_dex_data->profile_key);
+ if ((dex_data != nullptr) && (dex_data->checksum != other_dex_data->checksum)) {
+ LOG(WARNING) << "Checksum mismatch for dex " << other_dex_data->profile_key;
return false;
}
}
@@ -840,32 +874,28 @@
// First, build a mapping from other_dex_profile_index to this_dex_profile_index.
// This will make sure that the ClassReferences will point to the correct dex file.
SafeMap<uint8_t, uint8_t> dex_profile_index_remap;
- for (const auto& other_it : other.info_) {
- const std::string& other_dex_location = other_it.first;
- uint32_t other_checksum = other_it.second.checksum;
- const DexFileData& other_dex_data = other_it.second;
- const DexFileData* dex_data = GetOrAddDexFileData(other_dex_location, other_checksum);
+ for (const DexFileData* other_dex_data : other.info_) {
+ const DexFileData* dex_data = GetOrAddDexFileData(other_dex_data->profile_key,
+ other_dex_data->checksum);
if (dex_data == nullptr) {
return false; // Could happen if we exceed the number of allowed dex files.
}
- dex_profile_index_remap.Put(other_dex_data.profile_index, dex_data->profile_index);
+ dex_profile_index_remap.Put(other_dex_data->profile_index, dex_data->profile_index);
}
// Merge the actual profile data.
- for (const auto& other_it : other.info_) {
- const std::string& other_dex_location = other_it.first;
- const DexFileData& other_dex_data = other_it.second;
- auto info_it = info_.find(other_dex_location);
- DCHECK(info_it != info_.end());
+ for (const DexFileData* other_dex_data : other.info_) {
+ DexFileData* dex_data = const_cast<DexFileData*>(FindDexData(other_dex_data->profile_key));
+ DCHECK(dex_data != nullptr);
// Merge the classes.
- info_it->second.class_set.insert(other_dex_data.class_set.begin(),
- other_dex_data.class_set.end());
+ dex_data->class_set.insert(other_dex_data->class_set.begin(),
+ other_dex_data->class_set.end());
// Merge the methods and the inline caches.
- for (const auto& other_method_it : other_dex_data.method_map) {
+ for (const auto& other_method_it : other_dex_data->method_map) {
uint16_t other_method_index = other_method_it.first;
- auto method_it = info_it->second.method_map.FindOrAdd(other_method_index);
+ auto method_it = dex_data->method_map.FindOrAdd(other_method_index);
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;
@@ -905,28 +935,18 @@
ProfileCompilationInfo::FindMethod(const std::string& dex_location,
uint32_t dex_checksum,
uint16_t dex_method_index) const {
- auto info_it = info_.find(GetProfileDexFileKey(dex_location));
- if (info_it != info_.end()) {
- if (!ChecksumMatch(dex_checksum, info_it->second.checksum)) {
+ const DexFileData* dex_data = FindDexData(GetProfileDexFileKey(dex_location));
+ if (dex_data != nullptr) {
+ if (!ChecksumMatch(dex_checksum, dex_data->checksum)) {
return nullptr;
}
- const MethodMap& methods = info_it->second.method_map;
+ const MethodMap& methods = dex_data->method_map;
const auto method_it = methods.find(dex_method_index);
return method_it == methods.end() ? nullptr : &(method_it->second);
}
return nullptr;
}
-void ProfileCompilationInfo::DexFileToProfileIndex(
- /*out*/std::vector<DexReference>* dex_references) const {
- dex_references->resize(info_.size());
- for (const auto& info_it : info_) {
- DexReference& dex_ref = (*dex_references)[info_it.second.profile_index];
- dex_ref.dex_location = info_it.first;
- dex_ref.dex_checksum = info_it.second.checksum;
- }
-}
-
bool ProfileCompilationInfo::GetMethod(const std::string& dex_location,
uint32_t dex_checksum,
uint16_t dex_method_index,
@@ -936,7 +956,12 @@
return false;
}
- DexFileToProfileIndex(&pmi->dex_references);
+ pmi->dex_references.resize(info_.size());
+ for (const DexFileData* dex_data : info_) {
+ pmi->dex_references[dex_data->profile_index].dex_location = dex_data->profile_key;
+ pmi->dex_references[dex_data->profile_index].dex_checksum = dex_data->checksum;
+ }
+
// TODO(calin): maybe expose a direct pointer to avoid copying
pmi->inline_caches = *inline_caches;
return true;
@@ -944,12 +969,12 @@
bool ProfileCompilationInfo::ContainsClass(const DexFile& dex_file, dex::TypeIndex type_idx) const {
- auto info_it = info_.find(GetProfileDexFileKey(dex_file.GetLocation()));
- if (info_it != info_.end()) {
- if (!ChecksumMatch(dex_file, info_it->second.checksum)) {
+ const DexFileData* dex_data = FindDexData(GetProfileDexFileKey(dex_file.GetLocation()));
+ if (dex_data != nullptr) {
+ if (!ChecksumMatch(dex_file, dex_data->checksum)) {
return false;
}
- const std::set<dex::TypeIndex>& classes = info_it->second.class_set;
+ const std::set<dex::TypeIndex>& classes = dex_data->class_set;
return classes.find(type_idx) != classes.end();
}
return false;
@@ -957,16 +982,16 @@
uint32_t ProfileCompilationInfo::GetNumberOfMethods() const {
uint32_t total = 0;
- for (const auto& it : info_) {
- total += it.second.method_map.size();
+ for (const DexFileData* dex_data : info_) {
+ total += dex_data->method_map.size();
}
return total;
}
uint32_t ProfileCompilationInfo::GetNumberOfResolvedClasses() const {
uint32_t total = 0;
- for (const auto& it : info_) {
- total += it.second.class_set.size();
+ for (const DexFileData* dex_data : info_) {
+ total += dex_data->class_set.size();
}
return total;
}
@@ -999,35 +1024,27 @@
os << "ProfileInfo:";
const std::string kFirstDexFileKeySubstitute = ":classes.dex";
- // Write the entries in profile index order.
- std::vector<const std::string*> ordered_info_location(info_.size());
- std::vector<const DexFileData*> ordered_info_data(info_.size());
- for (const auto& it : info_) {
- ordered_info_location[it.second.profile_index] = &(it.first);
- ordered_info_data[it.second.profile_index] = &(it.second);
- }
- for (size_t profile_index = 0; profile_index < info_.size(); profile_index++) {
+
+ for (const DexFileData* dex_data : info_) {
os << "\n";
- const std::string& location = *ordered_info_location[profile_index];
- const DexFileData& dex_data = *ordered_info_data[profile_index];
if (print_full_dex_location) {
- os << location;
+ os << dex_data->profile_key;
} else {
// Replace the (empty) multidex suffix of the first key with a substitute for easier reading.
- std::string multidex_suffix = DexFile::GetMultiDexSuffix(location);
+ std::string multidex_suffix = DexFile::GetMultiDexSuffix(dex_data->profile_key);
os << (multidex_suffix.empty() ? kFirstDexFileKeySubstitute : multidex_suffix);
}
- os << " [index=" << static_cast<uint32_t>(dex_data.profile_index) << "]";
+ os << " [index=" << static_cast<uint32_t>(dex_data->profile_index) << "]";
const DexFile* dex_file = nullptr;
if (dex_files != nullptr) {
for (size_t i = 0; i < dex_files->size(); i++) {
- if (location == (*dex_files)[i]->GetLocation()) {
+ if (dex_data->profile_key == (*dex_files)[i]->GetLocation()) {
dex_file = (*dex_files)[i];
}
}
}
os << "\n\tmethods: ";
- for (const auto& method_it : dex_data.method_map) {
+ for (const auto& method_it : dex_data->method_map) {
if (dex_file != nullptr) {
os << "\n\t\t" << dex_file->PrettyMethod(method_it.first, true);
} else {
@@ -1052,7 +1069,7 @@
os << "], ";
}
os << "\n\tclasses: ";
- for (const auto class_it : dex_data.class_set) {
+ for (const auto class_it : dex_data->class_set) {
if (dex_file != nullptr) {
os << "\n\t\t" << dex_file->PrettyType(class_it);
} else {
@@ -1076,19 +1093,17 @@
if (info_.empty()) {
return;
}
- for (const auto& it : info_) {
- const std::string& location = it.first;
- const DexFileData& dex_data = it.second;
+ for (const DexFileData* dex_data : info_) {
const DexFile* dex_file = nullptr;
if (dex_files != nullptr) {
for (size_t i = 0; i < dex_files->size(); i++) {
- if (location == GetProfileDexFileKey((*dex_files)[i]->GetLocation()) &&
- dex_data.checksum == (*dex_files)[i]->GetLocationChecksum()) {
+ if (dex_data->profile_key == GetProfileDexFileKey((*dex_files)[i]->GetLocation()) &&
+ dex_data->checksum == (*dex_files)[i]->GetLocationChecksum()) {
dex_file = (*dex_files)[i];
}
}
}
- for (const auto class_it : dex_data.class_set) {
+ for (const auto class_it : dex_data->class_set) {
if (dex_file != nullptr) {
class_names->insert(std::string(dex_file->PrettyType(class_it)));
}
@@ -1097,7 +1112,19 @@
}
bool ProfileCompilationInfo::Equals(const ProfileCompilationInfo& other) {
- return info_.Equals(other.info_);
+ // No need to compare profile_key_map_. That's only a cache for fast search.
+ // All the information is already in the info_ vector.
+ if (info_.size() != other.info_.size()) {
+ return false;
+ }
+ for (size_t i = 0; i < info_.size(); i++) {
+ const DexFileData& dex_data = *info_[i];
+ const DexFileData& other_dex_data = *other.info_[i];
+ if (!(dex_data == other_dex_data)) {
+ return false;
+ }
+ }
+ return true;
}
std::set<DexCacheResolvedClasses> ProfileCompilationInfo::GetResolvedClasses(
@@ -1107,13 +1134,11 @@
key_to_location_map.emplace(GetProfileDexFileKey(location), location);
}
std::set<DexCacheResolvedClasses> ret;
- for (auto&& pair : info_) {
- const std::string& profile_key = pair.first;
- auto it = key_to_location_map.find(profile_key);
+ for (const DexFileData* dex_data : info_) {
+ const auto& it = key_to_location_map.find(dex_data->profile_key);
if (it != key_to_location_map.end()) {
- const DexFileData& data = pair.second;
- DexCacheResolvedClasses classes(it->second, it->second, data.checksum);
- classes.AddClasses(data.class_set.begin(), data.class_set.end());
+ DexCacheResolvedClasses classes(it->second, it->second, dex_data->checksum);
+ classes.AddClasses(dex_data->class_set.begin(), dex_data->class_set.end());
ret.insert(classes);
}
}
@@ -1121,8 +1146,8 @@
}
void ProfileCompilationInfo::ClearResolvedClasses() {
- for (auto& pair : info_) {
- pair.second.class_set.clear();
+ for (DexFileData* dex_data : info_) {
+ dex_data->class_set.clear();
}
}
@@ -1237,4 +1262,17 @@
return true;
}
+void ProfileCompilationInfo::ClearProfile() {
+ for (DexFileData* dex_data : info_) {
+ delete dex_data;
+ }
+ info_.clear();
+ profile_key_map_.clear();
+}
+
+bool ProfileCompilationInfo::IsEmpty() const {
+ DCHECK_EQ(info_.empty(), profile_key_map_.empty());
+ return info_.empty();
+}
+
} // namespace art
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index c3ac78f..87f7636 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -181,11 +181,16 @@
// Public methods to create, extend or query the profile.
+ ProfileCompilationInfo() {}
+ ProfileCompilationInfo(const ProfileCompilationInfo& pci);
+ ~ProfileCompilationInfo();
+
// Add the given methods and classes to the current profile object.
bool AddMethodsAndClasses(const std::vector<ProfileMethodInfo>& methods,
const std::set<DexCacheResolvedClasses>& resolved_classes);
// Load profile information from the given file descriptor.
+ // If the current profile is non-empty the load will fail.
bool Load(int fd);
// Merge the data from another ProfileCompilationInfo into the current object.
@@ -268,6 +273,7 @@
private:
enum ProfileLoadSatus {
+ kProfileLoadWouldOverwiteData,
kProfileLoadIOError,
kProfileLoadVersionMismatch,
kProfileLoadBadData,
@@ -278,14 +284,21 @@
using MethodMap = SafeMap<uint16_t, InlineCacheMap>;
// Internal representation of the profile information belonging to a dex file.
+ // Note that we could do without profile_key (the key used to encode the dex
+ // file in the profile) and profile_index (the index of the dex file in the
+ // profile) fields in this struct because we can infer them from
+ // profile_key_map_ and info_. However, it makes the profiles logic much
+ // simpler if we have references here as well.
struct DexFileData {
- DexFileData(uint32_t location_checksum, uint16_t index)
- : profile_index(index), checksum(location_checksum) {}
- // The profile index of this dex file (matches ClassReference#dex_profile_index)
+ DexFileData(const std::string& key, uint32_t location_checksum, uint16_t index)
+ : profile_key(key), profile_index(index), checksum(location_checksum) {}
+ // The profile key this data belongs to.
+ std::string profile_key;
+ // The profile index of this dex file (matches ClassReference#dex_profile_index).
uint8_t profile_index;
- // The dex checksum
+ // The dex checksum.
uint32_t checksum;
- // The methonds' profile information
+ // The methonds' profile information.
MethodMap method_map;
// The classes which have been profiled. Note that these don't necessarily include
// all the classes that can be found in the inline caches reference.
@@ -296,12 +309,9 @@
}
};
- // Maps dex file to their profile information.
- using DexFileToProfileInfoMap = SafeMap<const std::string, DexFileData>;
-
- // Return the profile data for the given dex location or null if the dex location
+ // Return the profile data for the given profile key or null if the dex location
// already exists but has a different checksum
- DexFileData* GetOrAddDexFileData(const std::string& dex_location, uint32_t checksum);
+ DexFileData* GetOrAddDexFileData(const std::string& profile_key, uint32_t checksum);
// Add a method index to the profile (without inline caches).
bool AddMethodIndex(const std::string& dex_location, uint32_t checksum, uint16_t method_idx);
@@ -332,6 +342,16 @@
// be the same as the profile index of the dex file (used to encode the ClassReferences).
void DexFileToProfileIndex(/*out*/std::vector<DexReference>* dex_references) const;
+ // Return the dex data associated with the given profile key or null if the profile
+ // doesn't contain the key.
+ const DexFileData* FindDexData(const std::string& profile_key) const;
+
+ // Clear all the profile data.
+ void ClearProfile();
+
+ // Checks if the profile is empty.
+ bool IsEmpty() const;
+
// Parsing functionality.
// The information present in the header of each profile line.
@@ -438,7 +458,15 @@
friend class ProfileAssistantTest;
friend class Dex2oatLayoutTest;
- DexFileToProfileInfoMap info_;
+ // Vector containing the actual profile info.
+ // The vector index is the profile index of the dex data and
+ // matched DexFileData::profile_index.
+ std::vector<DexFileData*> info_;
+
+ // Cache mapping profile keys to profile index.
+ // This is used to speed up searches since it avoids iterating
+ // over the info_ vector when searching by profile key.
+ SafeMap<const std::string, uint8_t> profile_key_map_;
};
} // namespace art
diff --git a/runtime/jit/profile_compilation_info_test.cc b/runtime/jit/profile_compilation_info_test.cc
index 5cd8e8f..c9f2d0e 100644
--- a/runtime/jit/profile_compilation_info_test.cc
+++ b/runtime/jit/profile_compilation_info_test.cc
@@ -195,7 +195,7 @@
dex_pc_data.AddClass(1, dex::TypeIndex(1));
dex_pc_data.AddClass(2, dex::TypeIndex(2));
- pmi.inline_caches.Put(dex_pc, dex_pc_data);
+ pmi.inline_caches.Put(dex_pc, dex_pc_data);
}
// Megamorphic
for (uint16_t dex_pc = 22; dex_pc < 33; dex_pc++) {
@@ -787,4 +787,26 @@
ASSERT_TRUE(info_no_inline_cache.Save(GetFd(profile)));
}
+TEST_F(ProfileCompilationInfoTest, LoadShouldClearExistingDataFromProfiles) {
+ ScratchFile profile;
+
+ ProfileCompilationInfo saved_info;
+ // Save a few methods.
+ for (uint16_t i = 0; i < 10; i++) {
+ ASSERT_TRUE(AddMethod("dex_location1", /* checksum */ 1, /* method_idx */ i, &saved_info));
+ }
+ ASSERT_TRUE(saved_info.Save(GetFd(profile)));
+ ASSERT_EQ(0, profile.GetFile()->Flush());
+ ASSERT_TRUE(profile.GetFile()->ResetOffset());
+
+ // Add a bunch of methods to test_info.
+ ProfileCompilationInfo test_info;
+ for (uint16_t i = 0; i < 10; i++) {
+ ASSERT_TRUE(AddMethod("dex_location2", /* checksum */ 2, /* method_idx */ i, &test_info));
+ }
+
+ // Attempt to load the saved profile into test_info.
+ // This should fail since the test_info already contains data and the load would overwrite it.
+ ASSERT_FALSE(test_info.Load(GetFd(profile)));
+}
} // namespace art