From: Calin Juravle Date: Thu, 24 Mar 2016 20:33:22 +0000 (+0000) Subject: Discard corrupted or out of date profiles X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=5d1bd0aecdbd4093028c83c4c23f0340ac835e85;p=android-x86%2Fart.git Discard corrupted or out of date profiles Until now we bailed out if the previous profile contained invalid data. This CLs forces the save and clears any data in a profile that has the wrong version or contains bad data. Bug: 27081617 (cherry picked from commit fe297a96bc6d3da11579709add9b4568730d2b4f) Change-Id: I9184e0483ea0a869d7aa92630acd6fa04a9d2e03 --- diff --git a/runtime/jit/offline_profiling_info.cc b/runtime/jit/offline_profiling_info.cc index fa71905be..024c73a2c 100644 --- a/runtime/jit/offline_profiling_info.cc +++ b/runtime/jit/offline_profiling_info.cc @@ -75,7 +75,9 @@ bool ProfileCompilationInfo::AddMethodsAndClasses( return true; } -bool ProfileCompilationInfo::MergeAndSave(const std::string& filename, uint64_t* bytes_written) { +bool ProfileCompilationInfo::MergeAndSave(const std::string& filename, + uint64_t* bytes_written, + bool force) { ScopedTrace trace(__PRETTY_FUNCTION__); ScopedFlock flock; std::string error; @@ -88,26 +90,35 @@ bool ProfileCompilationInfo::MergeAndSave(const std::string& filename, uint64_t* // Load the file but keep a copy around to be able to infer if the content has changed. ProfileCompilationInfo fileInfo; - if (!fileInfo.Load(fd)) { - LOG(WARNING) << "Could not load previous profile data from file " << filename; - return false; - } - - // Merge the content of file into the current object. - if (!MergeWith(fileInfo)) { - LOG(WARNING) << "Could not merge previous profile data from file " << filename; - } - - // If after the merge we have the same data as what is the file there's no point - // in actually doing the write. The file will be exactly the same as before. - if (Equals(fileInfo)) { - if (bytes_written != nullptr) { - *bytes_written = 0; + ProfileLoadSatus status = fileInfo.LoadInternal(fd, &error); + if (status == kProfileLoadSuccess) { + // Merge the content of file into the current object. + if (MergeWith(fileInfo)) { + // If after the merge we have the same data as what is the file there's no point + // in actually doing the write. The file will be exactly the same as before. + if (Equals(fileInfo)) { + if (bytes_written != nullptr) { + *bytes_written = 0; + } + return true; + } + } else { + LOG(WARNING) << "Could not merge previous profile data from file " << filename; + if (!force) { + return false; + } } - return true; + } else if (force && + ((status == kProfileLoadVersionMismatch) || (status == kProfileLoadBadData))) { + // Log a warning but don't return false. We will clear the profile anyway. + LOG(WARNING) << "Clearing bad or obsolete profile data from file " + << filename << ": " << error; + } else { + LOG(WARNING) << "Could not load profile data from file " << filename << ": " << error; + return false; } - // We need to clear the data because we don't support append to the profiles yet. + // We need to clear the data because we don't support appending to the profiles yet. if (!flock.GetFile()->ClearContent()) { PLOG(WARNING) << "Could not clear profile file: " << filename; return false; @@ -128,31 +139,6 @@ bool ProfileCompilationInfo::MergeAndSave(const std::string& filename, uint64_t* return result; } -bool ProfileCompilationInfo::SaveProfilingInfo( - const std::string& filename, - const std::vector& methods, - const std::set& resolved_classes, - uint64_t* bytes_written) { - if (methods.empty() && resolved_classes.empty()) { - VLOG(profiler) << "No info to save to " << filename; - if (bytes_written != nullptr) { - *bytes_written = 0; - } - return true; - } - - ProfileCompilationInfo info; - if (!info.AddMethodsAndClasses(methods, resolved_classes)) { - LOG(WARNING) << "Checksum mismatch when processing methods and resolved classes for " - << filename; - if (bytes_written != nullptr) { - *bytes_written = 0; - } - return false; - } - return info.MergeAndSave(filename, bytes_written); -} - // Returns true if all the bytes were successfully written to the file descriptor. static bool WriteBuffer(int fd, const uint8_t* buffer, size_t byte_count) { while (byte_count > 0) { @@ -535,18 +521,25 @@ ProfileCompilationInfo::ProfileLoadSatus ProfileCompilationInfo::LoadInternal( } bool ProfileCompilationInfo::MergeWith(const ProfileCompilationInfo& other) { + // 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 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; + return false; + } + } + // All checksums match. Import the 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); if (info_it == info_.end()) { info_it = info_.Put(other_dex_location, DexFileData(other_dex_data.checksum)); } - if (info_it->second.checksum != other_dex_data.checksum) { - LOG(WARNING) << "Checksum mismatch for dex " << other_dex_location; - return false; - } info_it->second.method_set.insert(other_dex_data.method_set.begin(), other_dex_data.method_set.end()); info_it->second.class_set.insert(other_dex_data.class_set.begin(), diff --git a/runtime/jit/offline_profiling_info.h b/runtime/jit/offline_profiling_info.h index 492f9f881..c4055b386 100644 --- a/runtime/jit/offline_profiling_info.h +++ b/runtime/jit/offline_profiling_info.h @@ -44,14 +44,6 @@ class ProfileCompilationInfo { static const uint8_t kProfileMagic[]; static const uint8_t kProfileVersion[]; - // Saves profile information about the given methods in the given file. - // Note that the saving proceeds only if the file can be locked for exclusive access. - // If not (the locking is not blocking), the function does not save and returns false. - static bool SaveProfilingInfo(const std::string& filename, - const std::vector& methods, - const std::set& resolved_classes, - uint64_t* bytes_written = nullptr); - // Add the given methods and classes to the current profile object. bool AddMethodsAndClasses(const std::vector& methods, const std::set& resolved_classes); @@ -63,7 +55,10 @@ class ProfileCompilationInfo { bool Save(int fd); // Loads and merges profile information from the given file into the current // object and tries to save it back to disk. - bool MergeAndSave(const std::string& filename, uint64_t* bytes_written); + // If `force` is true then the save will go through even if the given file + // has bad data or its version does not match. In this cases the profile content + // is ignored. + bool MergeAndSave(const std::string& filename, uint64_t* bytes_written, bool force); // Returns the number of methods that were profiled. uint32_t GetNumberOfMethods() const; diff --git a/runtime/jit/profile_compilation_info_test.cc b/runtime/jit/profile_compilation_info_test.cc index db681f4cb..ad9af7017 100644 --- a/runtime/jit/profile_compilation_info_test.cc +++ b/runtime/jit/profile_compilation_info_test.cc @@ -67,6 +67,17 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest { return static_cast(file.GetFd()); } + bool SaveProfilingInfo( + const std::string& filename, + const std::vector& methods, + const std::set& resolved_classes) { + ProfileCompilationInfo info; + if (!info.AddMethodsAndClasses(methods, resolved_classes)) { + return false; + } + return info.MergeAndSave(filename, nullptr, false); + } + // Cannot sizeof the actual arrays so hardcode the values here. // They should not change anyway. static constexpr int kProfileMagicSize = 4; @@ -87,9 +98,7 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethods) { // Save virtual methods from Main. std::set resolved_classes; std::vector main_methods = GetVirtualMethods(class_loader, "LMain;"); - ASSERT_TRUE(ProfileCompilationInfo::SaveProfilingInfo(profile.GetFilename(), - main_methods, - resolved_classes)); + ASSERT_TRUE(SaveProfilingInfo(profile.GetFilename(), main_methods, resolved_classes)); // Check that what we saved is in the profile. ProfileCompilationInfo info1; @@ -104,9 +113,7 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethods) { // Save virtual methods from Second. std::vector second_methods = GetVirtualMethods(class_loader, "LSecond;"); - ASSERT_TRUE(ProfileCompilationInfo::SaveProfilingInfo(profile.GetFilename(), - second_methods, - resolved_classes)); + ASSERT_TRUE(SaveProfilingInfo(profile.GetFilename(), second_methods, resolved_classes)); // Check that what we saved is in the profile (methods form Main and Second). ProfileCompilationInfo info2; diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc index 81d81a5c7..df42a80cc 100644 --- a/runtime/jit/profile_saver.cc +++ b/runtime/jit/profile_saver.cc @@ -211,7 +211,9 @@ bool ProfileSaver::ProcessProfilingInfo() { continue; } uint64_t bytes_written; - if (cached_info->MergeAndSave(filename, &bytes_written)) { + // Force the save. In case the profile data is corrupted or the the profile + // has the wrong version this will "fix" the file to the correct format. + if (cached_info->MergeAndSave(filename, &bytes_written, /*force*/ true)) { last_save_number_of_methods_ = cached_info->GetNumberOfMethods(); last_save_number_of_classes__ = cached_info->GetNumberOfResolvedClasses(); // Clear resolved classes. No need to store them around as