diff --git a/test/BUILD.gn b/test/BUILD.gn index f75c3eb865..d84a78b15c 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -222,6 +222,8 @@ rtc_library("perf_test") { visibility = [ "*" ] testonly = true sources = [ + "testsupport/perf_result_reporter.cc", + "testsupport/perf_result_reporter.h", "testsupport/perf_test.cc", "testsupport/perf_test.h", "testsupport/perf_test_graphjson_writer.cc", @@ -236,6 +238,7 @@ rtc_library("perf_test") { "../rtc_base:logging", "../rtc_base:rtc_numerics", "//third_party/abseil-cpp/absl/flags:flag", + "//third_party/abseil-cpp/absl/types:optional", ] if (rtc_enable_protobuf) { sources += [ "testsupport/perf_test_histogram_writer.cc" ] diff --git a/test/testsupport/perf_result_reporter.cc b/test/testsupport/perf_result_reporter.cc new file mode 100644 index 0000000000..e4c98e7446 --- /dev/null +++ b/test/testsupport/perf_result_reporter.cc @@ -0,0 +1,155 @@ +/* + * 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_result_reporter.h" + +#include + +namespace { + +// These characters mess with either the stdout parsing or the dashboard itself. +const std::vector& InvalidCharacters() { + static const std::vector kInvalidCharacters({"/", ":", "="}); + + return kInvalidCharacters; +} + +void CheckForInvalidCharacters(const std::string& str) { + for (const auto& invalid : InvalidCharacters()) { + RTC_CHECK(str.find(invalid) == std::string::npos) + << "Given invalid character for perf names '" << invalid << "'"; + } +} + +} // namespace + +namespace webrtc { +namespace test { + +namespace { + +// For now, mark all tests as "not important". This distinction mostly goes away +// in histograms anyway. +const bool kNotImportant = false; + +std::string UnitToString(Unit unit) { + // Down the line, we should convert directly from Unit to the histogram.proto + // enum values. We need to convert to strings until all uses of perf_test.h + // have been eliminated. We're not using the proto enum directly in the .h of + // this file because we don't want to limit the exposure of the proto. + // + // Keep this list up to date with kJsonToProtoUnit in histogram.cc in the + // Catapult repo. + switch (unit) { + case Unit::kMs: + return "ms"; + case Unit::kMsBestFitFormat: + return "msBestFitFormat"; + case Unit::kMsTs: + return "tsMs"; + case Unit::kNPercent: + return "n%"; + case Unit::kSizeInBytes: + return "sizeInBytes"; + case Unit::kBytesPerSecond: + return "bytesPerSecond"; + case Unit::kHertz: + return "Hz"; + case Unit::kUnitless: + return "unitless"; + case Unit::kCount: + return "count"; + case Unit::kSigma: + return "sigma"; + default: + RTC_NOTREACHED() << "Unknown unit " << unit; + return "unitless"; + } +} + +} // namespace + +PerfResultReporter::PerfResultReporter(const std::string& metric_basename, + const std::string& story_name) + : metric_basename_(metric_basename), story_name_(story_name) { + CheckForInvalidCharacters(metric_basename_); + CheckForInvalidCharacters(story_name_); +} + +PerfResultReporter::~PerfResultReporter() = default; + +void PerfResultReporter::RegisterMetric(const std::string& metric_suffix, + Unit unit) { + RegisterMetric(metric_suffix, unit, ImproveDirection::kNone); +} +void PerfResultReporter::RegisterMetric(const std::string& metric_suffix, + Unit unit, + ImproveDirection improve_direction) { + CheckForInvalidCharacters(metric_suffix); + RTC_CHECK(metric_map_.count(metric_suffix) == 0); + metric_map_.insert({metric_suffix, {unit, improve_direction}}); +} + +void PerfResultReporter::AddResult(const std::string& metric_suffix, + size_t value) const { + auto info = GetMetricInfoOrFail(metric_suffix); + + PrintResult(metric_basename_, metric_suffix, story_name_, value, + UnitToString(info.unit), kNotImportant, info.improve_direction); +} + +void PerfResultReporter::AddResult(const std::string& metric_suffix, + double value) const { + auto info = GetMetricInfoOrFail(metric_suffix); + + PrintResult(metric_basename_, metric_suffix, story_name_, value, + UnitToString(info.unit), kNotImportant, info.improve_direction); +} + +void PerfResultReporter::AddResultList( + const std::string& metric_suffix, + rtc::ArrayView values) const { + auto info = GetMetricInfoOrFail(metric_suffix); + + PrintResultList(metric_basename_, metric_suffix, story_name_, values, + UnitToString(info.unit), kNotImportant, + info.improve_direction); +} + +void PerfResultReporter::AddResultMeanAndError(const std::string& metric_suffix, + const double mean, + const double error) { + auto info = GetMetricInfoOrFail(metric_suffix); + + PrintResultMeanAndError(metric_basename_, metric_suffix, story_name_, mean, + error, UnitToString(info.unit), kNotImportant, + info.improve_direction); +} + +absl::optional PerfResultReporter::GetMetricInfo( + const std::string& metric_suffix) const { + auto iter = metric_map_.find(metric_suffix); + if (iter == metric_map_.end()) { + return absl::optional(); + } + + return absl::optional(iter->second); +} + +MetricInfo PerfResultReporter::GetMetricInfoOrFail( + const std::string& metric_suffix) const { + absl::optional info = GetMetricInfo(metric_suffix); + RTC_CHECK(info.has_value()) + << "Attempted to use unregistered metric " << metric_suffix; + return *info; +} + +} // namespace test +} // namespace webrtc diff --git a/test/testsupport/perf_result_reporter.h b/test/testsupport/perf_result_reporter.h new file mode 100644 index 0000000000..c8028574aa --- /dev/null +++ b/test/testsupport/perf_result_reporter.h @@ -0,0 +1,101 @@ +/* + * 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_RESULT_REPORTER_H_ +#define TEST_TESTSUPPORT_PERF_RESULT_REPORTER_H_ + +#include +#include + +#include "absl/types/optional.h" +#include "api/array_view.h" +#include "test/testsupport/perf_test.h" + +namespace webrtc { +namespace test { + +// These match the units in histogram.proto (in third_party/catapult). +enum class Unit { + kMs, + kMsBestFitFormat, + kMsTs, + kNPercent, + kSizeInBytes, + kBytesPerSecond, + kHertz, + kUnitless, + kCount, + kSigma, +}; + +struct MetricInfo { + Unit unit; + ImproveDirection improve_direction; +}; + +// A helper class for using the perf test printing functions safely, as +// otherwise it's easy to accidentally mix up arguments to produce usable but +// malformed perf data. See https://crbug.com/923564. +// +// Sample usage: +// auto reporter = PerfResultReporter("ramp_up_time", "bwe_15s"); +// reporter.RegisterImportantMetric( +// "_turn_over_tcp", Unit::kMs, ImproveDirection::kBiggerIsBetter); +// reporter.RegisterImportantMetric("_cpu_time", Unit::kMs); +// ... +// reporter.AddResult("turn_over_tcp", GetTurnOverTcpTime()); +// reporter.AddResult("turn_over_udp", GetTurnOverUdpTime()); +// +// This will show in the dashboard as +// (test binary name) > (bot) > ramp_up_time_turn_over_tcp > bwe_15s. +// (test binary name) > (bot) > ramp_up_time_turn_over_udp > bwe_15s. +// +// If you add more reporters that cover other user stories, they will show up +// as separate subtests (e.g. next to bwe_15s). +class PerfResultReporter { + public: + PerfResultReporter(const std::string& metric_basename, + const std::string& story_name); + ~PerfResultReporter(); + + void RegisterMetric(const std::string& metric_suffix, Unit unit); + void RegisterMetric(const std::string& metric_suffix, + Unit unit, + ImproveDirection improve_direction); + void AddResult(const std::string& metric_suffix, size_t value) const; + void AddResult(const std::string& metric_suffix, double value) const; + + void AddResultList(const std::string& metric_suffix, + rtc::ArrayView values) const; + + // Users should prefer AddResultList if possible, as otherwise the min/max + // values reported on the perf dashboard aren't useful. + // |mean_and_error| should be a comma-separated string of mean then + // error/stddev, e.g. "2.4,0.5". + void AddResultMeanAndError(const std::string& metric_suffix, + const double mean, + const double error); + + // Returns the metric info if it has been registered. + absl::optional GetMetricInfo( + const std::string& metric_suffix) const; + + private: + MetricInfo GetMetricInfoOrFail(const std::string& metric_suffix) const; + + std::string metric_basename_; + std::string story_name_; + std::unordered_map metric_map_; +}; + +} // namespace test +} // namespace webrtc + +#endif // TEST_TESTSUPPORT_PERF_RESULT_REPORTER_H_ diff --git a/test/testsupport/perf_test.cc b/test/testsupport/perf_test.cc index ae9ce6ee1f..2ab91901d1 100644 --- a/test/testsupport/perf_test.cc +++ b/test/testsupport/perf_test.cc @@ -25,10 +25,10 @@ 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."); + "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 {