Add PerfResultsReporter.

This is the WebRTC equivalent of testing/perf/perf_result_reporter.h
in Chromium. That class was introduced because the PrintResult
functions are quite hard to use right. It was easy to mix up
metrics, modifiers and stories, for instance.

I choose to introduce this new class because I need to create a new
API for PrintResult anyway. For instance, the important bool isn't
really supported by histograms. Also I would like to restrict units
to an enum because you cannot make up your own units anymore.
We could also have had a strictly checked string type, but that's
bad API design. An enum is better because the compiler will check
that the unit is valid rather than at runtime.

Furthermore, down the line we can probably make each reporter write
protos directly to /tmp and merge them later, instead of having a
singleton which writes results at the end and keeps all test results
in memory. This abstraction makes it easy to make a clean and simple
implementation of just that.

Steps:
1) land this
2) start rewriting perf tests to use this class
3) nuke PrintResult functions
4) don't convert units to string, convert directly from Unit
   to proto::Unit
5) write protos directly from this class (either through
   a singleton or directly) and nuke the perf results writer
   abstraction.

Bug: chromium:1029452
Change-Id: Ia919c371a69309130c797fdf01ae5bd64345ab2e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168770
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Patrik Höglund <phoglund@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30599}

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" ]

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 <vector>
+
+namespace {
+
+// These characters mess with either the stdout parsing or the dashboard itself.
+const std::vector<std::string>& InvalidCharacters() {
+  static const std::vector<std::string> 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<const double> 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<MetricInfo> PerfResultReporter::GetMetricInfo(
+    const std::string& metric_suffix) const {
+  auto iter = metric_map_.find(metric_suffix);
+  if (iter == metric_map_.end()) {
+    return absl::optional<MetricInfo>();
+  }
+
+  return absl::optional<MetricInfo>(iter->second);
+}
+
+MetricInfo PerfResultReporter::GetMetricInfoOrFail(
+    const std::string& metric_suffix) const {
+  absl::optional<MetricInfo> info = GetMetricInfo(metric_suffix);
+  RTC_CHECK(info.has_value())
+      << "Attempted to use unregistered metric " << metric_suffix;
+  return *info;
+}
+
+}  // namespace test
+}  // namespace webrtc

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 <string>
+#include <unordered_map>
+
+#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<const double> 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<MetricInfo> 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<std::string, MetricInfo> metric_map_;
+};
+
+}  // namespace test
+}  // namespace webrtc
+
+#endif  // TEST_TESTSUPPORT_PERF_RESULT_REPORTER_H_

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 {