summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Eric Holk <eholk@google.com> 2021-01-12 00:04:57 +0000
committer Eric Holk <eholk@google.com> 2021-01-12 17:16:13 +0000
commit8ef84f233a55972eb3a3d84c11c1617531af8e92 (patch)
treecb7113879d10fcafd3b3c2d969e75fba7c60dc35
parent46d6fc0e16d65468aa259fadf27ddfeef72987b7 (diff)
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
-rw-r--r--TEST_MAPPING6
-rw-r--r--libartbase/base/variant_map.h8
-rw-r--r--runtime/metrics/metrics.cc39
-rw-r--r--runtime/metrics/metrics.h18
-rw-r--r--runtime/parsed_options.cc5
-rw-r--r--runtime/runtime.cc17
-rw-r--r--runtime/runtime.h1
-rw-r--r--runtime/runtime_options.def1
-rw-r--r--test/2232-write-metrics-to-log/Android.bp31
-rw-r--r--test/2234-write-metrics-to-file/Android.bp31
-rwxr-xr-xtest/2234-write-metrics-to-file/check34
-rw-r--r--test/2234-write-metrics-to-file/expected-stderr.txt0
-rw-r--r--test/2234-write-metrics-to-file/expected-stdout.txt0
-rw-r--r--test/2234-write-metrics-to-file/info.txt1
-rwxr-xr-xtest/2234-write-metrics-to-file/run19
-rw-r--r--test/2234-write-metrics-to-file/src/Main.java20
-rw-r--r--test/knownfailures.json3
17 files changed, 218 insertions, 16 deletions
diff --git a/TEST_MAPPING b/TEST_MAPPING
index 65bed187dc..2d7ecba748 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 581bc234cc..7349bbc046 100644
--- a/libartbase/base/variant_map.h
+++ b/libartbase/base/variant_map.h
@@ -229,6 +229,14 @@ 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 81ae2b4dab..e85897b19e 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<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{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 @@ 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 7156d576e0..a6e395559a 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 @@ class ArtMetrics {
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 508e697f7f..5b8c9d3bc1 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -395,7 +395,12 @@ 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 2012a0492b..00d9ff1414 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -223,13 +223,6 @@ 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()
@@ -1719,8 +1712,7 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
// 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 @@ 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 7fd731eb0d..06f05a177e 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -980,6 +980,7 @@ 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 4462de22f5..80ec30833f 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -185,5 +185,6 @@ 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
new file mode 100644
index 0000000000..087c2e79c1
--- /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 0000000000..1dfcca4c14
--- /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 0000000000..16e6849b6c
--- /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 0000000000..e69de29bb2
--- /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 0000000000..e69de29bb2
--- /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 0000000000..d2e50dfde0
--- /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 0000000000..45cb55bf3d
--- /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 0000000000..f143017f08
--- /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 385db458a9..4c6976258b 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."]
},