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

This reverts commit 4c176b9de6c430422395017910633bcb001b2e84.

Reason for revert: Test fails on target.

Change-Id: Idfef53679cf602c7c10a9cc0ffb16fda583ed78f

16 files changed, 23 insertions, 269 deletions

diff --git a/TEST_MAPPING b/TEST_MAPPING
index 2d7ecba748..65bed187dc 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -382,12 +382,6 @@
       "name": "art-run-test-2030-long-running-child"
     },
     {
-      "name": "art-run-test-2232-write-metrics-to-log"
-    },
-    {
-      "name": "art-run-test-2234-write-metrics-to-file"
-    },
-    {
       "name": "art-run-test-300-package-override"
     },
     {
diff --git a/libartbase/base/time_utils.h b/libartbase/base/time_utils.h
index e1273a380a..cb0ab13ef9 100644
--- a/libartbase/base/time_utils.h
+++ b/libartbase/base/time_utils.h
@@ -87,10 +87,6 @@ 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 98a0b9da9b..1a5b227ffc 100644
--- a/openjdkjvmti/ti_thread.cc
+++ b/openjdkjvmti/ti_thread.cc
@@ -39,13 +39,12 @@
 #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"
@@ -124,12 +123,13 @@ 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, as well as the metrics background reporting thread.
+      // perfetto_hprof.
       if (art::kIsDebugBuild) {
         std::string name;
         self->GetThreadName(name);
-        if (name != "JDWP" && name != "Signal Catcher" && name != "perfetto_hprof_listener" &&
-            name != art::metrics::MetricsReporter::kBackgroundThreadName &&
+        if (name != "JDWP" &&
+            name != "Signal Catcher" &&
+            name != "perfetto_hprof_listener" &&
             !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 60add77d5a..e85897b19e 100644
--- a/runtime/metrics/metrics.cc
+++ b/runtime/metrics/metrics.cc
@@ -24,7 +24,6 @@
 #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"
@@ -120,75 +119,20 @@ void StreamBackend::ReportHistogram(DatumId histogram_type,
   }
 }
 
-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), 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();
-  }
-}
-
-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();
+std::unique_ptr<MetricsReporter> MetricsReporter::Create(ReportingConfig config,
+                                                         const ArtMetrics* metrics) {
+  std::unique_ptr<MetricsBackend> backend;
 
-          MaybeResetTimeout();
-        });
-  }
-
-  runtime_->DetachCurrentThread();
-  LOG_STREAM(DEBUG) << "Metrics reporting thread terminating";
+  // 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}};
 }
 
-void MetricsReporter::MaybeResetTimeout() {
-  if (config_.periodic_report_seconds.has_value()) {
-    messages_.SetTimeout(SecondsToMs(config_.periodic_report_seconds.value()));
-  }
-}
+MetricsReporter::MetricsReporter(ReportingConfig config, const ArtMetrics* metrics)
+    : config_{std::move(config)}, metrics_{metrics} {}
 
-void MetricsReporter::ReportMetrics() const {
-  if (config_.dump_to_logcat) {
+MetricsReporter::~MetricsReporter() {
+  // If we are configured to report metrics, do one final report at the end.
+  if (config_.ReportingEnabled()) {
     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
@@ -196,7 +140,7 @@ void MetricsReporter::ReportMetrics() const {
     // dump the metrics.
     [this](std::ostream& os) {
       StreamBackend backend{os};
-      runtime_->GetMetrics()->ReportAllMetrics(&backend);
+      metrics_->ReportAllMetrics(&backend);
     }(LOG_STREAM(INFO));
     LOG_STREAM(INFO) << "\n*** Done dumping ART internal metrics ***\n";
   }
@@ -204,7 +148,7 @@ void MetricsReporter::ReportMetrics() const {
     const auto& filename = config_.dump_to_file.value();
     std::ostringstream stream;
     StreamBackend backend{stream};
-    runtime_->GetMetrics()->ReportAllMetrics(&backend);
+    metrics_->ReportAllMetrics(&backend);
 
     std::string error_message;
     auto file{
@@ -224,8 +168,6 @@ 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 36948dd3ac..a6e395559a 100644
--- a/runtime/metrics/metrics.h
+++ b/runtime/metrics/metrics.h
@@ -24,11 +24,9 @@
 #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
@@ -346,7 +344,6 @@ 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);
 
@@ -356,60 +353,23 @@ 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, Runtime* runtime);
+  static std::unique_ptr<MetricsReporter> Create(ReportingConfig config, const ArtMetrics* metrics);
 
   ~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:
-  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 {};
+  explicit MetricsReporter(ReportingConfig config, const ArtMetrics* metrics);
 
-  MessageQueue<ShutdownRequestedMessage> messages_;
+  ReportingConfig config_;
+  const ArtMetrics* metrics_;
 };
 
 
diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc
index 6dd121c443..5b8c9d3bc1 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -401,13 +401,6 @@ 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 da586b34d1..00d9ff1414 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -440,9 +440,6 @@ 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
@@ -1069,10 +1066,6 @@ 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());
@@ -1822,7 +1815,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, this);
+    metrics_reporter_ = metrics::MetricsReporter::Create(metrics_config, GetMetrics());
   }
 }
 
diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def
index cda03a9897..80ec30833f 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -186,7 +186,5 @@ 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
diff --git a/test/2233-metrics-background-thread/Android.bp b/test/2233-metrics-background-thread/Android.bp
deleted file mode 100644
index 115bc8024e..0000000000
--- a/test/2233-metrics-background-thread/Android.bp
+++ /dev/null
@@ -1,31 +0,0 @@
-// Generated by `regen-test-files`. Do not edit manually.
-
-// Build rules for ART run-test `2233-metrics-background-thread`.
-
-// Test's Dex code.
-java_test {
-    name: "art-run-test-2233-metrics-background-thread",
-    defaults: ["art-run-test-defaults"],
-    test_config_template: ":art-run-test-target-template",
-    srcs: ["src/**/*.java"],
-    data: [
-        ":art-run-test-2233-metrics-background-thread-expected-stdout",
-        ":art-run-test-2233-metrics-background-thread-expected-stderr",
-    ],
-}
-
-// Test's expected standard output.
-genrule {
-    name: "art-run-test-2233-metrics-background-thread-expected-stdout",
-    out: ["art-run-test-2233-metrics-background-thread-expected-stdout.txt"],
-    srcs: ["expected-stdout.txt"],
-    cmd: "cp -f $(in) $(out)",
-}
-
-// Test's expected standard error.
-genrule {
-    name: "art-run-test-2233-metrics-background-thread-expected-stderr",
-    out: ["art-run-test-2233-metrics-background-thread-expected-stderr.txt"],
-    srcs: ["expected-stderr.txt"],
-    cmd: "cp -f $(in) $(out)",
-}
diff --git a/test/2233-metrics-background-thread/check b/test/2233-metrics-background-thread/check
deleted file mode 100755
index 3983a198d5..0000000000
--- a/test/2233-metrics-background-thread/check
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2021 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-# Inputs:
-# $1: Test's expected standard output
-# $2: Test's actual standard output
-# $3: Test's expected standard error
-# $4: Test's actual standard error
-
-# Check that one of the metrics appears in stderr.
-grep 'Metrics reporting thread started' "$4" >/dev/null
-THREAD_STARTED=$?
-
-grep 'Timer expired, reporting metrics' "$4" >/dev/null
-METRICS_REPORTED=$?
-
-if [[ $THREAD_START -ne 0 ]] ; then
-  # Print out the log and return with error.
-  echo Metrics reporting thread did not start.
-  cat "$4"
-  exit 1
-fi
-
-if [[ $METRICS_REPORTED -ne 0 ]] ; then
-  # Print out the log and return with error.
-  echo Did not detect metrics report.
-  cat "$4"
-  exit 1
-fi
-
-# Success.
-exit 0
diff --git a/test/2233-metrics-background-thread/expected-stderr.txt b/test/2233-metrics-background-thread/expected-stderr.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/2233-metrics-background-thread/expected-stderr.txt
+++ /dev/null
diff --git a/test/2233-metrics-background-thread/expected-stdout.txt b/test/2233-metrics-background-thread/expected-stdout.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/2233-metrics-background-thread/expected-stdout.txt
+++ /dev/null
diff --git a/test/2233-metrics-background-thread/info.txt b/test/2233-metrics-background-thread/info.txt
deleted file mode 100644
index c831b35f78..0000000000
--- a/test/2233-metrics-background-thread/info.txt
+++ /dev/null
@@ -1 +0,0 @@
-Tests metrics background reporting thread
diff --git a/test/2233-metrics-background-thread/run b/test/2233-metrics-background-thread/run
deleted file mode 100755
index 7042ecc10f..0000000000
--- a/test/2233-metrics-background-thread/run
+++ /dev/null
@@ -1,22 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2021 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-export ANDROID_LOG_TAGS="*:d"
-exec ${RUN} $@ \
-    --external-log-tags \
-    --runtime-option -Xwrite-metrics-to-log \
-    --runtime-option -Xmetrics-reporting-period=1 \
-    --runtime-option -Xdisable-final-metrics-report \
diff --git a/test/2233-metrics-background-thread/src/Main.java b/test/2233-metrics-background-thread/src/Main.java
deleted file mode 100644
index 5261e72651..0000000000
--- a/test/2233-metrics-background-thread/src/Main.java
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-public class Main {
-  public static void main(String[] args) throws InterruptedException {
-    // Sleep for 1.5 seconds to give the metrics reporting thread a chance to report.
-    Thread.sleep(1500);
-  }
-}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 643a31890e..6df3ae7952 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1353,8 +1353,7 @@
         "description": ["Failing on RI. Needs further investigating."]
     },
     {
-        "tests": ["2232-write-metrics-to-log",
-                  "2233-metrics-background-thread"],
+        "tests": ["2232-write-metrics-to-log"],
         "variant": "jvm",
         "description": ["RI does not support ART metrics."]
     },