Remove the histogram flag and all Chart JSON code.
Since the flag is now on by default, we can remove it (after all
callers stop passing it).
We can also remove all Chart JSON code from WebRTC since it is
no longer used.
Requires one recipe CL and one downstream CL to land first.
Bug: chromium:1029452
Change-Id: Ic1d62e8ab9dfcd255cd2bf51d153db80d59c564b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171878
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Patrik Höglund <phoglund@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30927}
diff --git a/audio/test/low_bandwidth_audio_test.py b/audio/test/low_bandwidth_audio_test.py
index 6a95903..51273f7 100755
--- a/audio/test/low_bandwidth_audio_test.py
+++ b/audio/test/low_bandwidth_audio_test.py
@@ -286,7 +286,6 @@
_LogCommand(test_command + [
'--sample_rate_hz=%d' % analyzer.sample_rate_hz,
'--test_case_prefix=%s' % analyzer.name,
- '--write_histogram_proto_json'
] + args.extra_test_args),
stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
perf_results_file = None
diff --git a/test/BUILD.gn b/test/BUILD.gn
index d84a78b..29d5d6d 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -226,8 +226,6 @@
"testsupport/perf_result_reporter.h",
"testsupport/perf_test.cc",
"testsupport/perf_test.h",
- "testsupport/perf_test_graphjson_writer.cc",
- "testsupport/perf_test_graphjson_writer.h",
"testsupport/perf_test_histogram_writer.h",
"testsupport/perf_test_result_writer.h",
]
diff --git a/test/test_main_lib.cc b/test/test_main_lib.cc
index f954a5e..15318b4 100644
--- a/test/test_main_lib.cc
+++ b/test/test_main_lib.cc
@@ -175,10 +175,10 @@
#else
int exit_code = RUN_ALL_TESTS();
- std::string chartjson_result_file =
+ std::string perf_output_file =
absl::GetFlag(FLAGS_isolated_script_test_perf_output);
- if (!chartjson_result_file.empty()) {
- if (!webrtc::test::WritePerfResults(chartjson_result_file)) {
+ if (!perf_output_file.empty()) {
+ if (!webrtc::test::WritePerfResults(perf_output_file)) {
return 1;
}
}
diff --git a/test/testsupport/perf_test.cc b/test/testsupport/perf_test.cc
index 5b2f7a0..310c7e3 100644
--- a/test/testsupport/perf_test.cc
+++ b/test/testsupport/perf_test.cc
@@ -19,22 +19,26 @@
#include "rtc_base/checks.h"
#include "rtc_base/critical_section.h"
-#include "test/testsupport/perf_test_graphjson_writer.h"
#include "test/testsupport/perf_test_histogram_writer.h"
-ABSL_FLAG(bool,
- write_histogram_proto_json,
- true,
- "Use the histogram C++ API, which will write Histogram protos "
- "instead of Chart JSON. See histogram.proto in third_party/catapult. "
- "This flag only has effect if --isolated_script_test_perf_output is "
- "specified");
-
namespace webrtc {
namespace test {
namespace {
+std::string UnitWithDirection(
+ const std::string& units,
+ webrtc::test::ImproveDirection improve_direction) {
+ switch (improve_direction) {
+ case webrtc::test::ImproveDirection::kNone:
+ return units;
+ case webrtc::test::ImproveDirection::kSmallerIsBetter:
+ return units + "_smallerIsBetter";
+ case webrtc::test::ImproveDirection::kBiggerIsBetter:
+ return units + "_biggerIsBetter";
+ }
+}
+
template <typename Container>
void OutputListToStream(std::ostream* ostream, const Container& values) {
const char* sep = "";
@@ -192,13 +196,8 @@
}
PerfTestResultWriter& GetPerfWriter() {
- if (absl::GetFlag(FLAGS_write_histogram_proto_json)) {
- static PerfTestResultWriter* writer = CreateHistogramWriter();
- return *writer;
- } else {
- static PerfTestResultWriter* writer = CreateGraphJsonWriter();
- return *writer;
- }
+ static PerfTestResultWriter* writer = CreateHistogramWriter();
+ return *writer;
}
} // namespace
@@ -222,12 +221,7 @@
bool WritePerfResults(const std::string& output_path) {
std::string results = GetPerfResults();
- FILE* output;
- if (absl::GetFlag(FLAGS_write_histogram_proto_json)) {
- output = fopen(output_path.c_str(), "wb");
- } else {
- output = fopen(output_path.c_str(), "w");
- }
+ FILE* output = fopen(output_path.c_str(), "wb");
if (output == NULL) {
printf("Failed to write to %s.\n", output_path.c_str());
return false;
diff --git a/test/testsupport/perf_test.h b/test/testsupport/perf_test.h
index 338a144..b0a5607 100644
--- a/test/testsupport/perf_test.h
+++ b/test/testsupport/perf_test.h
@@ -91,12 +91,11 @@
const bool important,
ImproveDirection improve_direction = ImproveDirection::kNone);
-// If --write_histogram_proto_json=false, this returns all perf results to date
-// in a JSON string formatted as described in dashboard/docs/data-format.md
-// in https://github.com/catapult-project/catapult/blob/master/. If
-// --write_histogram_proto_json=true, returns a string-encoded proto as
-// described in tracing/tracing/proto/histogram.proto in
+// Returns a string-encoded proto as described in
+// tracing/tracing/proto/histogram.proto in
// https://github.com/catapult-project/catapult/blob/master/.
+// If you want to print the proto in human readable format, use
+// tracing/bin/proto2json from third_party/catapult in your WebRTC checkout.
std::string GetPerfResults();
// Print into stdout plottable metrics for further post processing.
@@ -122,7 +121,4 @@
} // namespace test
} // namespace webrtc
-// Only for use by tests.
-ABSL_DECLARE_FLAG(bool, write_histogram_proto_json);
-
#endif // TEST_TESTSUPPORT_PERF_TEST_H_
diff --git a/test/testsupport/perf_test_graphjson_writer.cc b/test/testsupport/perf_test_graphjson_writer.cc
deleted file mode 100644
index cf49b29..0000000
--- a/test/testsupport/perf_test_graphjson_writer.cc
+++ /dev/null
@@ -1,144 +0,0 @@
-/*
- * Copyright (c) 2020 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-
-#include "test/testsupport/perf_test_graphjson_writer.h"
-
-#include <fstream>
-#include <map>
-#include <sstream>
-#include <string>
-#include <vector>
-
-#include "rtc_base/checks.h"
-#include "rtc_base/critical_section.h"
-
-namespace webrtc {
-namespace test {
-
-std::string UnitWithDirection(
- const std::string& units,
- webrtc::test::ImproveDirection improve_direction) {
- switch (improve_direction) {
- case webrtc::test::ImproveDirection::kNone:
- return units;
- case webrtc::test::ImproveDirection::kSmallerIsBetter:
- return units + "_smallerIsBetter";
- case webrtc::test::ImproveDirection::kBiggerIsBetter:
- return units + "_biggerIsBetter";
- }
-}
-
-template <typename Container>
-void OutputListToStream(std::ostream* ostream, const Container& values) {
- const char* sep = "";
- for (const auto& v : values) {
- (*ostream) << sep << v;
- sep = ",";
- }
-}
-
-namespace {
-
-class PerfTestGraphJsonWriter : public PerfTestResultWriter {
- public:
- PerfTestGraphJsonWriter() : crit_(), graphs_() {}
- void ClearResults() {
- rtc::CritScope lock(&crit_);
- graphs_.clear();
- }
-
- void LogResult(const std::string& graph_name,
- const std::string& trace_name,
- const double value,
- const std::string& units,
- const bool important,
- webrtc::test::ImproveDirection improve_direction) {
- std::ostringstream json_stream;
- json_stream << '"' << trace_name << R"(":{)";
- json_stream << R"("type":"scalar",)";
- json_stream << R"("value":)" << value << ',';
- json_stream << R"("units":")" << UnitWithDirection(units, improve_direction)
- << R"("})";
- rtc::CritScope lock(&crit_);
- graphs_[graph_name].push_back(json_stream.str());
- }
-
- void LogResultMeanAndError(const std::string& graph_name,
- const std::string& trace_name,
- const double mean,
- const double error,
- const std::string& units,
- const bool important,
- webrtc::test::ImproveDirection improve_direction) {
- std::ostringstream json_stream;
- json_stream << '"' << trace_name << R"(":{)";
- json_stream << R"("type":"list_of_scalar_values",)";
- json_stream << R"("values":[)" << mean << "],";
- json_stream << R"("std":)" << error << ',';
- json_stream << R"("units":")" << UnitWithDirection(units, improve_direction)
- << R"("})";
- rtc::CritScope lock(&crit_);
- graphs_[graph_name].push_back(json_stream.str());
- }
-
- void LogResultList(const std::string& graph_name,
- const std::string& trace_name,
- const rtc::ArrayView<const double> values,
- const std::string& units,
- const bool important,
- webrtc::test::ImproveDirection improve_direction) {
- std::ostringstream value_stream;
- value_stream.precision(8);
- value_stream << '[';
- OutputListToStream(&value_stream, values);
- value_stream << ']';
-
- std::ostringstream json_stream;
- json_stream << '"' << trace_name << R"(":{)";
- json_stream << R"("type":"list_of_scalar_values",)";
- json_stream << R"("values":)" << value_stream.str() << ',';
- json_stream << R"("units":")" << UnitWithDirection(units, improve_direction)
- << R"("})";
- rtc::CritScope lock(&crit_);
- graphs_[graph_name].push_back(json_stream.str());
- }
-
- std::string Serialize() const {
- std::ostringstream json_stream;
- json_stream << R"({"format_version":"1.0",)";
- json_stream << R"("charts":{)";
- rtc::CritScope lock(&crit_);
- for (auto graphs_it = graphs_.begin(); graphs_it != graphs_.end();
- ++graphs_it) {
- if (graphs_it != graphs_.begin())
- json_stream << ',';
- json_stream << '"' << graphs_it->first << "\":";
- json_stream << '{';
- OutputListToStream(&json_stream, graphs_it->second);
- json_stream << '}';
- }
- json_stream << "}}";
- return json_stream.str();
- }
-
- private:
- rtc::CriticalSection crit_;
- std::map<std::string, std::vector<std::string>> graphs_
- RTC_GUARDED_BY(&crit_);
-};
-
-} // namespace
-
-PerfTestResultWriter* CreateGraphJsonWriter() {
- return new PerfTestGraphJsonWriter();
-}
-
-} // namespace test
-} // namespace webrtc
diff --git a/test/testsupport/perf_test_graphjson_writer.h b/test/testsupport/perf_test_graphjson_writer.h
deleted file mode 100644
index ae32cfa..0000000
--- a/test/testsupport/perf_test_graphjson_writer.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (c) 2020 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-
-#ifndef TEST_TESTSUPPORT_PERF_TEST_GRAPHJSON_WRITER_H_
-#define TEST_TESTSUPPORT_PERF_TEST_GRAPHJSON_WRITER_H_
-
-#include <string>
-
-#include "test/testsupport/perf_test.h"
-#include "test/testsupport/perf_test_result_writer.h"
-
-namespace webrtc {
-namespace test {
-
-PerfTestResultWriter* CreateGraphJsonWriter();
-
-// Utilities that happen to be useful to perf_test.cc. Just move these back
-// to perf_test.cc when this file goes away.
-std::string UnitWithDirection(const std::string& units,
- webrtc::test::ImproveDirection improve_direction);
-
-} // namespace test
-} // namespace webrtc
-
-#endif // TEST_TESTSUPPORT_PERF_TEST_GRAPHJSON_WRITER_H_
diff --git a/test/testsupport/perf_test_unittest.cc b/test/testsupport/perf_test_unittest.cc
index d99014b..b779f4c 100644
--- a/test/testsupport/perf_test_unittest.cc
+++ b/test/testsupport/perf_test_unittest.cc
@@ -22,47 +22,6 @@
namespace proto = catapult::tracing::tracing::proto;
#endif
-namespace {
-
-const char* kJsonExpected = R"({
- "format_version":"1.0",
- "charts":{
- "foobar":{
- "baz_v":{
- "type":"scalar",
- "value":7,
- "units":"widgets"
- },
- "baz_me":{
- "type":"list_of_scalar_values",
- "values":[1],
- "std":2,
- "units":"lemurs"
- },
- "baz_vl":{
- "type":"list_of_scalar_values",
- "values":[1,2,3],
- "units":"units"
- }
- },
- "measurementmodifier":{
- "trace":{
- "type":"scalar",
- "value":42,
- "units":"units"
- }
- }
- }
-})";
-
-std::string RemoveSpaces(std::string s) {
- s.erase(std::remove(s.begin(), s.end(), ' '), s.end());
- s.erase(std::remove(s.begin(), s.end(), '\n'), s.end());
- return s;
-}
-
-} // namespace
-
namespace webrtc {
namespace test {
@@ -96,21 +55,6 @@
EXPECT_EQ(expected, ::testing::internal::GetCapturedStdout());
}
-TEST_F(PerfTest, TestGetPerfResultsJSON) {
- bool original_flag = absl::GetFlag(FLAGS_write_histogram_proto_json);
- absl::SetFlag(&FLAGS_write_histogram_proto_json, false);
-
- PrintResult("measurement", "modifier", "trace", 42, "units", false);
- PrintResult("foo", "bar", "baz_v", 7, "widgets", true);
- PrintResultMeanAndError("foo", "bar", "baz_me", 1, 2, "lemurs", false);
- const double kListOfScalars[] = {1, 2, 3};
- PrintResultList("foo", "bar", "baz_vl", kListOfScalars, "units", false);
-
- EXPECT_EQ(RemoveSpaces(kJsonExpected), GetPerfResults());
-
- absl::SetFlag(&FLAGS_write_histogram_proto_json, original_flag);
-}
-
TEST_F(PerfTest, TestClearPerfResults) {
PrintResult("measurement", "modifier", "trace", 42, "units", false);
ClearPerfResults();
@@ -158,16 +102,6 @@
EXPECT_EQ(hist2.unit().unit(), proto::MS_BEST_FIT_FORMAT);
}
-TEST_F(PerfTest, TestClearPerfResultsHistograms) {
- bool original_flag = absl::GetFlag(FLAGS_write_histogram_proto_json);
- absl::SetFlag(&FLAGS_write_histogram_proto_json, true);
- PrintResult("measurement", "modifier", "trace", 42, "ms", false);
- ClearPerfResults();
- EXPECT_EQ("", GetPerfResults());
-
- absl::SetFlag(&FLAGS_write_histogram_proto_json, original_flag);
-}
-
#endif // WEBRTC_ENABLE_PROTOBUF
#if GTEST_HAS_DEATH_TEST
