From 2ea27968d35b9ce744c15130248cfe8f455aa7b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrik=20H=C3=B6glund?= Date: Mon, 13 Jan 2020 15:10:40 +0100 Subject: [PATCH] Extract an interface from the perf results logger. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new interface is called PerfTestResultWriter and is currently implemented by PerfResultsLogger (renamed PerfTestGraphJsonWriter). I plan to introduce a second implementation of the perf logger that uses the new Histogram C++ API. I add a flag that chooses between the two implementations so I can try it out (perhaps by setting up a second, limited run of webrtc_perf_tests on the perf bots that uses the new implementation). The histogram C++ implementation will come in the next patch. As a side effect, I disentangled the plottable counter printer from the perf result printer so it will work for both implementations. The only thing they had in common was that both wrote JSON anyway. See the bug for details on the new API. Bug: chromium:1029452 Change-Id: Icb21b25ced08ea73aeecd221e9d51f2adf3dab1b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165389 Reviewed-by: Artem Titov Commit-Queue: Patrik Höglund Cr-Commit-Position: refs/heads/master@{#30243} --- PRESUBMIT.py | 5 +- rtc_tools/BUILD.gn | 2 + test/BUILD.gn | 7 + test/testsupport/perf_test.cc | 139 ++++------------- .../testsupport/perf_test_graphjson_writer.cc | 144 ++++++++++++++++++ test/testsupport/perf_test_graphjson_writer.h | 32 ++++ .../testsupport/perf_test_histogram_writer.cc | 28 ++++ test/testsupport/perf_test_histogram_writer.h | 24 +++ test/testsupport/perf_test_result_writer.h | 56 +++++++ 9 files changed, 323 insertions(+), 114 deletions(-) create mode 100644 test/testsupport/perf_test_graphjson_writer.cc create mode 100644 test/testsupport/perf_test_graphjson_writer.h create mode 100644 test/testsupport/perf_test_histogram_writer.cc create mode 100644 test/testsupport/perf_test_histogram_writer.h create mode 100644 test/testsupport/perf_test_result_writer.h diff --git a/PRESUBMIT.py b/PRESUBMIT.py index c200609d7c..08e6024da0 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -502,7 +502,10 @@ def CheckNoStreamUsageIsAdded(input_api, output_api, is_test = any(file_path.endswith(x) for x in ['_test.cc', '_tests.cc', '_unittest.cc', '_unittests.cc']) - return file_path.startswith('examples') or is_test + return (file_path.startswith('examples') or + file_path.startswith('test') or + is_test) + for f in input_api.AffectedSourceFiles(file_filter): # Usage of stringstream is allowed under examples/ and in tests. diff --git a/rtc_tools/BUILD.gn b/rtc_tools/BUILD.gn index bd4d5ad4d5..8ca20afb35 100644 --- a/rtc_tools/BUILD.gn +++ b/rtc_tools/BUILD.gn @@ -80,6 +80,7 @@ rtc_library("video_file_writer") { } rtc_library("video_quality_analysis") { + testonly = true sources = [ "frame_analyzer/linear_least_squares.cc", "frame_analyzer/linear_least_squares.h", @@ -207,6 +208,7 @@ if (!build_with_chromium) { } rtc_library("reference_less_video_analysis_lib") { + testonly = true sources = [ "frame_analyzer/reference_less_video_analysis_lib.cc", "frame_analyzer/reference_less_video_analysis_lib.h", diff --git a/test/BUILD.gn b/test/BUILD.gn index 1176b6e417..77a202f3fb 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -222,15 +222,22 @@ rtc_library("field_trial") { rtc_library("perf_test") { visibility = [ "*" ] + testonly = true sources = [ "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.cc", + "testsupport/perf_test_histogram_writer.h", + "testsupport/perf_test_result_writer.h", ] deps = [ "../api:array_view", "../rtc_base:checks", "../rtc_base:criticalsection", "../rtc_base:rtc_numerics", + "//third_party/abseil-cpp/absl/flags:flag", ] } diff --git a/test/testsupport/perf_test.cc b/test/testsupport/perf_test.cc index 17aca7ec44..eedb0c8062 100644 --- a/test/testsupport/perf_test.cc +++ b/test/testsupport/perf_test.cc @@ -12,15 +12,24 @@ #include -#include #include -#include #include #include #include +#include "absl/flags/flag.h" #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, + false, + "Use the histogram C++ API, which will write Histogram proto JSON " + "instead of Chart JSON. Note, Histogram set JSON and Histogram " + "proto JSON are not quite the same thing. This flag only has effect " + "if --isolated_script_test_perf_output is specified."); namespace webrtc { namespace test { @@ -36,19 +45,6 @@ void OutputListToStream(std::ostream* ostream, const Container& values) { } } -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"; - } -} - struct PlottableCounter { std::string graph_name; std::string trace_name; @@ -196,102 +192,20 @@ ResultsLinePrinter& GetResultsLinePrinter() { return *printer_; } -class PerfResultsLogger { - public: - PerfResultsLogger() : crit_(), graphs_() {} - void ClearResults() { - rtc::CritScope lock(&crit_); - graphs_.clear(); +PerfTestResultWriter& GetPerfWriter() { + if (absl::GetFlag(FLAGS_write_histogram_proto_json)) { + static PerfTestResultWriter* writer = CreateHistogramWriter(); + return *writer; + } else { + static PerfTestResultWriter* writer = CreateGraphJsonWriter(); + return *writer; } - - 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 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 ToJSON() const; - - private: - rtc::CriticalSection crit_; - std::map> graphs_ - RTC_GUARDED_BY(&crit_); -}; - -std::string PerfResultsLogger::ToJSON() 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(); -} - -PerfResultsLogger& GetPerfResultsLogger() { - static PerfResultsLogger* const logger_ = new PerfResultsLogger(); - return *logger_; } } // namespace void ClearPerfResults() { - GetPerfResultsLogger().ClearResults(); + GetPerfWriter().ClearResults(); } void SetPerfResultsOutput(FILE* output) { @@ -300,7 +214,7 @@ void SetPerfResultsOutput(FILE* output) { } std::string GetPerfResultsJSON() { - return GetPerfResultsLogger().ToJSON(); + return GetPerfWriter().ToJSON(); } void PrintPlottableResults(const std::vector& desired_graphs) { @@ -325,9 +239,8 @@ void PrintResult(const std::string& measurement, RTC_CHECK(std::isfinite(value)) << "Expected finite value for graph " << graph_name << ", trace name " << trace << ", units " << units << ", got " << value; - - GetPerfResultsLogger().LogResult(graph_name, trace, value, units, important, - improve_direction); + GetPerfWriter().LogResult(graph_name, trace, value, units, important, + improve_direction); GetResultsLinePrinter().PrintResult(graph_name, trace, value, units, important, improve_direction); } @@ -360,8 +273,8 @@ void PrintResultMeanAndError(const std::string& measurement, RTC_CHECK(std::isfinite(error)); std::string graph_name = measurement + modifier; - GetPerfResultsLogger().LogResultMeanAndError( - graph_name, trace, mean, error, units, important, improve_direction); + GetPerfWriter().LogResultMeanAndError(graph_name, trace, mean, error, units, + important, improve_direction); GetResultsLinePrinter().PrintResultMeanAndError( graph_name, trace, mean, error, units, important, improve_direction); } @@ -378,8 +291,8 @@ void PrintResultList(const std::string& measurement, } std::string graph_name = measurement + modifier; - GetPerfResultsLogger().LogResultList(graph_name, trace, values, units, - important, improve_direction); + GetPerfWriter().LogResultList(graph_name, trace, values, units, important, + improve_direction); GetResultsLinePrinter().PrintResultList(graph_name, trace, values, units, important, improve_direction); } diff --git a/test/testsupport/perf_test_graphjson_writer.cc b/test/testsupport/perf_test_graphjson_writer.cc new file mode 100644 index 0000000000..5a8ee64709 --- /dev/null +++ b/test/testsupport/perf_test_graphjson_writer.cc @@ -0,0 +1,144 @@ +/* + * 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 +#include +#include +#include +#include + +#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 +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 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 ToJSON() 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> 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 new file mode 100644 index 0000000000..ae32cfa9e0 --- /dev/null +++ b/test/testsupport/perf_test_graphjson_writer.h @@ -0,0 +1,32 @@ +/* + * 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 + +#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_histogram_writer.cc b/test/testsupport/perf_test_histogram_writer.cc new file mode 100644 index 0000000000..d82294bc68 --- /dev/null +++ b/test/testsupport/perf_test_histogram_writer.cc @@ -0,0 +1,28 @@ +/* + * 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_histogram_writer.h" + +#include + +#include + +namespace webrtc { +namespace test { + +namespace {} // namespace + +PerfTestResultWriter* CreateHistogramWriter() { + RTC_CHECK(false) << "Not implemented"; + return nullptr; +} + +} // namespace test +} // namespace webrtc diff --git a/test/testsupport/perf_test_histogram_writer.h b/test/testsupport/perf_test_histogram_writer.h new file mode 100644 index 0000000000..244e69fc45 --- /dev/null +++ b/test/testsupport/perf_test_histogram_writer.h @@ -0,0 +1,24 @@ +/* + * 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_HISTOGRAM_WRITER_H_ +#define TEST_TESTSUPPORT_PERF_TEST_HISTOGRAM_WRITER_H_ + +#include "test/testsupport/perf_test_result_writer.h" + +namespace webrtc { +namespace test { + +PerfTestResultWriter* CreateHistogramWriter(); + +} // namespace test +} // namespace webrtc + +#endif // TEST_TESTSUPPORT_PERF_TEST_HISTOGRAM_WRITER_H_ diff --git a/test/testsupport/perf_test_result_writer.h b/test/testsupport/perf_test_result_writer.h new file mode 100644 index 0000000000..5e932ba51c --- /dev/null +++ b/test/testsupport/perf_test_result_writer.h @@ -0,0 +1,56 @@ +/* + * 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_RESULT_WRITER_H_ +#define TEST_TESTSUPPORT_PERF_TEST_RESULT_WRITER_H_ + +#include +#include + +#include "test/testsupport/perf_test.h" + +namespace webrtc { +namespace test { + +// Interface for classes that write perf results to some kind of JSON format. +class PerfTestResultWriter { + public: + virtual ~PerfTestResultWriter() = default; + + virtual void ClearResults() = 0; + virtual 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) = 0; + virtual 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) = 0; + virtual void LogResultList( + const std::string& graph_name, + const std::string& trace_name, + const rtc::ArrayView values, + const std::string& units, + const bool important, + webrtc::test::ImproveDirection improve_direction) = 0; + + virtual std::string ToJSON() const = 0; +}; + +} // namespace test +} // namespace webrtc + +#endif // TEST_TESTSUPPORT_PERF_TEST_RESULT_WRITER_H_