diff options
-rw-r--r-- | cmdline/cmdline_parser_test.cc | 24 | ||||
-rw-r--r-- | cmdline/cmdline_types.h | 6 | ||||
-rw-r--r-- | runtime/jit/profile_saver.cc | 127 | ||||
-rw-r--r-- | runtime/jit/profile_saver.h | 5 | ||||
-rw-r--r-- | runtime/jit/profile_saver_options.h | 84 |
5 files changed, 100 insertions, 146 deletions
diff --git a/cmdline/cmdline_parser_test.cc b/cmdline/cmdline_parser_test.cc index e586dd4292..ea688d650c 100644 --- a/cmdline/cmdline_parser_test.cc +++ b/cmdline/cmdline_parser_test.cc @@ -36,12 +36,11 @@ namespace art { // This has a gtest dependency, which is why it's in the gtest only. bool operator==(const ProfileSaverOptions& lhs, const ProfileSaverOptions& rhs) { 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.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_ && - lhs.max_notification_before_wake_ == rhs.max_notification_before_wake_; + lhs.min_save_period_ms_ == rhs.min_save_period_ms_ && + 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_ && + lhs.max_notification_before_wake_ == rhs.max_notification_before_wake_; } bool UsuallyEquals(double expected, double actual) { @@ -489,18 +488,17 @@ TEST_F(CmdlineParserTest, TestJitOptions) { * -Xps-* */ TEST_F(CmdlineParserTest, ProfileSaverOptions) { - ProfileSaverOptions opt = ProfileSaverOptions(true, 1, 2, 3, 4, 5, 6, 7, 8, "abc", true); + ProfileSaverOptions opt = ProfileSaverOptions(true, 1, 2, 3, 4, 5, 6, 7, "abc", true); EXPECT_SINGLE_PARSE_VALUE(opt, "-Xjitsaveprofilinginfo " "-Xps-min-save-period-ms:1 " "-Xps-min-first-save-ms:2 " - "-Xps-save-resolved-classes-delay-ms:3 " - "-Xps-min-methods-to-save:4 " - "-Xps-min-classes-to-save:5 " - "-Xps-min-notification-before-wake:6 " - "-Xps-max-notification-before-wake:7 " - "-Xps-inline-cache-threshold:8 " + "-Xps-min-methods-to-save:3 " + "-Xps-min-classes-to-save:4 " + "-Xps-min-notification-before-wake:5 " + "-Xps-max-notification-before-wake:6 " + "-Xps-inline-cache-threshold:7 " "-Xps-profile-path:abc " "-Xps-profile-boot-class-path", M::ProfileSaverOpts); diff --git a/cmdline/cmdline_types.h b/cmdline/cmdline_types.h index 4a33fb8b55..fe7a55d559 100644 --- a/cmdline/cmdline_types.h +++ b/cmdline/cmdline_types.h @@ -826,10 +826,8 @@ struct CmdlineType<ProfileSaverOptions> : CmdlineTypeParser<ProfileSaverOptions> type_parser.Parse(suffix)); } if (option.starts_with("save-resolved-classes-delay-ms:")) { - CmdlineType<unsigned int> type_parser; - return ParseInto(existing, - &ProfileSaverOptions::save_resolved_classes_delay_ms_, - type_parser.Parse(suffix)); + LOG(WARNING) << "-Xps-save-resolved-classes-delay-ms is deprecated"; + return Result::SuccessNoValue(); } if (option.starts_with("hot-startup-method-samples:")) { LOG(WARNING) << "-Xps-hot-startup-method-samples option is deprecated"; diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc index 3506faaa7f..81cb384f62 100644 --- a/runtime/jit/profile_saver.cc +++ b/runtime/jit/profile_saver.cc @@ -124,35 +124,27 @@ void ProfileSaver::Run() { // under mutex, but should drop it. Locks::profiler_lock_->ExclusiveUnlock(self); - bool check_for_first_save = - options_.GetMinFirstSaveMs() != ProfileSaverOptions::kMinFirstSaveMsNotSet; - bool force_early_first_save = check_for_first_save && IsFirstSave(); - - // Fetch the resolved classes for the app images after sleeping for - // options_.GetSaveResolvedClassesDelayMs(). - // TODO(calin) This only considers the case of the primary profile file. - // Anything that gets loaded in the same VM will not have their resolved - // classes save (unless they started before the initial saving was done). - { + // Fetch the resolved classes for the app images after waiting for Startup + // completion notification. + const uint64_t thread_start_time = NanoTime(); + + // Wait for startup to complete with a timeout at StartupCompletedTask. + // Note that we may be woken up by JIT notifications. + // We need to wait for startup to complete to make sure we have + // the resolved classes and methods. + while (!Runtime::Current()->GetStartupCompleted() && !ShuttingDown(self)) { MutexLock mu(self, wait_lock_); - - const uint64_t sleep_time = MsToNs(force_early_first_save - ? options_.GetMinFirstSaveMs() - : options_.GetSaveResolvedClassesDelayMs()); - const uint64_t start_time = NanoTime(); - const uint64_t end_time = start_time + sleep_time; - while (!Runtime::Current()->GetStartupCompleted() || force_early_first_save) { - const uint64_t current_time = NanoTime(); - if (current_time >= end_time) { - break; - } - period_condition_.TimedWait(self, NsToMs(end_time - current_time), 0); - } - total_ms_of_sleep_ += NsToMs(NanoTime() - start_time); + // Make sure to sleep again until startup is completed. + period_condition_.Wait(self); } + // Mark collected classes/methods as startup. FetchAndCacheResolvedClassesAndMethods(/*startup=*/ true); + bool is_min_first_save_set = + options_.GetMinFirstSaveMs() != ProfileSaverOptions::kMinFirstSaveMsNotSet; + bool force_first_save = is_min_first_save_set && IsFirstSave(); + // When we save without waiting for JIT notifications we use a simple // exponential back off policy bounded by max_wait_without_jit. uint32_t max_wait_without_jit = options_.GetMinSavePeriodMs() * 16; @@ -160,24 +152,35 @@ void ProfileSaver::Run() { // Loop for the profiled methods. while (!ShuttingDown(self)) { - // Sleep only if we don't have to force an early first save configured - // with GetMinFirstSaveMs(). - // If we do have to save early, move directly to the processing part - // since we already slept before fetching and resolving the startup - // classes. - if (!force_early_first_save) { - uint64_t sleep_start = NanoTime(); - uint64_t sleep_time = 0; + // In case of force_first_save we need to count from the start of the thread. + uint64_t sleep_start = force_first_save ? thread_start_time : NanoTime(); + uint64_t sleep_time = 0; + { + MutexLock mu(self, wait_lock_); + if (options_.GetWaitForJitNotificationsToSave()) { + period_condition_.Wait(self); + } else { + period_condition_.TimedWait(self, cur_wait_without_jit, 0); + if (cur_wait_without_jit < max_wait_without_jit) { + cur_wait_without_jit *= 2; + } + } + sleep_time = NanoTime() - sleep_start; + } + // Check if the thread was woken up for shutdown. + if (ShuttingDown(self)) { + break; + } + total_number_of_wake_ups_++; + // We might have been woken up by a huge number of notifications to guarantee saving. + // If we didn't meet the minimum saving period go back to sleep (only if missed by + // a reasonable margin). + uint64_t min_save_period_ns = MsToNs(force_first_save ? options_.GetMinFirstSaveMs() : + options_.GetMinSavePeriodMs()); + while (min_save_period_ns * 0.9 > sleep_time) { { MutexLock mu(self, wait_lock_); - if (options_.GetWaitForJitNotificationsToSave()) { - period_condition_.Wait(self); - } else { - period_condition_.TimedWait(self, cur_wait_without_jit, 0); - if (cur_wait_without_jit < max_wait_without_jit) { - cur_wait_without_jit *= 2; - } - } + period_condition_.TimedWait(self, NsToMs(min_save_period_ns - sleep_time), 0); sleep_time = NanoTime() - sleep_start; } // Check if the thread was woken up for shutdown. @@ -185,24 +188,8 @@ void ProfileSaver::Run() { break; } total_number_of_wake_ups_++; - // We might have been woken up by a huge number of notifications to guarantee saving. - // If we didn't meet the minimum saving period go back to sleep (only if missed by - // a reasonable margin). - uint64_t min_save_period_ns = MsToNs(options_.GetMinSavePeriodMs()); - while (min_save_period_ns * 0.9 > sleep_time) { - { - MutexLock mu(self, wait_lock_); - period_condition_.TimedWait(self, NsToMs(min_save_period_ns - sleep_time), 0); - sleep_time = NanoTime() - sleep_start; - } - // Check if the thread was woken up for shutdown. - if (ShuttingDown(self)) { - break; - } - total_number_of_wake_ups_++; - } - total_ms_of_sleep_ += NsToMs(NanoTime() - sleep_start); } + total_ms_of_sleep_ += NsToMs(NanoTime() - sleep_start); if (ShuttingDown(self)) { break; @@ -210,15 +197,12 @@ void ProfileSaver::Run() { uint16_t number_of_new_methods = 0; uint64_t start_work = NanoTime(); - // If we force an early_first_save do not run FetchAndCacheResolvedClassesAndMethods - // again. We just did it. So pass true to skip_class_and_method_fetching. bool profile_saved_to_disk = ProcessProfilingInfo( /*force_save=*/ false, - /*skip_class_and_method_fetching=*/ force_early_first_save, &number_of_new_methods); // Reset the flag, so we can continue on the normal schedule. - force_early_first_save = false; + force_first_save = false; // Update the notification counter based on result. Note that there might be contention on this // but we don't care about to be 100% precise. @@ -780,10 +764,7 @@ void ProfileSaver::FetchAndCacheResolvedClassesAndMethods(bool startup) { << " sampled methods in " << PrettyDuration(NanoTime() - start_time); } -bool ProfileSaver::ProcessProfilingInfo( - bool force_save, - bool skip_class_and_method_fetching, - /*out*/uint16_t* number_of_new_methods) { +bool ProfileSaver::ProcessProfilingInfo(bool force_save, /*out*/uint16_t* number_of_new_methods) { ScopedTrace trace(__PRETTY_FUNCTION__); // Resolve any new registered locations. @@ -801,11 +782,7 @@ bool ProfileSaver::ProcessProfilingInfo( *number_of_new_methods = 0; } - if (!skip_class_and_method_fetching) { - // We only need to do this once, not once per dex location. - // TODO: Figure out a way to only do it when stuff has changed? It takes 30-50ms. - FetchAndCacheResolvedClassesAndMethods(/*startup=*/ false); - } + FetchAndCacheResolvedClassesAndMethods(/*startup=*/ false); for (const auto& it : tracked_locations) { if (!force_save && ShuttingDown(Thread::Current())) { @@ -1085,10 +1062,7 @@ void ProfileSaver::Stop(bool dump_info) { // Force save everything before destroying the thread since we want profiler_pthread_ to remain // valid. - profile_saver->ProcessProfilingInfo( - /*force_ save=*/ true, - /*skip_class_and_method_fetching=*/ false, - /*number_of_new_methods=*/ nullptr); + profile_saver->ProcessProfilingInfo(/*force_ save=*/ true, /*number_of_new_methods=*/ nullptr); // Wait for the saver thread to stop. CHECK_PTHREAD_CALL(pthread_join, (profiler_pthread, nullptr), "profile saver thread shutdown"); @@ -1207,10 +1181,7 @@ void ProfileSaver::ForceProcessProfiles() { // but we only use this in testing when we now this won't happen. // Refactor the way we handle the instance so that we don't end up in this situation. if (saver != nullptr) { - saver->ProcessProfilingInfo( - /*force_save=*/ true, - /*skip_class_and_method_fetching=*/ false, - /*number_of_new_methods=*/ nullptr); + saver->ProcessProfilingInfo(/*force_save=*/ true, /*number_of_new_methods=*/ nullptr); } } diff --git a/runtime/jit/profile_saver.h b/runtime/jit/profile_saver.h index 1042db9a89..ee99bf1594 100644 --- a/runtime/jit/profile_saver.h +++ b/runtime/jit/profile_saver.h @@ -80,10 +80,7 @@ class ProfileSaver { // written to disk. // If force_save is true, the saver will ignore any constraints which limit IO (e.g. will write // the profile to disk even if it's just one new method). - bool ProcessProfilingInfo( - bool force_save, - bool skip_class_and_method_fetching, - /*out*/uint16_t* number_of_new_methods) + bool ProcessProfilingInfo(bool force_save, /*out*/uint16_t* number_of_new_methods) REQUIRES(!Locks::profiler_lock_) REQUIRES(!Locks::mutator_lock_); diff --git a/runtime/jit/profile_saver_options.h b/runtime/jit/profile_saver_options.h index ed2f00f48f..707ebf480a 100644 --- a/runtime/jit/profile_saver_options.h +++ b/runtime/jit/profile_saver_options.h @@ -25,55 +25,50 @@ struct ProfileSaverOptions { // Default value for the min save period on first use, indicating that the // period is not configured. static constexpr uint32_t kMinFirstSaveMsNotSet = 0; - static constexpr uint32_t kSaveResolvedClassesDelayMs = 5 * 1000; // 5 seconds static constexpr uint32_t kMinMethodsToSave = 10; static constexpr uint32_t kMinClassesToSave = 10; static constexpr uint32_t kMinNotificationBeforeWake = 10; static constexpr uint32_t kMaxNotificationBeforeWake = 50; static constexpr uint16_t kInlineCacheThreshold = 4000; - ProfileSaverOptions() : - enabled_(false), - min_save_period_ms_(kMinSavePeriodMs), - min_first_save_ms_(kMinFirstSaveMsNotSet), - save_resolved_classes_delay_ms_(kSaveResolvedClassesDelayMs), - min_methods_to_save_(kMinMethodsToSave), - min_classes_to_save_(kMinClassesToSave), - min_notification_before_wake_(kMinNotificationBeforeWake), - max_notification_before_wake_(kMaxNotificationBeforeWake), - inline_cache_threshold_(kInlineCacheThreshold), - profile_path_(""), - profile_boot_class_path_(false), - profile_aot_code_(false), - wait_for_jit_notifications_to_save_(true) {} + ProfileSaverOptions() + : enabled_(false), + min_save_period_ms_(kMinSavePeriodMs), + min_first_save_ms_(kMinFirstSaveMsNotSet), + min_methods_to_save_(kMinMethodsToSave), + min_classes_to_save_(kMinClassesToSave), + min_notification_before_wake_(kMinNotificationBeforeWake), + max_notification_before_wake_(kMaxNotificationBeforeWake), + inline_cache_threshold_(kInlineCacheThreshold), + profile_path_(""), + profile_boot_class_path_(false), + profile_aot_code_(false), + wait_for_jit_notifications_to_save_(true) {} - ProfileSaverOptions( - bool enabled, - uint32_t min_save_period_ms, - uint32_t min_first_save_ms, - uint32_t save_resolved_classes_delay_ms, - uint32_t min_methods_to_save, - uint32_t min_classes_to_save, - uint32_t min_notification_before_wake, - uint32_t max_notification_before_wake, - uint16_t inline_cache_threshold, - const std::string& profile_path, - bool profile_boot_class_path, - bool profile_aot_code = false, - bool wait_for_jit_notifications_to_save = true) - : enabled_(enabled), - min_save_period_ms_(min_save_period_ms), - min_first_save_ms_(min_first_save_ms), - save_resolved_classes_delay_ms_(save_resolved_classes_delay_ms), - min_methods_to_save_(min_methods_to_save), - min_classes_to_save_(min_classes_to_save), - min_notification_before_wake_(min_notification_before_wake), - max_notification_before_wake_(max_notification_before_wake), - inline_cache_threshold_(inline_cache_threshold), - profile_path_(profile_path), - profile_boot_class_path_(profile_boot_class_path), - profile_aot_code_(profile_aot_code), - wait_for_jit_notifications_to_save_(wait_for_jit_notifications_to_save) {} + ProfileSaverOptions(bool enabled, + uint32_t min_save_period_ms, + uint32_t min_first_save_ms, + uint32_t min_methods_to_save, + uint32_t min_classes_to_save, + uint32_t min_notification_before_wake, + uint32_t max_notification_before_wake, + uint16_t inline_cache_threshold, + const std::string& profile_path, + bool profile_boot_class_path, + bool profile_aot_code = false, + bool wait_for_jit_notifications_to_save = true) + : enabled_(enabled), + min_save_period_ms_(min_save_period_ms), + min_first_save_ms_(min_first_save_ms), + min_methods_to_save_(min_methods_to_save), + min_classes_to_save_(min_classes_to_save), + min_notification_before_wake_(min_notification_before_wake), + max_notification_before_wake_(max_notification_before_wake), + inline_cache_threshold_(inline_cache_threshold), + profile_path_(profile_path), + profile_boot_class_path_(profile_boot_class_path), + profile_aot_code_(profile_aot_code), + wait_for_jit_notifications_to_save_(wait_for_jit_notifications_to_save) {} bool IsEnabled() const { return enabled_; @@ -88,9 +83,6 @@ struct ProfileSaverOptions { uint32_t GetMinFirstSaveMs() const { return min_first_save_ms_; } - uint32_t GetSaveResolvedClassesDelayMs() const { - return save_resolved_classes_delay_ms_; - } uint32_t GetMinMethodsToSave() const { return min_methods_to_save_; } @@ -126,7 +118,6 @@ struct ProfileSaverOptions { os << "enabled_" << pso.enabled_ << ", min_save_period_ms_" << pso.min_save_period_ms_ << ", min_first_save_ms_" << pso.min_first_save_ms_ - << ", save_resolved_classes_delay_ms_" << pso.save_resolved_classes_delay_ms_ << ", 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_ @@ -141,7 +132,6 @@ struct ProfileSaverOptions { bool enabled_; uint32_t min_save_period_ms_; uint32_t min_first_save_ms_; - uint32_t save_resolved_classes_delay_ms_; uint32_t min_methods_to_save_; uint32_t min_classes_to_save_; uint32_t min_notification_before_wake_; |