Revert "Revert^2 "[metrics] Add file output support""

This reverts commit 8ef84f233a55972eb3a3d84c11c1617531af8e92.

Bug: 175025360
Bug: 170149255

Reason for revert: test failing on target

Change-Id: I8d82462f6fb853ece4a4b295de17ab13b1f1b6f1

17 files changed, 16 insertions, 218 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/variant_map.h b/libartbase/base/variant_map.h
index 7349bbc046..581bc234cc 100644
--- a/libartbase/base/variant_map.h
+++ b/libartbase/base/variant_map.h
@@ -229,14 +229,6 @@ struct VariantMap {
     return GetValuePtr(key);
   }
 
-  // Look up the value from the key and return the value wrapped in a std::optional. If it was not
-  // set in the map, return an empty std::optional.
-  template <typename TValue>
-  std::optional<TValue> GetOptional(const TKey<TValue>& key) const {
-    auto* ptr = Get(key);
-    return (ptr == nullptr) ? std::optional<TValue>{} : std::make_optional(*ptr);
-  }
-
   // Lookup the value from the key. If it was not set in the map, return the default value.
   // The default value is either the key's default, or TValue{} if the key doesn't have a default.
   template <typename TValue>
diff --git a/runtime/metrics/metrics.cc b/runtime/metrics/metrics.cc
index e85897b19e..81ae2b4dab 100644
--- a/runtime/metrics/metrics.cc
+++ b/runtime/metrics/metrics.cc
@@ -16,20 +16,12 @@
 
 #include "metrics.h"
 
-#include <sstream>
-
-#include "android-base/file.h"
 #include "android-base/logging.h"
 #include "base/macros.h"
-#include "base/scoped_flock.h"
-#include "runtime.h"
-#include "runtime_options.h"
 
 #pragma clang diagnostic push
 #pragma clang diagnostic error "-Wconversion"
 
-using android::base::WriteStringToFd;
-
 namespace art {
 namespace metrics {
 
@@ -124,15 +116,15 @@ std::unique_ptr<MetricsReporter> MetricsReporter::Create(ReportingConfig config,
   std::unique_ptr<MetricsBackend> backend;
 
   // 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{config, metrics}};
 }
 
 MetricsReporter::MetricsReporter(ReportingConfig config, const ArtMetrics* metrics)
-    : config_{std::move(config)}, metrics_{metrics} {}
+    : config_{config}, metrics_{metrics} {}
 
 MetricsReporter::~MetricsReporter() {
   // If we are configured to report metrics, do one final report at the end.
-  if (config_.ReportingEnabled()) {
+  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
@@ -144,31 +136,6 @@ MetricsReporter::~MetricsReporter() {
     }(LOG_STREAM(INFO));
     LOG_STREAM(INFO) << "\n*** Done dumping ART internal metrics ***\n";
   }
-  if (config_.dump_to_file.has_value()) {
-    const auto& filename = config_.dump_to_file.value();
-    std::ostringstream stream;
-    StreamBackend backend{stream};
-    metrics_->ReportAllMetrics(&backend);
-
-    std::string error_message;
-    auto file{
-        LockedFile::Open(filename.c_str(), O_CREAT | O_WRONLY | O_APPEND, true, &error_message)};
-    if (file.get() == nullptr) {
-      LOG(WARNING) << "Could open metrics file '" << filename << "': " << error_message;
-    } else {
-      if (!WriteStringToFd(stream.str(), file.get()->Fd())) {
-        PLOG(WARNING) << "Error writing metrics to file";
-      }
-    }
-  }
-}
-
-ReportingConfig ReportingConfig::FromRuntimeArguments(const RuntimeArgumentMap& args) {
-  using M = RuntimeArgumentMap;
-  return {
-      .dump_to_logcat = args.Exists(M::WriteMetricsToLog),
-      .dump_to_file = args.GetOptional(M::WriteMetricsToFile),
-  };
 }
 
 }  // namespace metrics
diff --git a/runtime/metrics/metrics.h b/runtime/metrics/metrics.h
index a6e395559a..7156d576e0 100644
--- a/runtime/metrics/metrics.h
+++ b/runtime/metrics/metrics.h
@@ -21,7 +21,6 @@
 
 #include <array>
 #include <atomic>
-#include <optional>
 #include <ostream>
 #include <string_view>
 #include <vector>
@@ -59,10 +58,6 @@
 // per metric.
 
 namespace art {
-
-class Runtime;
-struct RuntimeArgumentMap;
-
 namespace metrics {
 
 /**
@@ -345,16 +340,9 @@ class ArtMetrics {
 std::string DatumName(DatumId datum);
 
 struct ReportingConfig {
-  static ReportingConfig FromRuntimeArguments(const RuntimeArgumentMap& args);
-
-  // Causes metrics to be written to the log, which makes them show up in logcat.
-  bool dump_to_logcat{false};
-
-  // If set, provides a file name to enable metrics logging to a file.
-  std::optional<std::string> dump_to_file;
-
-  // Returns whether any options are set that enables metrics reporting.
-  constexpr bool ReportingEnabled() const { return dump_to_logcat || dump_to_file.has_value(); }
+  bool dump_to_logcat;
+  // TODO(eholk): this will grow to support other configurations, such as logging to a file, or
+  // statsd. There will also be options for reporting after a period of time, or at certain events.
 };
 
 // MetricsReporter handles periodically reporting ART metrics.
diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc
index 5b8c9d3bc1..508e697f7f 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -395,12 +395,7 @@ std::unique_ptr<RuntimeParser> ParsedOptions::MakeParser(bool ignore_unrecognize
       .Define("-Xuse-stderr-logger")
           .IntoKey(M::UseStderrLogger)
       .Define("-Xwrite-metrics-to-log")
-          .WithHelp("Enables writing ART metrics to logcat")
           .IntoKey(M::WriteMetricsToLog)
-      .Define("-Xwrite-metrics-to-file=_")
-          .WithHelp("Enables writing ART metrics to the given file")
-          .WithType<std::string>()
-          .IntoKey(M::WriteMetricsToFile)
       .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..2012a0492b 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -223,6 +223,13 @@ void CheckConstants() {
   CHECK_EQ(mirror::Array::kFirstElementOffset, mirror::Array::FirstElementOffset());
 }
 
+metrics::ReportingConfig ParseMetricsReportingConfig(const RuntimeArgumentMap& args) {
+  using M = RuntimeArgumentMap;
+  return {
+      .dump_to_logcat = args.Exists(M::WriteMetricsToLog),
+  };
+}
+
 }  // namespace
 
 Runtime::Runtime()
@@ -1712,7 +1719,8 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
   // Class-roots are setup, we can now finish initializing the JniIdManager.
   GetJniIdManager()->Init(self);
 
-  InitMetrics(runtime_options);
+  metrics_reporter_ =
+      metrics::MetricsReporter::Create(ParseMetricsReportingConfig(runtime_options), &metrics_);
 
   // Runtime initialization is largely done now.
   // We load plugins first since that can modify the runtime state slightly.
@@ -1812,13 +1820,6 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
   return true;
 }
 
-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());
-  }
-}
-
 bool Runtime::EnsurePluginLoaded(const char* plugin_name, std::string* error_msg) {
   // Is the plugin already loaded?
   for (const Plugin& p : plugins_) {
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 06f05a177e..7fd731eb0d 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -980,7 +980,6 @@ class Runtime {
       SHARED_TRYLOCK_FUNCTION(true, Locks::mutator_lock_);
   void InitNativeMethods() REQUIRES(!Locks::mutator_lock_);
   void RegisterRuntimeNativeMethods(JNIEnv* env);
-  void InitMetrics(const RuntimeArgumentMap& runtime_options);
 
   void StartDaemonThreads();
   void StartSignalCatcher();
diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def
index 80ec30833f..4462de22f5 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -185,6 +185,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)
 
 #undef RUNTIME_OPTIONS_KEY
diff --git a/test/2232-write-metrics-to-log/Android.bp b/test/2232-write-metrics-to-log/Android.bp
deleted file mode 100644
index 087c2e79c1..0000000000
--- a/test/2232-write-metrics-to-log/Android.bp
+++ /dev/null
@@ -1,31 +0,0 @@
-// Generated by `regen-test-files`. Do not edit manually.
-
-// Build rules for ART run-test `2232-write-metrics-to-log`.
-
-// Test's Dex code.
-java_test {
-    name: "art-run-test-2232-write-metrics-to-log",
-    defaults: ["art-run-test-defaults"],
-    test_config_template: ":art-run-test-target-template",
-    srcs: ["src/**/*.java"],
-    data: [
-        ":art-run-test-2232-write-metrics-to-log-expected-stdout",
-        ":art-run-test-2232-write-metrics-to-log-expected-stderr",
-    ],
-}
-
-// Test's expected standard output.
-genrule {
-    name: "art-run-test-2232-write-metrics-to-log-expected-stdout",
-    out: ["art-run-test-2232-write-metrics-to-log-expected-stdout.txt"],
-    srcs: ["expected-stdout.txt"],
-    cmd: "cp -f $(in) $(out)",
-}
-
-// Test's expected standard error.
-genrule {
-    name: "art-run-test-2232-write-metrics-to-log-expected-stderr",
-    out: ["art-run-test-2232-write-metrics-to-log-expected-stderr.txt"],
-    srcs: ["expected-stderr.txt"],
-    cmd: "cp -f $(in) $(out)",
-}
diff --git a/test/2234-write-metrics-to-file/Android.bp b/test/2234-write-metrics-to-file/Android.bp
deleted file mode 100644
index 1dfcca4c14..0000000000
--- a/test/2234-write-metrics-to-file/Android.bp
+++ /dev/null
@@ -1,31 +0,0 @@
-// Generated by `regen-test-files`. Do not edit manually.
-
-// Build rules for ART run-test `2234-write-metrics-to-file`.
-
-// Test's Dex code.
-java_test {
-    name: "art-run-test-2234-write-metrics-to-file",
-    defaults: ["art-run-test-defaults"],
-    test_config_template: ":art-run-test-target-template",
-    srcs: ["src/**/*.java"],
-    data: [
-        ":art-run-test-2234-write-metrics-to-file-expected-stdout",
-        ":art-run-test-2234-write-metrics-to-file-expected-stderr",
-    ],
-}
-
-// Test's expected standard output.
-genrule {
-    name: "art-run-test-2234-write-metrics-to-file-expected-stdout",
-    out: ["art-run-test-2234-write-metrics-to-file-expected-stdout.txt"],
-    srcs: ["expected-stdout.txt"],
-    cmd: "cp -f $(in) $(out)",
-}
-
-// Test's expected standard error.
-genrule {
-    name: "art-run-test-2234-write-metrics-to-file-expected-stderr",
-    out: ["art-run-test-2234-write-metrics-to-file-expected-stderr.txt"],
-    srcs: ["expected-stderr.txt"],
-    cmd: "cp -f $(in) $(out)",
-}
diff --git a/test/2234-write-metrics-to-file/check b/test/2234-write-metrics-to-file/check
deleted file mode 100755
index 16e6849b6c..0000000000
--- a/test/2234-write-metrics-to-file/check
+++ /dev/null
@@ -1,34 +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 'ClassVerificationTotalTime' "$2" >/dev/null
-MSG_FOUND=$?
-
-if [[ $MSG_FOUND -ne 0 ]] ; then
-  # Print out the log and return with error.
-  cat "$2"
-  exit 1
-fi
-
-# Success.
-exit 0
diff --git a/test/2234-write-metrics-to-file/expected-stderr.txt b/test/2234-write-metrics-to-file/expected-stderr.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/2234-write-metrics-to-file/expected-stderr.txt
+++ /dev/null
diff --git a/test/2234-write-metrics-to-file/expected-stdout.txt b/test/2234-write-metrics-to-file/expected-stdout.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/2234-write-metrics-to-file/expected-stdout.txt
+++ /dev/null
diff --git a/test/2234-write-metrics-to-file/info.txt b/test/2234-write-metrics-to-file/info.txt
deleted file mode 100644
index d2e50dfde0..0000000000
--- a/test/2234-write-metrics-to-file/info.txt
+++ /dev/null
@@ -1 +0,0 @@
-Checks that metrics can be written to a file.
diff --git a/test/2234-write-metrics-to-file/run b/test/2234-write-metrics-to-file/run
deleted file mode 100755
index 45cb55bf3d..0000000000
--- a/test/2234-write-metrics-to-file/run
+++ /dev/null
@@ -1,19 +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.
-
-# Use /dev/stdout as the file so that buildbot can automatically grab the output for the check
-# script.
-exec ${RUN} $@ --runtime-option -Xwrite-metrics-to-file=/dev/stdout
diff --git a/test/2234-write-metrics-to-file/src/Main.java b/test/2234-write-metrics-to-file/src/Main.java
deleted file mode 100644
index f143017f08..0000000000
--- a/test/2234-write-metrics-to-file/src/Main.java
+++ /dev/null
@@ -1,20 +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) {
-  }
-}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 4c6976258b..385db458a9 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1351,8 +1351,7 @@
         "description": ["Failing on RI. Needs further investigating."]
     },
     {
-        "tests": ["2232-write-metrics-to-log",
-                  "2234-write-metrics-to-file"],
+        "tests": ["2232-write-metrics-to-log"],
         "variant": "jvm",
         "description": ["RI does not support ART metrics."]
     },