Revert^2 "[metrics] Add file output support"
Adds a new command line option, -Xwrite-metrics-to-file=_, which
specifies a file for writing metrics to. This can be used in conjunction
with -Xwrite-metrics-to-log, which will cause the output to go to both
logcat and the file.
Writing metrics to a file can be helpful for testing or benchmarking
environments.
Access to the file is protected using advisory locking, so it is safe to
have multiple processes write metrics to the same file.
This reverts commit ce2e0957f065002293ef152da28b6fa941c6baa3.
Reason for revert: Fixed test failures by adding Android.bp file.
Test: test/run-test --host test/2234-write-metrics-to-file
Bug: 175025360
Bug: 170149255
Change-Id: I6768f5b4920896fc0f8e5866c878af999f73012d
diff --git a/TEST_MAPPING b/TEST_MAPPING
index 65bed18..2d7ecba 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -382,6 +382,12 @@
"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 581bc23..7349bbc 100644
--- a/libartbase/base/variant_map.h
+++ b/libartbase/base/variant_map.h
@@ -229,6 +229,14 @@
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 81ae2b4..e85897b 100644
--- a/runtime/metrics/metrics.cc
+++ b/runtime/metrics/metrics.cc
@@ -16,12 +16,20 @@
#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 {
@@ -116,15 +124,15 @@
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{config, metrics}};
+ return std::unique_ptr<MetricsReporter>{new MetricsReporter{std::move(config), metrics}};
}
MetricsReporter::MetricsReporter(ReportingConfig config, const ArtMetrics* metrics)
- : config_{config}, metrics_{metrics} {}
+ : config_{std::move(config)}, metrics_{metrics} {}
MetricsReporter::~MetricsReporter() {
// If we are configured to report metrics, do one final report at the end.
- if (config_.dump_to_logcat) {
+ 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
@@ -136,6 +144,31 @@
}(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 7156d57..a6e39555 100644
--- a/runtime/metrics/metrics.h
+++ b/runtime/metrics/metrics.h
@@ -21,6 +21,7 @@
#include <array>
#include <atomic>
+#include <optional>
#include <ostream>
#include <string_view>
#include <vector>
@@ -58,6 +59,10 @@
// per metric.
namespace art {
+
+class Runtime;
+struct RuntimeArgumentMap;
+
namespace metrics {
/**
@@ -340,9 +345,16 @@
std::string DatumName(DatumId datum);
struct ReportingConfig {
- 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.
+ 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(); }
};
// MetricsReporter handles periodically reporting ART metrics.
diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc
index 508e697..5b8c9d3 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -395,7 +395,12 @@
.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 2012a04..00d9ff1 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -223,13 +223,6 @@
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()
@@ -1719,8 +1712,7 @@
// Class-roots are setup, we can now finish initializing the JniIdManager.
GetJniIdManager()->Init(self);
- metrics_reporter_ =
- metrics::MetricsReporter::Create(ParseMetricsReportingConfig(runtime_options), &metrics_);
+ InitMetrics(runtime_options);
// Runtime initialization is largely done now.
// We load plugins first since that can modify the runtime state slightly.
@@ -1820,6 +1812,13 @@
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 7fd731e..06f05a1 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -980,6 +980,7 @@
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 4462de2..80ec308 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -185,5 +185,6 @@
// 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
new file mode 100644
index 0000000..087c2e7
--- /dev/null
+++ b/test/2232-write-metrics-to-log/Android.bp
@@ -0,0 +1,31 @@
+// 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
new file mode 100644
index 0000000..1dfcca4
--- /dev/null
+++ b/test/2234-write-metrics-to-file/Android.bp
@@ -0,0 +1,31 @@
+// 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
new file mode 100755
index 0000000..16e6849
--- /dev/null
+++ b/test/2234-write-metrics-to-file/check
@@ -0,0 +1,34 @@
+#!/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
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/2234-write-metrics-to-file/expected-stderr.txt
diff --git a/test/2234-write-metrics-to-file/expected-stdout.txt b/test/2234-write-metrics-to-file/expected-stdout.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/2234-write-metrics-to-file/expected-stdout.txt
diff --git a/test/2234-write-metrics-to-file/info.txt b/test/2234-write-metrics-to-file/info.txt
new file mode 100644
index 0000000..d2e50df
--- /dev/null
+++ b/test/2234-write-metrics-to-file/info.txt
@@ -0,0 +1 @@
+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
new file mode 100755
index 0000000..45cb55b
--- /dev/null
+++ b/test/2234-write-metrics-to-file/run
@@ -0,0 +1,19 @@
+#!/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
new file mode 100644
index 0000000..f143017
--- /dev/null
+++ b/test/2234-write-metrics-to-file/src/Main.java
@@ -0,0 +1,20 @@
+/*
+ * 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 385db45..4c69762 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1351,7 +1351,8 @@
"description": ["Failing on RI. Needs further investigating."]
},
{
- "tests": ["2232-write-metrics-to-log"],
+ "tests": ["2232-write-metrics-to-log",
+ "2234-write-metrics-to-file"],
"variant": "jvm",
"description": ["RI does not support ART metrics."]
},
