Merge "Merge profiles without needing to creating profile_compilation_info object"
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index c9bfc9c..9e05117 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -28,6 +28,7 @@
#include <cstdlib>
#include <string>
#include <vector>
+#include <iostream>
#include "android-base/file.h"
@@ -47,8 +48,10 @@
namespace art {
const uint8_t ProfileCompilationInfo::kProfileMagic[] = { 'p', 'r', 'o', '\0' };
-// Last profile version: update the multidex separator.
-const uint8_t ProfileCompilationInfo::kProfileVersion[] = { '0', '0', '9', '\0' };
+// Last profile version: merge profiles directly from the file without creating
+// profile_compilation_info object. All the profile line headers are now placed together
+// before corresponding method_encodings and class_ids.
+const uint8_t ProfileCompilationInfo::kProfileVersion[] = { '0', '1', '0', '\0' };
static constexpr uint16_t kMaxDexFileKeyLength = PATH_MAX;
@@ -177,9 +180,36 @@
return true;
}
+bool ProfileCompilationInfo::MergeWith(const std::string& filename) {
+ std::string error;
+ int flags = O_RDONLY | O_NOFOLLOW | O_CLOEXEC;
+ ScopedFlock profile_file = LockedFile::Open(filename.c_str(), flags,
+ /*block*/false, &error);
+
+ if (profile_file.get() == nullptr) {
+ LOG(WARNING) << "Couldn't lock the profile file " << filename << ": " << error;
+ return false;
+ }
+
+ int fd = profile_file->Fd();
+
+ ProfileLoadSatus status = LoadInternal(fd, &error);
+ if (status == kProfileLoadSuccess) {
+ return true;
+ }
+
+ LOG(WARNING) << "Could not load profile data from file " << filename << ": " << error;
+ return false;
+}
+
bool ProfileCompilationInfo::Load(const std::string& filename, bool clear_if_invalid) {
ScopedTrace trace(__PRETTY_FUNCTION__);
std::string error;
+
+ if (!IsEmpty()) {
+ return kProfileLoadWouldOverwiteData;
+ }
+
int flags = O_RDWR | O_NOFOLLOW | O_CLOEXEC;
// There's no need to fsync profile data right away. We get many chances
// to write it again in case something goes wrong. We can rely on a simple
@@ -288,15 +318,14 @@
/**
* Serialization format:
- * magic,version,number_of_dex_files,uncompressed_size_of_zipped_data,compressed_data_size,
- * zipped[dex_location1,number_of_classes1,methods_region_size,dex_location_checksum1
- * num_method_ids,
- * method_encoding_11,method_encoding_12...,class_id1,class_id2...
- * startup/post startup bitmap,
- * dex_location2,number_of_classes2,methods_region_size,dex_location_checksum2, num_method_ids,
- * method_encoding_21,method_encoding_22...,,class_id1,class_id2...
- * startup/post startup bitmap,
- * .....]
+ * [profile_header, zipped[[profile_line_header1, profile_line_header2...],[profile_line_data1,
+ * profile_line_data2...]]]
+ * profile_header:
+ * magic,version,number_of_dex_files,uncompressed_size_of_zipped_data,compressed_data_size
+ * profile_line_header:
+ * dex_location,number_of_classes,methods_region_size,dex_location_checksum,num_method_ids
+ * profile_line_data:
+ * method_encoding_1,method_encoding_2...,class_id1,class_id2...,startup/post startup bitmap
* The method_encoding is:
* method_id,number_of_inline_caches,inline_cache1,inline_cache2...
* The inline_cache is:
@@ -358,12 +387,10 @@
// 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.
+ // Write profile line headers.
for (const DexFileData* dex_data_ptr : info_) {
const DexFileData& dex_data = *dex_data_ptr;
- // Note that we allow dex files without any methods or classes, so that
- // inline caches can refer valid dex files.
-
if (dex_data.profile_key.size() >= kMaxDexFileKeyLength) {
LOG(WARNING) << "DexFileKey exceeds allocated limit";
return false;
@@ -381,6 +408,13 @@
AddUintToBuffer(&buffer, dex_data.num_method_ids); // uint32_t
AddStringToBuffer(&buffer, dex_data.profile_key);
+ }
+
+ for (const DexFileData* dex_data_ptr : info_) {
+ const DexFileData& dex_data = *dex_data_ptr;
+
+ // Note that we allow dex files without any methods or classes, so that
+ // inline caches can refer valid dex files.
uint16_t last_method_index = 0;
for (const auto& method_it : dex_data.method_map) {
@@ -690,10 +724,12 @@
} \
while (false)
-bool ProfileCompilationInfo::ReadInlineCache(SafeBuffer& buffer,
- uint8_t number_of_dex_files,
- /*out*/ InlineCacheMap* inline_cache,
- /*out*/ std::string* error) {
+bool ProfileCompilationInfo::ReadInlineCache(
+ SafeBuffer& buffer,
+ uint8_t number_of_dex_files,
+ const SafeMap<uint8_t, uint8_t>& dex_profile_index_remap,
+ /*out*/ InlineCacheMap* inline_cache,
+ /*out*/ std::string* error) {
uint16_t inline_cache_size;
READ_UINT(uint16_t, buffer, inline_cache_size, error);
for (; inline_cache_size > 0; inline_cache_size--) {
@@ -723,7 +759,8 @@
for (; dex_classes_size > 0; dex_classes_size--) {
uint16_t type_index;
READ_UINT(uint16_t, buffer, type_index, error);
- dex_pc_data->AddClass(dex_profile_index, dex::TypeIndex(type_index));
+ dex_pc_data->AddClass(dex_profile_index_remap.Get(dex_profile_index),
+ dex::TypeIndex(type_index));
}
}
}
@@ -733,6 +770,7 @@
bool ProfileCompilationInfo::ReadMethods(SafeBuffer& buffer,
uint8_t number_of_dex_files,
const ProfileLineHeader& line_header,
+ const SafeMap<uint8_t, uint8_t>& dex_profile_index_remap,
/*out*/std::string* error) {
uint32_t unread_bytes_before_operation = buffer.CountUnreadBytes();
if (unread_bytes_before_operation < line_header.method_region_size_bytes) {
@@ -751,7 +789,11 @@
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 (!ReadInlineCache(buffer, number_of_dex_files, inline_cache, error)) {
+ if (!ReadInlineCache(buffer,
+ number_of_dex_files,
+ dex_profile_index_remap,
+ inline_cache,
+ error)) {
return false;
}
}
@@ -954,6 +996,8 @@
SafeBuffer& buffer,
uint8_t number_of_dex_files,
const ProfileLineHeader& line_header,
+ const SafeMap<uint8_t, uint8_t>& dex_profile_index_remap,
+ bool merge_classes,
/*out*/std::string* error) {
DexFileData* data = GetOrAddDexFileData(line_header.dex_location,
line_header.checksum,
@@ -964,12 +1008,14 @@
return kProfileLoadBadData;
}
- if (!ReadMethods(buffer, number_of_dex_files, line_header, error)) {
+ if (!ReadMethods(buffer, number_of_dex_files, line_header, dex_profile_index_remap, error)) {
return kProfileLoadBadData;
}
- if (!ReadClasses(buffer, line_header, error)) {
- return kProfileLoadBadData;
+ if (merge_classes) {
+ if (!ReadClasses(buffer, line_header, error)) {
+ return kProfileLoadBadData;
+ }
}
const size_t bytes = data->bitmap_storage.size();
@@ -986,9 +1032,10 @@
// 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) {
+bool ProfileCompilationInfo::Load(int fd, bool merge_classes) {
std::string error;
- ProfileLoadSatus status = LoadInternal(fd, &error);
+
+ ProfileLoadSatus status = LoadInternal(fd, &error, merge_classes);
if (status == kProfileLoadSuccess) {
return true;
@@ -1000,14 +1047,10 @@
// TODO(calin): fail fast if the dex checksums don't match.
ProfileCompilationInfo::ProfileLoadSatus ProfileCompilationInfo::LoadInternal(
- int fd, std::string* error) {
+ int fd, std::string* error, bool merge_classes) {
ScopedTrace trace(__PRETTY_FUNCTION__);
DCHECK_GE(fd, 0);
- if (!IsEmpty()) {
- return kProfileLoadWouldOverwiteData;
- }
-
struct stat stat_buffer;
if (fstat(fd, &stat_buffer) != 0) {
return kProfileLoadIOError;
@@ -1071,6 +1114,8 @@
return kProfileLoadBadData;
}
+ std::vector<ProfileLineHeader> profile_line_headers;
+ // Read profile line headers.
for (uint8_t k = 0; k < number_of_dex_files; k++) {
ProfileLineHeader line_header;
@@ -1079,9 +1124,22 @@
if (status != kProfileLoadSuccess) {
return status;
}
+ profile_line_headers.push_back(line_header);
+ }
+ SafeMap<uint8_t, uint8_t> dex_profile_index_remap;
+ if (!RemapProfileIndex(profile_line_headers, &dex_profile_index_remap)) {
+ return kProfileLoadBadData;
+ }
+
+ for (uint8_t k = 0; k < number_of_dex_files; k++) {
// Now read the actual profile line.
- status = ReadProfileLine(uncompressed_data, number_of_dex_files, line_header, error);
+ status = ReadProfileLine(uncompressed_data,
+ number_of_dex_files,
+ profile_line_headers[k],
+ dex_profile_index_remap,
+ merge_classes,
+ error);
if (status != kProfileLoadSuccess) {
return status;
}
@@ -1096,6 +1154,37 @@
}
}
+bool ProfileCompilationInfo::RemapProfileIndex(
+ const std::vector<ProfileLineHeader>& profile_line_headers,
+ /*out*/SafeMap<uint8_t, uint8_t>* dex_profile_index_remap) {
+ // First verify that all checksums match. This will avoid adding garbage to
+ // the current profile info.
+ // Note that the number of elements should be very small, so this should not
+ // be a performance issue.
+ for (const ProfileLineHeader other_profile_line_header : profile_line_headers) {
+ // verify_checksum is false because we want to differentiate between a missing dex data and
+ // a mismatched checksum.
+ const DexFileData* dex_data = FindDexData(other_profile_line_header.dex_location,
+ 0u,
+ false /* verify_checksum */);
+ if ((dex_data != nullptr) && (dex_data->checksum != other_profile_line_header.checksum)) {
+ LOG(WARNING) << "Checksum mismatch for dex " << other_profile_line_header.dex_location;
+ return false;
+ }
+ }
+ // All checksums match. Import the data.
+ uint32_t num_dex_files = static_cast<uint32_t>(profile_line_headers.size());
+ for (uint32_t i = 0; i < num_dex_files; i++) {
+ const DexFileData* dex_data = GetOrAddDexFileData(profile_line_headers[i].dex_location,
+ profile_line_headers[i].checksum,
+ profile_line_headers[i].num_method_ids);
+ if (dex_data == nullptr) {
+ return false; // Could happen if we exceed the number of allowed dex files.
+ }
+ dex_profile_index_remap->Put(i, dex_data->profile_index);
+ }
+ return true;
+}
std::unique_ptr<uint8_t[]> ProfileCompilationInfo::DeflateBuffer(const uint8_t* in_buffer,
uint32_t in_size,
uint32_t* compressed_data_size) {
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index ffb67ae..d38237d 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -288,9 +288,10 @@
// Add hotness flags for a simple method.
bool AddMethodHotness(const MethodReference& method_ref, const MethodHotness& hotness);
- // Load profile information from the given file descriptor.
+ // Load or Merge profile information from the given file descriptor.
// If the current profile is non-empty the load will fail.
- bool Load(int fd);
+ // If merge_classes is set to false, classes will not be merged/loaded.
+ bool Load(int fd, bool merge_classes = true);
// Load profile information from the given file
// If the current profile is non-empty the load will fail.
@@ -303,6 +304,9 @@
// we don't want all of the classes to be image classes.
bool MergeWith(const ProfileCompilationInfo& info, bool merge_classes = true);
+ // Merge profile information from the given file descriptor.
+ bool MergeWith(const std::string& filename);
+
// Save the profile data to the given file descriptor.
bool Save(int fd);
@@ -594,7 +598,7 @@
};
// Entry point for profile loding functionality.
- ProfileLoadSatus LoadInternal(int fd, std::string* error);
+ ProfileLoadSatus LoadInternal(int fd, std::string* error, bool merge_classes = true);
// Read the profile header from the given fd and store the number of profile
// lines into number_of_dex_files.
@@ -619,6 +623,8 @@
ProfileLoadSatus ReadProfileLine(SafeBuffer& buffer,
uint8_t number_of_dex_files,
const ProfileLineHeader& line_header,
+ const SafeMap<uint8_t, uint8_t>& dex_profile_index_remap,
+ bool merge_classes,
/*out*/std::string* error);
// Read all the classes from the buffer into the profile `info_` structure.
@@ -630,11 +636,18 @@
bool ReadMethods(SafeBuffer& buffer,
uint8_t number_of_dex_files,
const ProfileLineHeader& line_header,
+ const SafeMap<uint8_t, uint8_t>& dex_profile_index_remap,
/*out*/std::string* error);
+ // The method generates mapping of profile indices while merging a new profile
+ // data into current data. It returns true, if the mapping was successful.
+ bool RemapProfileIndex(const std::vector<ProfileLineHeader>& profile_line_headers,
+ /*out*/SafeMap<uint8_t, uint8_t>* dex_profile_index_remap);
+
// Read the inline cache encoding from line_bufer into inline_cache.
bool ReadInlineCache(SafeBuffer& buffer,
uint8_t number_of_dex_files,
+ const SafeMap<uint8_t, uint8_t>& dex_profile_index_remap,
/*out*/InlineCacheMap* inline_cache,
/*out*/std::string* error);
diff --git a/runtime/jit/profile_compilation_info_test.cc b/runtime/jit/profile_compilation_info_test.cc
index 6010bce..40d303f 100644
--- a/runtime/jit/profile_compilation_info_test.cc
+++ b/runtime/jit/profile_compilation_info_test.cc
@@ -387,6 +387,23 @@
ASSERT_FALSE(info1.MergeWith(info2));
}
+
+TEST_F(ProfileCompilationInfoTest, MergeFdFail) {
+ ScratchFile profile;
+
+ ProfileCompilationInfo info1;
+ ASSERT_TRUE(AddMethod("dex_location", /* checksum */ 1, /* method_idx */ 1, &info1));
+ // Use the same file, change the checksum.
+ ProfileCompilationInfo info2;
+ ASSERT_TRUE(AddMethod("dex_location", /* checksum */ 2, /* method_idx */ 2, &info2));
+
+ ASSERT_TRUE(info1.Save(profile.GetFd()));
+ ASSERT_EQ(0, profile.GetFile()->Flush());
+ ASSERT_TRUE(profile.GetFile()->ResetOffset());
+
+ ASSERT_FALSE(info2.Load(profile.GetFd()));
+}
+
TEST_F(ProfileCompilationInfoTest, SaveMaxMethods) {
ScratchFile profile;
@@ -833,29 +850,6 @@
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)));
-}
-
TEST_F(ProfileCompilationInfoTest, SampledMethodsTest) {
ProfileCompilationInfo test_info;
static constexpr size_t kNumMethods = 1000;