Extract an interface from the perf results logger.

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 <titovartem@webrtc.org>
Commit-Queue: Patrik Höglund <phoglund@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30243}

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.

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

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

test/testsupport/perf_test.cc

@@ -12,15 +12,24 @@
 
 #include <stdio.h>
 
-#include <cmath>
 #include <fstream>
-#include <map>
 #include <set>
 #include <sstream>
 #include <vector>
 
+#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<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 ToJSON() const;
-
- private:
-  rtc::CriticalSection crit_;
-  std::map<std::string, std::vector<std::string>> 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<std::string>& 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);
 }

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 <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 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<std::string, std::vector<std::string>> graphs_
+      RTC_GUARDED_BY(&crit_);
+};
+
+}  // namespace
+
+PerfTestResultWriter* CreateGraphJsonWriter() {
+  return new PerfTestGraphJsonWriter();
+}
+
+}  // namespace test
+}  // namespace webrtc

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 <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_

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 <stdlib.h>
+
+#include <map>
+
+namespace webrtc {
+namespace test {
+
+namespace {}  // namespace
+
+PerfTestResultWriter* CreateHistogramWriter() {
+  RTC_CHECK(false) << "Not implemented";
+  return nullptr;
+}
+
+}  // namespace test
+}  // namespace webrtc

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_

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 <stdio.h>
+#include <string>
+
+#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<const double> 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_