Add O_CREAT to ProfileCompilationInfo::SaveFallback.
Before this change, ProfileCompilationInfo::SaveFallback relies on
ProfileSaver::ProcessProfilingInfo to create an empty file beforehand if
the file didn't exist. This doesn't work if SaveFallback is called by
other callers (e.g., CopyAndUpdateProfileKey), and there can be a race.
After this change, ProcessProfilingInfo no longer creates an empty file,
and SaveFallback creates the file itself.
This change also avoids unnecessary file creation when saving is not
needed.
Bug: 275378665
Bug: 282191456
Test: art/test.py -b --host -r -t 595-profile-saving
Test: art/test.py --target -r -t 595-profile-saving (chroot on U device)
Test: art/test.py --target -r -t 595-profile-saving (chroot on T device)
Test: profman --copy-and-update-profile-key --profile-file=YouTube.dm --output-profile-type=app --apk=YouTube.apk --dex-location=base.apk --reference-profile-file=/tmp/1.prof
Change-Id: I01ef27791c2625f0cbd242fcaa8056df4b7d24d0
diff --git a/libprofile/profile/profile_compilation_info.cc b/libprofile/profile/profile_compilation_info.cc
index 189a3d2..30b0a1c 100644
--- a/libprofile/profile/profile_compilation_info.cc
+++ b/libprofile/profile/profile_compilation_info.cc
@@ -16,6 +16,7 @@
#include "profile_compilation_info.h"
+#include <fcntl.h>
#include <sys/file.h>
#include <sys/stat.h>
#include <sys/types.h>
@@ -783,6 +784,9 @@
LockedFile::Open(filename.c_str(), flags, /*block=*/false, &error);
if (profile_file.get() == nullptr) {
+ if (clear_if_invalid && errno == ENOENT) {
+ return true;
+ }
LOG(WARNING) << "Couldn't lock the profile file " << filename << ": " << error;
return false;
}
@@ -800,6 +804,8 @@
(status == ProfileLoadStatus::kBadData))) {
LOG(WARNING) << "Clearing bad or obsolete profile data from file "
<< filename << ": " << error;
+ // When ART Service is enabled, this is the only place where we mutate a profile in place.
+ // TODO(jiakaiz): Get rid of this.
if (profile_file->ClearContent()) {
return true;
} else {
@@ -878,9 +884,9 @@
bool ProfileCompilationInfo::SaveFallback(const std::string& filename, uint64_t* bytes_written) {
std::string error;
#ifdef _WIN32
- int flags = O_WRONLY;
+ int flags = O_WRONLY | O_CREAT;
#else
- int flags = O_WRONLY | O_NOFOLLOW | O_CLOEXEC;
+ int flags = O_WRONLY | O_NOFOLLOW | O_CLOEXEC | O_CREAT;
#endif
// 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
diff --git a/libprofile/profile/profile_compilation_info.h b/libprofile/profile/profile_compilation_info.h
index a6fd32a..57e80a5 100644
--- a/libprofile/profile/profile_compilation_info.h
+++ b/libprofile/profile/profile_compilation_info.h
@@ -463,10 +463,12 @@
// the dex_file they are in.
bool VerifyProfileData(const std::vector<const DexFile*>& dex_files);
- // Load profile information from the given file
+ // Loads profile information from the given file.
+ // Returns true on success, false otherwise.
// If the current profile is non-empty the load will fail.
- // If clear_if_invalid is true and the file is invalid the method clears the
- // the file and returns true.
+ // If clear_if_invalid is true:
+ // - If the file is invalid, the method clears the file and returns true.
+ // - If the file doesn't exist, the method returns true.
bool Load(const std::string& filename, bool clear_if_invalid);
// Merge the data from another ProfileCompilationInfo into the current object. Only merges
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index cdc4bdb..175b0f5 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -868,22 +868,15 @@
}
{
ProfileCompilationInfo info(Runtime::Current()->GetArenaPool(),
- /*for_boot_image=*/ options_.GetProfileBootClassPath());
- if (OS::FileExists(filename.c_str())) {
- if (!info.Load(filename, /*clear_if_invalid=*/true)) {
- LOG(WARNING) << "Could not forcefully load profile " << filename;
- continue;
- }
- } else {
- // Create a file if it doesn't exist.
- unix_file::FdFile file(filename,
- O_WRONLY | O_TRUNC | O_CREAT,
- S_IRUSR | S_IWUSR,
- /*check_usage=*/false);
- if (!file.IsValid()) {
- LOG(WARNING) << "Could not create profile " << filename;
- continue;
- }
+ /*for_boot_image=*/options_.GetProfileBootClassPath());
+ // Load the existing profile before saving.
+ // If the file is updated between `Load` and `Save`, the update will be lost. This is
+ // acceptable. The main reason is that the lost entries will eventually come back if the user
+ // keeps using the same methods, or they won't be needed if the user doesn't use the same
+ // methods again.
+ if (!info.Load(filename, /*clear_if_invalid=*/true)) {
+ LOG(WARNING) << "Could not forcefully load profile " << filename;
+ continue;
}
uint64_t last_save_number_of_methods = info.GetNumberOfMethods();