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
Change-Id: I0df27947006d57b15d6311420c79288148736f62

5 files changed, 34 insertions, 7 deletions

diff --git a/runtime/base/scoped_flock.cc b/runtime/base/scoped_flock.cc
index 5394e53fa3..af57329977 100644
--- 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 cc22056443..7196b025b1 100644
--- 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 52649c7075..0acce1e421 100644
--- 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 46d89fb8a5..7130fc3732 100644
--- 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 1db09b4445..0add4965d1 100644
--- 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;
   }