Do not cache profiles in the ProfileSaver.

Profiles with a lot of inline caches have a large memory
footprint. Do not cache them in the saver anymore.

The cache was useful to avoid doing unnecessary IO but it
takes too much memory now. Disable it until we update the
profile format to take less space.

This trades off IO for memory. As an effect we will do one extra
read to detect if we have enough new information worth saving. The
saving period has been increased to 40secs (from 20) in a previous
CL which also helps to balance the IO.

Test: m test-art-host-gtest
      manual inspection with meminfo
Bug: 37711886

Change-Id: I9cfdd6fb70c289221e74ccf1b6449f28a7fb693d
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index d922bc4..7a5de2a 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -117,9 +117,7 @@
   return true;
 }
 
-bool ProfileCompilationInfo::MergeAndSave(const std::string& filename,
-                                          uint64_t* bytes_written,
-                                          bool force) {
+bool ProfileCompilationInfo::Load(const std::string& filename, bool clear_if_invalid) {
   ScopedTrace trace(__PRETTY_FUNCTION__);
   ScopedFlock flock;
   std::string error;
@@ -134,36 +132,42 @@
 
   int fd = flock.GetFile()->Fd();
 
-  // Load the file but keep a copy around to be able to infer if the content has changed.
-  ProfileCompilationInfo fileInfo;
-  ProfileLoadSatus status = fileInfo.LoadInternal(fd, &error);
+  ProfileLoadSatus status = 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;
-      }
+    return true;
+  }
+
+  if (clear_if_invalid &&
+      ((status == kProfileLoadVersionMismatch) || (status == kProfileLoadBadData))) {
+    LOG(WARNING) << "Clearing bad or obsolete profile data from file "
+                 << filename << ": " << error;
+    if (flock.GetFile()->ClearContent()) {
+      return true;
     } else {
-      LOG(WARNING) << "Could not merge previous profile data from file " << filename;
-      if (!force) {
-        return false;
-      }
+      PLOG(WARNING) << "Could not clear profile file: " << filename;
+      return false;
     }
-  } 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;
+  }
+
+  LOG(WARNING) << "Could not load profile data from file " << filename << ": " << error;
+  return false;
+}
+
+bool ProfileCompilationInfo::Save(const std::string& filename, uint64_t* bytes_written) {
+  ScopedTrace trace(__PRETTY_FUNCTION__);
+  ScopedFlock flock;
+  std::string error;
+  int flags = O_WRONLY | 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;
   }
 
+  int fd = flock.GetFile()->Fd();
+
   // 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;
@@ -174,10 +178,14 @@
   // access and fail immediately if we can't.
   bool result = Save(fd);
   if (result) {
-    VLOG(profiler) << "Successfully saved profile info to " << filename
-        << " Size: " << GetFileSizeBytes(filename);
-    if (bytes_written != nullptr) {
-      *bytes_written = GetFileSizeBytes(filename);
+    int64_t size = GetFileSizeBytes(filename);
+    if (size != -1) {
+      VLOG(profiler)
+        << "Successfully saved profile info to " << filename << " Size: "
+        << size;
+      if (bytes_written != nullptr) {
+        *bytes_written = static_cast<uint64_t>(size);
+      }
     }
   } else {
     VLOG(profiler) << "Failed to save profile info to " << filename;
@@ -886,6 +894,7 @@
   }
 }
 
+// TODO(calin): fail fast if the dex checksums don't match.
 ProfileCompilationInfo::ProfileLoadSatus ProfileCompilationInfo::LoadInternal(
       int fd, std::string* error) {
   ScopedTrace trace(__PRETTY_FUNCTION__);
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index 9e47cc1..ee1935f 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -196,18 +196,20 @@
   // If the current profile is non-empty the load will fail.
   bool Load(int fd);
 
+  // Load profile information from the given file
+  // 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.
+  bool Load(const std::string& filename, bool clear_if_invalid);
+
   // Merge the data from another ProfileCompilationInfo into the current object.
   bool MergeWith(const ProfileCompilationInfo& info);
 
   // Save the profile data to the given file descriptor.
   bool Save(int fd);
 
-  // Load and merge profile information from the given file into the current
-  // object and tries to save it back to disk.
-  // 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);
+  // Save the current profile into the given file. The file will be cleared before saving.
+  bool Save(const std::string& filename, uint64_t* bytes_written);
 
   // Return 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 c9f2d0e..e8f4ce2 100644
--- a/runtime/jit/profile_compilation_info_test.cc
+++ b/runtime/jit/profile_compilation_info_test.cc
@@ -92,7 +92,15 @@
     if (info.GetNumberOfMethods() != profile_methods.size()) {
       return false;
     }
-    return info.MergeAndSave(filename, nullptr, false);
+    ProfileCompilationInfo file_profile;
+    if (!file_profile.Load(filename, false)) {
+      return false;
+    }
+    if (!info.MergeWith(file_profile)) {
+      return false;
+    }
+
+    return info.Save(filename, nullptr);
   }
 
   // Saves the given art methods to a profile backed by 'filename' and adds
@@ -145,7 +153,7 @@
     if (info.GetNumberOfMethods() != profile_methods.size()) {
       return false;
     }
-    return info.MergeAndSave(filename, nullptr, false);
+    return info.Save(filename, nullptr);
   }
 
   ProfileCompilationInfo::OfflineProfileMethodInfo ConvertProfileMethodInfo(
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index 1441987..1e6f7a8 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -171,14 +171,6 @@
   }
 }
 
-ProfileSaver::ProfileInfoCache* ProfileSaver::GetCachedProfiledInfo(const std::string& filename) {
-  auto info_it = profile_cache_.find(filename);
-  if (info_it == profile_cache_.end()) {
-    info_it = profile_cache_.Put(filename, ProfileInfoCache());
-  }
-  return &info_it->second;
-}
-
 // Get resolved methods that have a profile info or more than kStartupMethodSamples samples.
 // Excludes native methods and classes in the boot image.
 class GetMethodsVisitor : public ClassVisitor {
@@ -252,9 +244,11 @@
                        << " (" << classes.GetDexLocation() << ")";
       }
     }
-    ProfileInfoCache* cached_info = GetCachedProfiledInfo(filename);
-    cached_info->profile.AddMethodsAndClasses(profile_methods_for_location,
-                                              resolved_classes_for_location);
+    auto info_it = profile_cache_.Put(filename, ProfileCompilationInfo());
+
+    ProfileCompilationInfo* cached_info = &(info_it->second);
+    cached_info->AddMethodsAndClasses(profile_methods_for_location,
+                                      resolved_classes_for_location);
     total_number_of_profile_entries_cached += resolved_classes_for_location.size();
   }
   max_number_of_profile_entries_cached_ = std::max(
@@ -297,16 +291,22 @@
       jit_code_cache_->GetProfiledMethods(locations, profile_methods);
       total_number_of_code_cache_queries_++;
     }
+    ProfileCompilationInfo info;
+    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();
+    uint64_t last_save_number_of_classes = info.GetNumberOfResolvedClasses();
 
-    ProfileInfoCache* cached_info = GetCachedProfiledInfo(filename);
-    ProfileCompilationInfo* cached_profile = &cached_info->profile;
-    cached_profile->AddMethodsAndClasses(profile_methods, std::set<DexCacheResolvedClasses>());
-    int64_t delta_number_of_methods =
-        cached_profile->GetNumberOfMethods() -
-        static_cast<int64_t>(cached_info->last_save_number_of_methods);
-    int64_t delta_number_of_classes =
-        cached_profile->GetNumberOfResolvedClasses() -
-        static_cast<int64_t>(cached_info->last_save_number_of_classes);
+    info.AddMethodsAndClasses(profile_methods, std::set<DexCacheResolvedClasses>());
+    auto profile_cache_it = profile_cache_.find(filename);
+    if (profile_cache_it != profile_cache_.end()) {
+      info.MergeWith(profile_cache_it->second);
+    }
+
+    int64_t delta_number_of_methods = info.GetNumberOfMethods() - last_save_number_of_methods;
+    int64_t delta_number_of_classes = info.GetNumberOfResolvedClasses() - last_save_number_of_classes;
 
     if (!force_save &&
         delta_number_of_methods < options_.GetMinMethodsToSave() &&
@@ -324,12 +324,12 @@
     uint64_t 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_profile->MergeAndSave(filename, &bytes_written, /*force*/ true)) {
-      cached_info->last_save_number_of_methods = cached_profile->GetNumberOfMethods();
-      cached_info->last_save_number_of_classes = cached_profile->GetNumberOfResolvedClasses();
-      // Clear resolved classes. No need to store them around as
-      // they don't change after the first write.
-      cached_profile->ClearResolvedClasses();
+    if (info.Save(filename, &bytes_written)) {
+      // We managed to save the profile. Clear the cache stored during startup.
+      if (profile_cache_it != profile_cache_.end()) {
+        profile_cache_.erase(profile_cache_it);
+        total_number_of_profile_entries_cached = 0;
+      }
       if (bytes_written > 0) {
         total_number_of_writes_++;
         total_bytes_written_ += bytes_written;
@@ -345,13 +345,8 @@
       LOG(WARNING) << "Could not save profiling info to " << filename;
       total_number_of_failed_writes_++;
     }
-    total_number_of_profile_entries_cached +=
-        cached_profile->GetNumberOfMethods() +
-        cached_profile->GetNumberOfResolvedClasses();
   }
-  max_number_of_profile_entries_cached_ = std::max(
-      max_number_of_profile_entries_cached_,
-      total_number_of_profile_entries_cached);
+
   return profile_file_saved;
 }
 
@@ -575,7 +570,10 @@
                                  uint16_t method_idx) {
   MutexLock mu(Thread::Current(), *Locks::profiler_lock_);
   if (instance_ != nullptr) {
-    const ProfileCompilationInfo& info = instance_->GetCachedProfiledInfo(profile)->profile;
+    ProfileCompilationInfo info;
+    if (!info.Load(profile, /*clear_if_invalid*/false)) {
+      return false;
+    }
     return info.ContainsMethod(MethodReference(dex_file, method_idx));
   }
   return false;
diff --git a/runtime/jit/profile_saver.h b/runtime/jit/profile_saver.h
index bd539a4..60c9cc6 100644
--- a/runtime/jit/profile_saver.h
+++ b/runtime/jit/profile_saver.h
@@ -61,14 +61,6 @@
                             uint16_t method_idx);
 
  private:
-  // A cache structure which keeps track of the data saved to disk.
-  // It is used to reduce the number of disk read/writes.
-  struct ProfileInfoCache {
-    ProfileCompilationInfo profile;
-    uint32_t last_save_number_of_methods = 0;
-    uint32_t last_save_number_of_classes = 0;
-  };
-
   ProfileSaver(const ProfileSaverOptions& options,
                const std::string& output_filename,
                jit::JitCodeCache* jit_code_cache,
@@ -102,10 +94,6 @@
                            const std::vector<std::string>& code_paths)
       REQUIRES(Locks::profiler_lock_);
 
-  // Retrieves the cached profile compilation info for the given profile file.
-  // If no entry exists, a new empty one will be created, added to the cache and
-  // then returned.
-  ProfileInfoCache* GetCachedProfiledInfo(const std::string& filename);
   // Fetches the current resolved classes and methods from the ClassLinker and stores them in the
   // profile_cache_ for later save.
   void FetchAndCacheResolvedClassesAndMethods();
@@ -139,10 +127,11 @@
   uint32_t jit_activity_notifications_;
 
   // A local cache for the profile information. Maps each tracked file to its
-  // profile information. The size of this cache is usually very small and tops
+  // profile information. This is used to cache the startup classes so that
+  // we don't hammer the disk to save them right away.
+  // The size of this cache is usually very small and tops
   // to just a few hundreds entries in the ProfileCompilationInfo objects.
-  // It helps avoiding unnecessary writes to disk.
-  SafeMap<std::string, ProfileInfoCache> profile_cache_;
+  SafeMap<std::string, ProfileCompilationInfo> profile_cache_;
 
   // Save period condition support.
   Mutex wait_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;