Revert^4 "[metrics] Add background reporting thread"

This adds a background thread that reports metrics every N seconds,
where N is specified by the -Xmetrics-reporting-period command line
option. Periodic reporting is disabled by default.

This reverts commit 1060838894e34785139b5e3583fbc9edad7fa7f9.

Reason for revert: Remove problematic test

Test: test/run-test --host 2233-metrics-background-thread
Test: adb shell stop && \
      adb shell setprop dalvik.vm.extra-opts \
          -Xmetrics-reporting-period=30\\\ -Xwrite-metrics-to-log && \
      adb shell start && \
      adb logcat  # observe metrics in log
Bug: 170149255
Change-Id: I3d72043bbb1e652728253585aae5486598658d2b

7 files changed, 140 insertions, 22 deletions

diff --git a/libartbase/base/time_utils.h b/libartbase/base/time_utils.h
index cb0ab13ef9..e1273a380a 100644
--- a/libartbase/base/time_utils.h
+++ b/libartbase/base/time_utils.h
@@ -87,6 +87,10 @@ static constexpr uint64_t UsToNs(uint64_t us) {
   return us * 1000;
 }
 
+static constexpr uint64_t SecondsToMs(uint64_t seconds) {
+  return seconds * 1000;
+}
+
 static constexpr time_t SaturatedTimeT(int64_t secs) {
   if (sizeof(time_t) < sizeof(int64_t)) {
     return static_cast<time_t>(std::min(secs,
diff --git a/openjdkjvmti/ti_thread.cc b/openjdkjvmti/ti_thread.cc
index 1a5b227ffc..98a0b9da9b 100644
--- a/openjdkjvmti/ti_thread.cc
+++ b/openjdkjvmti/ti_thread.cc
@@ -39,12 +39,13 @@
 #include "base/mutex.h"
 #include "deopt_manager.h"
 #include "events-inl.h"
-#include "gc/system_weak.h"
 #include "gc/collector_type.h"
 #include "gc/gc_cause.h"
 #include "gc/scoped_gc_critical_section.h"
+#include "gc/system_weak.h"
 #include "gc_root-inl.h"
 #include "jni/jni_internal.h"
+#include "metrics/metrics.h"
 #include "mirror/class.h"
 #include "mirror/object-inl.h"
 #include "mirror/string.h"
@@ -123,13 +124,12 @@ struct ThreadCallback : public art::ThreadLifecycleCallback {
     if (!started) {
       // Runtime isn't started. We only expect at most the signal handler or JIT threads to be
       // started here; this includes the perfetto_hprof_listener signal handler thread for
-      // perfetto_hprof.
+      // perfetto_hprof, as well as the metrics background reporting thread.
       if (art::kIsDebugBuild) {
         std::string name;
         self->GetThreadName(name);
-        if (name != "JDWP" &&
-            name != "Signal Catcher" &&
-            name != "perfetto_hprof_listener" &&
+        if (name != "JDWP" && name != "Signal Catcher" && name != "perfetto_hprof_listener" &&
+            name != art::metrics::MetricsReporter::kBackgroundThreadName &&
             !android::base::StartsWith(name, "Jit thread pool") &&
             !android::base::StartsWith(name, "Runtime worker thread")) {
           LOG(FATAL) << "Unexpected thread before start: " << name << " id: "
diff --git a/runtime/metrics/metrics.cc b/runtime/metrics/metrics.cc
index e85897b19e..60add77d5a 100644
--- a/runtime/metrics/metrics.cc
+++ b/runtime/metrics/metrics.cc
@@ -24,6 +24,7 @@
 #include "base/scoped_flock.h"
 #include "runtime.h"
 #include "runtime_options.h"
+#include "thread-current-inl.h"
 
 #pragma clang diagnostic push
 #pragma clang diagnostic error "-Wconversion"
@@ -119,20 +120,75 @@ void StreamBackend::ReportHistogram(DatumId histogram_type,
   }
 }
 
-std::unique_ptr<MetricsReporter> MetricsReporter::Create(ReportingConfig config,
-                                                         const ArtMetrics* metrics) {
-  std::unique_ptr<MetricsBackend> backend;
-
+std::unique_ptr<MetricsReporter> MetricsReporter::Create(ReportingConfig config, Runtime* runtime) {
   // We can't use std::make_unique here because the MetricsReporter constructor is private.
-  return std::unique_ptr<MetricsReporter>{new MetricsReporter{std::move(config), metrics}};
+  return std::unique_ptr<MetricsReporter>{new MetricsReporter{std::move(config), runtime}};
+}
+
+MetricsReporter::MetricsReporter(ReportingConfig config, Runtime* runtime)
+    : config_{std::move(config)}, runtime_{runtime} {}
+
+MetricsReporter::~MetricsReporter() { MaybeStopBackgroundThread(); }
+
+void MetricsReporter::MaybeStartBackgroundThread() {
+  if (config_.BackgroundReportingEnabled()) {
+    CHECK(!thread_.has_value());
+
+    thread_.emplace(&MetricsReporter::BackgroundThreadRun, this);
+  }
+}
+
+void MetricsReporter::MaybeStopBackgroundThread() {
+  if (thread_.has_value()) {
+    messages_.SendMessage(ShutdownRequestedMessage{});
+    thread_->join();
+  }
+  // Do one final metrics report, if enabled.
+  if (config_.report_metrics_on_shutdown) {
+    ReportMetrics();
+  }
 }
 
-MetricsReporter::MetricsReporter(ReportingConfig config, const ArtMetrics* metrics)
-    : config_{std::move(config)}, metrics_{metrics} {}
+void MetricsReporter::BackgroundThreadRun() {
+  LOG_STREAM(DEBUG) << "Metrics reporting thread started";
+
+  // AttachCurrentThread is needed so we can safely use the ART concurrency primitives within the
+  // messages_ MessageQueue.
+  runtime_->AttachCurrentThread(kBackgroundThreadName,
+                                /*as_daemon=*/true,
+                                runtime_->GetSystemThreadGroup(),
+                                /*create_peer=*/true);
+  bool running = true;
+
+  MaybeResetTimeout();
+
+  while (running) {
+    messages_.SwitchReceive(
+        [&]([[maybe_unused]] ShutdownRequestedMessage message) {
+          LOG_STREAM(DEBUG) << "Shutdown request received";
+          running = false;
+        },
+        [&]([[maybe_unused]] TimeoutExpiredMessage message) {
+          LOG_STREAM(DEBUG) << "Timer expired, reporting metrics";
+
+          ReportMetrics();
+
+          MaybeResetTimeout();
+        });
+  }
+
+  runtime_->DetachCurrentThread();
+  LOG_STREAM(DEBUG) << "Metrics reporting thread terminating";
+}
+
+void MetricsReporter::MaybeResetTimeout() {
+  if (config_.periodic_report_seconds.has_value()) {
+    messages_.SetTimeout(SecondsToMs(config_.periodic_report_seconds.value()));
+  }
+}
 
-MetricsReporter::~MetricsReporter() {
-  // If we are configured to report metrics, do one final report at the end.
-  if (config_.ReportingEnabled()) {
+void MetricsReporter::ReportMetrics() const {
+  if (config_.dump_to_logcat) {
     LOG_STREAM(INFO) << "\n*** ART internal metrics ***\n\n";
     // LOG_STREAM(INFO) destroys the stream at the end of the statement, which makes it tricky pass
     // it to store as a field in StreamBackend. To get around this, we use an immediately-invoked
@@ -140,7 +196,7 @@ MetricsReporter::~MetricsReporter() {
     // dump the metrics.
     [this](std::ostream& os) {
       StreamBackend backend{os};
-      metrics_->ReportAllMetrics(&backend);
+      runtime_->GetMetrics()->ReportAllMetrics(&backend);
     }(LOG_STREAM(INFO));
     LOG_STREAM(INFO) << "\n*** Done dumping ART internal metrics ***\n";
   }
@@ -148,7 +204,7 @@ MetricsReporter::~MetricsReporter() {
     const auto& filename = config_.dump_to_file.value();
     std::ostringstream stream;
     StreamBackend backend{stream};
-    metrics_->ReportAllMetrics(&backend);
+    runtime_->GetMetrics()->ReportAllMetrics(&backend);
 
     std::string error_message;
     auto file{
@@ -168,6 +224,8 @@ ReportingConfig ReportingConfig::FromRuntimeArguments(const RuntimeArgumentMap&
   return {
       .dump_to_logcat = args.Exists(M::WriteMetricsToLog),
       .dump_to_file = args.GetOptional(M::WriteMetricsToFile),
+      .report_metrics_on_shutdown = !args.Exists(M::DisableFinalMetricsReport),
+      .periodic_report_seconds = args.GetOptional(M::MetricsReportingPeriod),
   };
 }
 
diff --git a/runtime/metrics/metrics.h b/runtime/metrics/metrics.h
index a6e395559a..36948dd3ac 100644
--- a/runtime/metrics/metrics.h
+++ b/runtime/metrics/metrics.h
@@ -24,9 +24,11 @@
 #include <optional>
 #include <ostream>
 #include <string_view>
+#include <thread>
 #include <vector>
 
 #include "android-base/logging.h"
+#include "base/message_queue.h"
 #include "base/time_utils.h"
 
 #pragma clang diagnostic push
@@ -344,6 +346,7 @@ class ArtMetrics {
 // Returns a human readable name for the given DatumId.
 std::string DatumName(DatumId datum);
 
+// Defines the set of options for how metrics reporting happens.
 struct ReportingConfig {
   static ReportingConfig FromRuntimeArguments(const RuntimeArgumentMap& args);
 
@@ -353,23 +356,60 @@ struct ReportingConfig {
   // If set, provides a file name to enable metrics logging to a file.
   std::optional<std::string> dump_to_file;
 
+  // Indicates whether to report the final state of metrics on shutdown.
+  //
+  // Note that reporting only happens if some output, such as logcat, is enabled.
+  bool report_metrics_on_shutdown{true};
+
+  // If set, metrics will be reported every time this many seconds elapses.
+  std::optional<unsigned int> periodic_report_seconds;
+
   // Returns whether any options are set that enables metrics reporting.
   constexpr bool ReportingEnabled() const { return dump_to_logcat || dump_to_file.has_value(); }
+
+  // Returns whether any options are set that requires a background reporting thread.
+  constexpr bool BackgroundReportingEnabled() const {
+    return ReportingEnabled() && periodic_report_seconds.has_value();
+  }
 };
 
 // MetricsReporter handles periodically reporting ART metrics.
 class MetricsReporter {
  public:
   // Creates a MetricsReporter instance that matches the options selected in ReportingConfig.
-  static std::unique_ptr<MetricsReporter> Create(ReportingConfig config, const ArtMetrics* metrics);
+  static std::unique_ptr<MetricsReporter> Create(ReportingConfig config, Runtime* runtime);
 
   ~MetricsReporter();
 
+  // Creates and runs the background reporting thread.
+  void MaybeStartBackgroundThread();
+
+  // Sends a request to the background thread to shutdown.
+  void MaybeStopBackgroundThread();
+
+  static constexpr const char* kBackgroundThreadName = "Metrics Background Reporting Thread";
+
  private:
-  explicit MetricsReporter(ReportingConfig config, const ArtMetrics* metrics);
+  MetricsReporter(ReportingConfig config, Runtime* runtime);
+
+  // The background reporting thread main loop.
+  void BackgroundThreadRun();
+
+  // Calls messages_.SetTimeout if needed.
+  void MaybeResetTimeout();
+
+  // Outputs the current state of the metrics to the destination set by config_.
+  void ReportMetrics() const;
+
+  const ReportingConfig config_;
+  Runtime* runtime_;
+
+  std::optional<std::thread> thread_;
+
+  // A message indicating that the reporting thread should shut down.
+  struct ShutdownRequestedMessage {};
 
-  ReportingConfig config_;
-  const ArtMetrics* metrics_;
+  MessageQueue<ShutdownRequestedMessage> messages_;
 };
 
 
diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc
index 5b8c9d3bc1..6dd121c443 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -401,6 +401,13 @@ std::unique_ptr<RuntimeParser> ParsedOptions::MakeParser(bool ignore_unrecognize
           .WithHelp("Enables writing ART metrics to the given file")
           .WithType<std::string>()
           .IntoKey(M::WriteMetricsToFile)
+      .Define("-Xdisable-final-metrics-report")
+          .WithHelp("Disables reporting metrics when ART shuts down")
+          .IntoKey(M::DisableFinalMetricsReport)
+      .Define("-Xmetrics-reporting-period=_")
+          .WithHelp("The time in seconds between metrics reports")
+          .WithType<unsigned int>()
+          .IntoKey(M::MetricsReportingPeriod)
       .Define("-Xonly-use-system-oat-files")
           .IntoKey(M::OnlyUseSystemOatFiles)
       .Define("-Xverifier-logging-threshold=_")
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 00d9ff1414..da586b34d1 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -440,6 +440,9 @@ Runtime::~Runtime() {
   delete signal_catcher_;
   signal_catcher_ = nullptr;
 
+  // Shutdown metrics reporting.
+  metrics_reporter_.reset();
+
   // Make sure all other non-daemon threads have terminated, and all daemon threads are suspended.
   // Also wait for daemon threads to quiesce, so that in addition to being "suspended", they
   // no longer access monitor and thread list data structures. We leak user daemon threads
@@ -1066,6 +1069,10 @@ void Runtime::InitNonZygoteOrPostFork(
   // before fork aren't attributed to an app.
   heap_->ResetGcPerformanceInfo();
 
+  if (metrics_reporter_ != nullptr) {
+    metrics_reporter_->MaybeStartBackgroundThread();
+  }
+
   StartSignalCatcher();
 
   ScopedObjectAccess soa(Thread::Current());
@@ -1815,7 +1822,7 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
 void Runtime::InitMetrics(const RuntimeArgumentMap& runtime_options) {
   auto metrics_config = metrics::ReportingConfig::FromRuntimeArguments(runtime_options);
   if (metrics_config.ReportingEnabled()) {
-    metrics_reporter_ = metrics::MetricsReporter::Create(metrics_config, GetMetrics());
+    metrics_reporter_ = metrics::MetricsReporter::Create(metrics_config, this);
   }
 }
 
diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def
index 80ec30833f..cda03a9897 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -186,5 +186,7 @@ RUNTIME_OPTIONS_KEY (bool,                PerfettoHprof,                  false)
 // Whether to dump ART metrics to logcat
 RUNTIME_OPTIONS_KEY (Unit,                WriteMetricsToLog)
 RUNTIME_OPTIONS_KEY (std::string,         WriteMetricsToFile)
+RUNTIME_OPTIONS_KEY (Unit,                DisableFinalMetricsReport)
+RUNTIME_OPTIONS_KEY (unsigned int,        MetricsReportingPeriod)
 
 #undef RUNTIME_OPTIONS_KEY