Do not fsync profiles on close

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 close(), no sync, and let
the kernel decide when to write to disk. This should
improve the I/O behavior of saving profiles.

Bug: 36817443
Test: m test-art-host-gtest

(cherry picked from commit df674c45091d01f504bf1bb7d241678ecd449ae0)

Change-Id: Idf503beccf912e26f31abbcf66087afb9a8fe138

diff --git a/runtime/base/scoped_flock.cc b/runtime/base/scoped_flock.cc
index 5394e53..af57329 100644 (file)
--- a/runtime/base/scoped_flock.cc
+++ b/runtime/base/scoped_flock.cc
@@ -33,11 +33,22 @@ bool ScopedFlock::Init(const char* filename, std::string* error_msg) {
 }
 
 bool ScopedFlock::Init(const char* filename, int flags, bool block, std::string* error_msg) {
+  return Init(filename, flags, block, true, error_msg);
+}
+
+bool ScopedFlock::Init(const char* filename,
+                       int flags,
+                       bool block,
+                       bool flush_on_close,
+                       std::string* error_msg) {
+  flush_on_close_ = flush_on_close;
   while (true) {
     if (file_.get() != nullptr) {
       UNUSED(file_->FlushCloseOrErase());  // Ignore result.
     }
-    file_.reset(OS::OpenFileWithFlags(filename, flags));
+
+    bool check_usage = flush_on_close;  // Check usage only if we need to flush on close.
+    file_.reset(OS::OpenFileWithFlags(filename, flags, check_usage));
     if (file_.get() == nullptr) {
       *error_msg = StringPrintf("Failed to open file '%s': %s", filename, strerror(errno));
       return false;
@@ -86,6 +97,7 @@ bool ScopedFlock::Init(const char* filename, int flags, bool block, std::string*
 }
 
 bool ScopedFlock::Init(File* file, std::string* error_msg) {
+  flush_on_close_ = true;
   file_.reset(new File(dup(file->Fd()), file->GetPath(), file->CheckUsage(), file->ReadOnlyMode()));
   if (file_->Fd() == -1) {
     file_.reset();
@@ -111,7 +123,7 @@ bool ScopedFlock::HasFile() {
   return file_.get() != nullptr;
 }
 
-ScopedFlock::ScopedFlock() { }
+ScopedFlock::ScopedFlock() : flush_on_close_(true) { }
 
 ScopedFlock::~ScopedFlock() {
   if (file_.get() != nullptr) {
@@ -121,7 +133,7 @@ ScopedFlock::~ScopedFlock() {
       UNREACHABLE();
     }
     int close_result = -1;
-    if (file_->ReadOnlyMode()) {
+    if (file_->ReadOnlyMode() || !flush_on_close_) {
       close_result = file_->Close();
     } else {
       close_result = file_->FlushCloseOrErase();

diff --git a/runtime/base/scoped_flock.h b/runtime/base/scoped_flock.h
index cc22056..7196b02 100644 (file)
--- a/runtime/base/scoped_flock.h
+++ b/runtime/base/scoped_flock.h
@@ -38,7 +38,16 @@ class ScopedFlock {
   // locking will be retried if the file changed. In non-blocking mode, false
   // is returned and no attempt is made to re-acquire the lock.
   //
+  // The argument `flush_on_close` controls whether or not the file
+  // will be explicitly flushed before close.
+  //
   // The file is opened with the provided flags.
+  bool Init(const char* filename,
+            int flags,
+            bool block,
+            bool flush_on_close,
+            std::string* error_msg);
+  // Calls Init(filename, flags, block, true, error_msg);
   bool Init(const char* filename, int flags, bool block, std::string* error_msg);
   // Calls Init(filename, O_CREAT | O_RDWR, true, errror_msg)
   bool Init(const char* filename, std::string* error_msg);
@@ -57,6 +66,7 @@ class ScopedFlock {
 
  private:
   std::unique_ptr<File> file_;
+  bool flush_on_close_;
   DISALLOW_COPY_AND_ASSIGN(ScopedFlock);
 };
 

diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index 52649c7..0acce1e 100644 (file)
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -115,7 +115,11 @@ bool ProfileCompilationInfo::MergeAndSave(const std::string& filename,
   ScopedTrace trace(__PRETTY_FUNCTION__);
   ScopedFlock flock;
   std::string error;
-  if (!flock.Init(filename.c_str(), O_RDWR | O_NOFOLLOW | O_CLOEXEC, /* block */ false, &error)) {
+  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
+  // close(), no sync, and let to the kernel decide when to write to disk.
+  if (!flock.Init(filename.c_str(), flags, /*block*/false, /*flush_on_close*/false, &error)) {
     LOG(WARNING) << "Couldn't lock the profile file " << filename << ": " << error;
     return false;
   }

diff --git a/runtime/os.h b/runtime/os.h
index 46d89fb..7130fc3 100644 (file)
--- a/runtime/os.h
+++ b/runtime/os.h
@@ -44,7 +44,7 @@ class OS {
   static File* CreateEmptyFileWriteOnly(const char* name);
 
   // Open a file with the specified open(2) flags.
-  static File* OpenFileWithFlags(const char* name, int flags);
+  static File* OpenFileWithFlags(const char* name, int flags, bool auto_flush = true);
 
   // Check if a file exists.
   static bool FileExists(const char* name);

diff --git a/runtime/os_linux.cc b/runtime/os_linux.cc
index 1db09b4..0add496 100644 (file)
--- a/runtime/os_linux.cc
+++ b/runtime/os_linux.cc
@@ -51,10 +51,11 @@ File* OS::CreateEmptyFileWriteOnly(const char* name) {
   return art::CreateEmptyFile(name, O_WRONLY | O_TRUNC | O_NOFOLLOW | O_CLOEXEC);
 }
 
-File* OS::OpenFileWithFlags(const char* name, int flags) {
+File* OS::OpenFileWithFlags(const char* name, int flags, bool auto_flush) {
   CHECK(name != nullptr);
   bool read_only = ((flags & O_ACCMODE) == O_RDONLY);
-  std::unique_ptr<File> file(new File(name, flags, 0666, !read_only));
+  bool check_usage = !read_only && auto_flush;
+  std::unique_ptr<File> file(new File(name, flags, 0666, check_usage));
   if (!file->IsOpened()) {
     return nullptr;
   }