diff --git a/test/BUILD.gn b/test/BUILD.gn index 01539d5d0a..6acea6ec84 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -282,8 +282,6 @@ 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_histogram_writer.h", diff --git a/test/testsupport/perf_result_reporter.cc b/test/testsupport/perf_result_reporter.cc deleted file mode 100644 index 436d1baa0a..0000000000 --- a/test/testsupport/perf_result_reporter.cc +++ /dev/null @@ -1,158 +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_result_reporter.h" - -#include - -#include "absl/strings/string_view.h" - -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(absl::string_view 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_DCHECK_NOTREACHED() << "Unknown unit " << unit; - return "unitless"; - } -} - -} // namespace - -PerfResultReporter::PerfResultReporter(absl::string_view metric_basename, - absl::string_view story_name) - : metric_basename_(metric_basename), story_name_(story_name) { - CheckForInvalidCharacters(metric_basename_); - CheckForInvalidCharacters(story_name_); -} - -PerfResultReporter::~PerfResultReporter() = default; - -void PerfResultReporter::RegisterMetric(absl::string_view metric_suffix, - Unit unit) { - RegisterMetric(metric_suffix, unit, ImproveDirection::kNone); -} -void PerfResultReporter::RegisterMetric(absl::string_view metric_suffix, - Unit unit, - ImproveDirection improve_direction) { - CheckForInvalidCharacters(metric_suffix); - std::string metric(metric_suffix); - RTC_CHECK(metric_map_.count(metric) == 0); - metric_map_.insert({std::move(metric), {unit, improve_direction}}); -} - -void PerfResultReporter::AddResult(absl::string_view 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(absl::string_view 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( - absl::string_view 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(absl::string_view 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( - absl::string_view metric_suffix) const { - auto iter = metric_map_.find(std::string(metric_suffix)); - if (iter == metric_map_.end()) { - return absl::optional(); - } - - return absl::optional(iter->second); -} - -MetricInfo PerfResultReporter::GetMetricInfoOrFail( - absl::string_view 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 deleted file mode 100644 index a1572341c7..0000000000 --- a/test/testsupport/perf_result_reporter.h +++ /dev/null @@ -1,102 +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_RESULT_REPORTER_H_ -#define TEST_TESTSUPPORT_PERF_RESULT_REPORTER_H_ - -#include -#include - -#include "absl/strings/string_view.h" -#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(absl::string_view metric_basename, - absl::string_view story_name); - ~PerfResultReporter(); - - void RegisterMetric(absl::string_view metric_suffix, Unit unit); - void RegisterMetric(absl::string_view metric_suffix, - Unit unit, - ImproveDirection improve_direction); - void AddResult(absl::string_view metric_suffix, size_t value) const; - void AddResult(absl::string_view metric_suffix, double value) const; - - void AddResultList(absl::string_view 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(absl::string_view metric_suffix, - double mean, - double error); - - // Returns the metric info if it has been registered. - absl::optional GetMetricInfo( - absl::string_view metric_suffix) const; - - private: - MetricInfo GetMetricInfoOrFail(absl::string_view 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_