Merge "Only compile hot methods"

10 files changed, 47 insertions, 38 deletions

diff --git a/cmdline/cmdline_parser_test.cc b/cmdline/cmdline_parser_test.cc
index 9f12f64a31..07639e8a7d 100644
--- a/cmdline/cmdline_parser_test.cc
+++ b/cmdline/cmdline_parser_test.cc
@@ -35,7 +35,7 @@ namespace art {
     return lhs.enabled_ == rhs.enabled_ &&
         lhs.min_save_period_ms_ == rhs.min_save_period_ms_ &&
         lhs.save_resolved_classes_delay_ms_ == rhs.save_resolved_classes_delay_ms_ &&
-        lhs.startup_method_samples_ == rhs.startup_method_samples_ &&
+        lhs.hot_startup_method_samples_ == rhs.hot_startup_method_samples_ &&
         lhs.min_methods_to_save_ == rhs.min_methods_to_save_ &&
         lhs.min_classes_to_save_ == rhs.min_classes_to_save_ &&
         lhs.min_notification_before_wake_ == rhs.min_notification_before_wake_ &&
@@ -490,7 +490,7 @@ TEST_F(CmdlineParserTest, ProfileSaverOptions) {
                             "-Xjitsaveprofilinginfo "
                             "-Xps-min-save-period-ms:1 "
                             "-Xps-save-resolved-classes-delay-ms:2 "
-                            "-Xps-startup-method-samples:3 "
+                            "-Xps-hot-startup-method-samples:3 "
                             "-Xps-min-methods-to-save:4 "
                             "-Xps-min-classes-to-save:5 "
                             "-Xps-min-notification-before-wake:6 "
diff --git a/cmdline/cmdline_types.h b/cmdline/cmdline_types.h
index 0d2aed8ad1..185a0e403e 100644
--- a/cmdline/cmdline_types.h
+++ b/cmdline/cmdline_types.h
@@ -727,10 +727,10 @@ struct CmdlineType<ProfileSaverOptions> : CmdlineTypeParser<ProfileSaverOptions>
              &ProfileSaverOptions::save_resolved_classes_delay_ms_,
              type_parser.Parse(suffix));
     }
-    if (android::base::StartsWith(option, "startup-method-samples:")) {
+    if (android::base::StartsWith(option, "hot-startup-method-samples:")) {
       CmdlineType<unsigned int> type_parser;
       return ParseInto(existing,
-             &ProfileSaverOptions::startup_method_samples_,
+             &ProfileSaverOptions::hot_startup_method_samples_,
              type_parser.Parse(suffix));
     }
     if (android::base::StartsWith(option, "min-methods-to-save:")) {
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 69f853a1c9..29413d9a0a 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -1000,8 +1000,9 @@ bool CompilerDriver::ShouldCompileBasedOnProfile(const MethodReference& method_r
   if (profile_compilation_info_ == nullptr) {
     return false;
   }
-  // TODO: Revisit compiling all startup methods. b/36457259
-  bool result = profile_compilation_info_->IsStartupOrHotMethod(method_ref);
+  // Compile only hot methods, it is the profile saver's job to decide what startup methods to mark
+  // as hot.
+  bool result = profile_compilation_info_->ContainsHotMethod(method_ref);
 
   if (kDebugProfileGuidedCompilation) {
     LOG(INFO) << "[ProfileGuidedCompilation] "
diff --git a/dexlayout/dex_visualize.cc b/dexlayout/dex_visualize.cc
index 829e9feda8..d279bcb65c 100644
--- a/dexlayout/dex_visualize.cc
+++ b/dexlayout/dex_visualize.cc
@@ -174,7 +174,7 @@ class Dumper {
                       ProfileCompilationInfo* profile_info) {
     if (profile_info != nullptr) {
       uint32_t method_idx = method->GetMethodId()->GetIndex();
-      if (!profile_info->ContainsMethod(MethodReference(dex_file, method_idx))) {
+      if (!profile_info->ContainsHotMethod(MethodReference(dex_file, method_idx))) {
         return;
       }
     }
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index c4e6bfeea5..3852a5bc53 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -1250,7 +1250,7 @@ bool ProfileCompilationInfo::IsStartupOrHotMethod(const std::string& dex_locatio
   return method_it != methods.end();
 }
 
-bool ProfileCompilationInfo::ContainsMethod(const MethodReference& method_ref) const {
+bool ProfileCompilationInfo::ContainsHotMethod(const MethodReference& method_ref) const {
   return FindMethod(method_ref.dex_file->GetLocation(),
                     method_ref.dex_file->GetLocationChecksum(),
                     method_ref.dex_method_index) != nullptr;
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index ca204d8140..a9f2fb6dd8 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -246,8 +246,8 @@ class ProfileCompilationInfo {
                             uint32_t dex_checksum,
                             uint16_t dex_method_index) const;
 
-  // Return true if the method reference is present in the profiling info.
-  bool ContainsMethod(const MethodReference& method_ref) const;
+  // Return true if the method reference iS present and hot in the profiling info.
+  bool ContainsHotMethod(const MethodReference& method_ref) const;
 
   // Return true if the class's type is present in the profiling info.
   bool ContainsClass(const DexFile& dex_file, dex::TypeIndex type_idx) const;
diff --git a/runtime/jit/profile_compilation_info_test.cc b/runtime/jit/profile_compilation_info_test.cc
index 615149feb3..39670afb0c 100644
--- a/runtime/jit/profile_compilation_info_test.cc
+++ b/runtime/jit/profile_compilation_info_test.cc
@@ -298,7 +298,8 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethods) {
   {
     ScopedObjectAccess soa(self);
     for (ArtMethod* m : main_methods) {
-      ASSERT_TRUE(info1.ContainsMethod(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())));
+      ASSERT_TRUE(info1.ContainsHotMethod(
+          MethodReference(m->GetDexFile(), m->GetDexMethodIndex())));
     }
   }
 
@@ -314,10 +315,12 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethods) {
   {
     ScopedObjectAccess soa(self);
     for (ArtMethod* m : main_methods) {
-      ASSERT_TRUE(info2.ContainsMethod(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())));
+      ASSERT_TRUE(
+          info2.ContainsHotMethod(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())));
     }
     for (ArtMethod* m : second_methods) {
-      ASSERT_TRUE(info2.ContainsMethod(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())));
+      ASSERT_TRUE(
+          info2.ContainsHotMethod(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())));
     }
   }
 }
@@ -663,7 +666,8 @@ TEST_F(ProfileCompilationInfoTest, SaveArtMethodsWithInlineCaches) {
   {
     ScopedObjectAccess soa(self);
     for (ArtMethod* m : main_methods) {
-      ASSERT_TRUE(info.ContainsMethod(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())));
+      ASSERT_TRUE(
+          info.ContainsHotMethod(MethodReference(m->GetDexFile(), m->GetDexMethodIndex())));
       const ProfileMethodInfo& pmi = profile_methods_map.find(m)->second;
       std::unique_ptr<ProfileCompilationInfo::OfflineProfileMethodInfo> offline_pmi =
           info.GetMethod(m->GetDexFile()->GetLocation(),
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index c96ca88874..6128d82e24 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -184,11 +184,11 @@ void ProfileSaver::NotifyJitActivityInternal() {
 class GetMethodsVisitor : public ClassVisitor {
  public:
   GetMethodsVisitor(std::vector<MethodReference>* hot_methods,
-                    std::vector<MethodReference>* startup_methods,
-                    uint32_t startup_method_samples)
+                    std::vector<MethodReference>* sampled_methods,
+                    uint32_t hot_method_sample_threshold)
     : hot_methods_(hot_methods),
-      startup_methods_(startup_methods),
-      startup_method_samples_(startup_method_samples) {}
+      sampled_methods_(sampled_methods),
+      hot_method_sample_threshold_(hot_method_sample_threshold) {}
 
   virtual bool operator()(ObjPtr<mirror::Class> klass) REQUIRES_SHARED(Locks::mutator_lock_) {
     if (Runtime::Current()->GetHeap()->ObjectIsInBootImageSpace(klass)) {
@@ -198,12 +198,14 @@ class GetMethodsVisitor : public ClassVisitor {
       if (!method.IsNative() && !method.IsProxyMethod()) {
         const uint16_t counter = method.GetCounter();
         MethodReference ref(method.GetDexFile(), method.GetDexMethodIndex());
+        // Mark startup methods as hot if they have more than hot_method_sample_threshold_ samples.
+        // This means they will get compiled by the compiler driver.
         if (method.GetProfilingInfo(kRuntimePointerSize) != nullptr ||
-            (method.GetAccessFlags() & kAccPreviouslyWarm) != 0) {
+            (method.GetAccessFlags() & kAccPreviouslyWarm) != 0 ||
+            counter >= hot_method_sample_threshold_) {
           hot_methods_->push_back(ref);
-          startup_methods_->push_back(ref);
-        } else if (counter >= startup_method_samples_) {
-          startup_methods_->push_back(ref);
+        } else if (counter != 0) {
+          sampled_methods_->push_back(ref);
         }
       } else {
         CHECK_EQ(method.GetCounter(), 0u);
@@ -214,8 +216,8 @@ class GetMethodsVisitor : public ClassVisitor {
 
  private:
   std::vector<MethodReference>* const hot_methods_;
-  std::vector<MethodReference>* const startup_methods_;
-  uint32_t startup_method_samples_;
+  std::vector<MethodReference>* const sampled_methods_;
+  uint32_t hot_method_sample_threshold_;
 };
 
 void ProfileSaver::FetchAndCacheResolvedClassesAndMethods() {
@@ -241,11 +243,11 @@ void ProfileSaver::FetchAndCacheResolvedClassesAndMethods() {
       ScopedTrace trace2("Get hot methods");
       GetMethodsVisitor visitor(&hot_methods,
                                 &startup_methods,
-                                options_.GetStartupMethodSamples());
+                                options_.GetHotStartupMethodSamples());
       class_linker->VisitClasses(&visitor);
       VLOG(profiler) << "Profile saver recorded " << hot_methods.size() << " hot methods and "
                      << startup_methods.size() << " startup methods with threshold "
-                     << options_.GetStartupMethodSamples();
+                     << options_.GetHotStartupMethodSamples();
     }
   }
   MutexLock mu(self, *Locks::profiler_lock_);
@@ -256,12 +258,14 @@ void ProfileSaver::FetchAndCacheResolvedClassesAndMethods() {
     const std::string& filename = it.first;
     const std::set<std::string>& locations = it.second;
     std::vector<ProfileMethodInfo> profile_methods_for_location;
+    std::vector<MethodReference> startup_methods_for_locations;
     for (const MethodReference& ref : hot_methods) {
       if (locations.find(ref.dex_file->GetBaseLocation()) != locations.end()) {
         profile_methods_for_location.emplace_back(ref.dex_file, ref.dex_method_index);
+        // Hot methods are also startup methods since this function is only invoked during startup.
+        startup_methods_for_locations.push_back(ref);
       }
     }
-    std::vector<MethodReference> startup_methods_for_locations;
     for (const MethodReference& ref : startup_methods) {
       if (locations.find(ref.dex_file->GetBaseLocation()) != locations.end()) {
         startup_methods_for_locations.push_back(ref);
@@ -618,7 +622,7 @@ bool ProfileSaver::HasSeenMethod(const std::string& profile,
     if (!info.Load(profile, /*clear_if_invalid*/false)) {
       return false;
     }
-    return info.ContainsMethod(MethodReference(dex_file, method_idx));
+    return info.ContainsHotMethod(MethodReference(dex_file, method_idx));
   }
   return false;
 }
diff --git a/runtime/jit/profile_saver_options.h b/runtime/jit/profile_saver_options.h
index 07aeb66eb6..455bc1aae4 100644
--- a/runtime/jit/profile_saver_options.h
+++ b/runtime/jit/profile_saver_options.h
@@ -22,8 +22,8 @@ struct ProfileSaverOptions {
  public:
   static constexpr uint32_t kMinSavePeriodMs = 40 * 1000;  // 40 seconds
   static constexpr uint32_t kSaveResolvedClassesDelayMs = 5 * 1000;  // 5 seconds
-  // Minimum number of JIT samples during launch to include a method into the profile.
-  static constexpr uint32_t kStartupMethodSamples = 1;
+  // Minimum number of JIT samples during launch to mark a method as hot in the profile.
+  static constexpr uint32_t kHotStartupMethodSamples = 1;
   static constexpr uint32_t kMinMethodsToSave = 10;
   static constexpr uint32_t kMinClassesToSave = 10;
   static constexpr uint32_t kMinNotificationBeforeWake = 10;
@@ -33,7 +33,7 @@ struct ProfileSaverOptions {
     enabled_(false),
     min_save_period_ms_(kMinSavePeriodMs),
     save_resolved_classes_delay_ms_(kSaveResolvedClassesDelayMs),
-    startup_method_samples_(kStartupMethodSamples),
+    hot_startup_method_samples_(kHotStartupMethodSamples),
     min_methods_to_save_(kMinMethodsToSave),
     min_classes_to_save_(kMinClassesToSave),
     min_notification_before_wake_(kMinNotificationBeforeWake),
@@ -44,7 +44,7 @@ struct ProfileSaverOptions {
       bool enabled,
       uint32_t min_save_period_ms,
       uint32_t save_resolved_classes_delay_ms,
-      uint32_t startup_method_samples,
+      uint32_t hot_startup_method_samples,
       uint32_t min_methods_to_save,
       uint32_t min_classes_to_save,
       uint32_t min_notification_before_wake,
@@ -53,7 +53,7 @@ struct ProfileSaverOptions {
     enabled_(enabled),
     min_save_period_ms_(min_save_period_ms),
     save_resolved_classes_delay_ms_(save_resolved_classes_delay_ms),
-    startup_method_samples_(startup_method_samples),
+    hot_startup_method_samples_(hot_startup_method_samples),
     min_methods_to_save_(min_methods_to_save),
     min_classes_to_save_(min_classes_to_save),
     min_notification_before_wake_(min_notification_before_wake),
@@ -73,8 +73,8 @@ struct ProfileSaverOptions {
   uint32_t GetSaveResolvedClassesDelayMs() const {
     return save_resolved_classes_delay_ms_;
   }
-  uint32_t GetStartupMethodSamples() const {
-    return startup_method_samples_;
+  uint32_t GetHotStartupMethodSamples() const {
+    return hot_startup_method_samples_;
   }
   uint32_t GetMinMethodsToSave() const {
     return min_methods_to_save_;
@@ -96,7 +96,7 @@ struct ProfileSaverOptions {
     os << "enabled_" << pso.enabled_
         << ", min_save_period_ms_" << pso.min_save_period_ms_
         << ", save_resolved_classes_delay_ms_" << pso.save_resolved_classes_delay_ms_
-        << ", startup_method_samples_" << pso.startup_method_samples_
+        << ", hot_startup_method_samples_" << pso.hot_startup_method_samples_
         << ", min_methods_to_save_" << pso.min_methods_to_save_
         << ", min_classes_to_save_" << pso.min_classes_to_save_
         << ", min_notification_before_wake_" << pso.min_notification_before_wake_
@@ -107,7 +107,7 @@ struct ProfileSaverOptions {
   bool enabled_;
   uint32_t min_save_period_ms_;
   uint32_t save_resolved_classes_delay_ms_;
-  uint32_t startup_method_samples_;
+  uint32_t hot_startup_method_samples_;
   uint32_t min_methods_to_save_;
   uint32_t min_classes_to_save_;
   uint32_t min_notification_before_wake_;
diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc
index ef4957c0ba..abb6f8c018 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -709,7 +709,7 @@ void ParsedOptions::Usage(const char* fmt, ...) {
   UsageMessage(stream, "  -Xmethod-trace-file-size:integervalue\n");
   UsageMessage(stream, "  -Xps-min-save-period-ms:integervalue\n");
   UsageMessage(stream, "  -Xps-save-resolved-classes-delay-ms:integervalue\n");
-  UsageMessage(stream, "  -Xps-startup-method-samples:integervalue\n");
+  UsageMessage(stream, "  -Xps-hot-startup-method-samples:integervalue\n");
   UsageMessage(stream, "  -Xps-min-methods-to-save:integervalue\n");
   UsageMessage(stream, "  -Xps-min-classes-to-save:integervalue\n");
   UsageMessage(stream, "  -Xps-min-notification-before-wake:integervalue\n");