Revert "[metrics] Add file output support"
This reverts commit 51a9283984b05511bd98f9bcbd1f2f18900239cc.
Reason for revert: The new test is broken on device.
Bug: 175025360
Bug: 170149255
Change-Id: I94d33f9e3d2203a0323fd5dc34f5a6f3e6147367
diff --git a/libartbase/base/variant_map.h b/libartbase/base/variant_map.h
index 7349bbc..581bc23 100644
--- a/libartbase/base/variant_map.h
+++ b/libartbase/base/variant_map.h
@@ -229,14 +229,6 @@
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 e85897b..81ae2b4 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<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 @@
}(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 a6e3955..7156d57 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 @@
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 5b8c9d3..508e697 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -395,12 +395,7 @@
.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 568d500..ac3c392 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -223,6 +223,13 @@
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()
@@ -1709,7 +1716,8 @@
// 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.
@@ -1809,13 +1817,6 @@
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 06f05a1..7fd731e 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -980,7 +980,6 @@
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 80ec308..4462de2 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -185,6 +185,5 @@
// 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/2234-write-metrics-to-file/check b/test/2234-write-metrics-to-file/check
deleted file mode 100755
index b904095..0000000
--- a/test/2234-write-metrics-to-file/check
+++ /dev/null
@@ -1,34 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2020 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 e69de29..0000000
--- 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 e69de29..0000000
--- 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 d2e50df..0000000
--- 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 d2ff292..0000000
--- a/test/2234-write-metrics-to-file/run
+++ /dev/null
@@ -1,19 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2020 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 0426192..0000000
--- a/test/2234-write-metrics-to-file/src/Main.java
+++ /dev/null
@@ -1,20 +0,0 @@
-/*
- * Copyright (C) 2020 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 4c69762..385db45 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."]
},