Make PrintResultList receive a vector of doubles instead of a string.

Also, add more tests to perf_test_unittest.

Bug: webrtc:8566
Change-Id: I8864db7172fa207803d310c4a5fee4bf820a56bd
Reviewed-on: https://webrtc-review.googlesource.com/25823
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Reviewed-by: Patrik Höglund <phoglund@webrtc.org>
Commit-Queue: Edward Lemur <ehmaldonado@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20906}

call/call_perf_tests.cc

@@ -130,8 +130,7 @@ class VideoRtcpAndSyncObserver : public test::RtpRtcpObserver,
 
   void PrintResults() {
     test::PrintResultList("stream_offset", "", "synchronization",
-                          test::ValuesToString(sync_offset_ms_list_), "ms",
+                          sync_offset_ms_list_, "ms", false);
-                          false);
   }
 
  private:
@@ -140,7 +139,7 @@ class VideoRtcpAndSyncObserver : public test::RtpRtcpObserver,
   int64_t first_time_in_sync_;
   rtc::CriticalSection crit_;
   VideoReceiveStream* receive_stream_ RTC_GUARDED_BY(crit_);
-  std::vector<int> sync_offset_ms_list_;
+  std::vector<double> sync_offset_ms_list_;
 };
 
 void CallPerfTest::TestAudioVideoSync(FecMode fec,
@@ -458,8 +457,7 @@ void CallPerfTest::TestCaptureNtpTime(const FakeNetworkPipe::Config& net_config,
                              "estimated capture NTP time to be "
                              "within bounds.";
       test::PrintResultList("capture_ntp_time", "", "real - estimated",
-                            test::ValuesToString(time_offset_ms_list_), "ms",
+                            time_offset_ms_list_, "ms", true);
-                            true);
     }
 
     rtc::CriticalSection crit_;
@@ -474,7 +472,7 @@ void CallPerfTest::TestCaptureNtpTime(const FakeNetworkPipe::Config& net_config,
     uint32_t rtp_start_timestamp_;
     typedef std::map<uint32_t, uint32_t> FrameCaptureTimeList;
     FrameCaptureTimeList capture_time_list_ RTC_GUARDED_BY(&crit_);
-    std::vector<int> time_offset_ms_list_;
+    std::vector<double> time_offset_ms_list_;
   } test(net_config, threshold_ms, start_time_ms, run_time_ms);
 
   RunBaseTest(&test);
@@ -651,8 +649,7 @@ void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) {
           "bitrate_stats_",
           (pad_to_min_bitrate_ ? "min_transmit_bitrate"
                                : "without_min_transmit_bitrate"),
-          "bitrate_kbps", test::ValuesToString(bitrate_kbps_list_), "kbps",
+          "bitrate_kbps", bitrate_kbps_list_, "kbps", false);
-          false);
     }
 
     VideoSendStream* send_stream_;
@@ -661,7 +658,7 @@ void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) {
     const int min_acceptable_bitrate_;
     const int max_acceptable_bitrate_;
     int num_bitrate_observations_in_range_;
-    std::vector<size_t> bitrate_kbps_list_;
+    std::vector<double> bitrate_kbps_list_;
   } test(pad_to_min_bitrate);
 
   fake_encoder_.SetMaxBitrate(kMaxEncodeBitrateKbps);

modules/audio_processing/audio_processing_performance_unittest.cc

@@ -269,12 +269,9 @@ class TimedThreadApiProcessor {
         "us", false);
 
     if (kPrintAllDurations) {
-      std::string value_string = "";
-      for (int64_t duration : api_call_durations_) {
-        value_string += std::to_string(duration) + ",";
-      }
       webrtc::test::PrintResultList("apm_call_durations", sample_rate_name,
-                                    processor_name, value_string, "us", false);
+                                    processor_name, api_call_durations_, "us",
+                                    false);
     }
   }
 
@@ -432,7 +429,7 @@ class TimedThreadApiProcessor {
   AudioFrameData frame_data_;
   webrtc::Clock* clock_;
   const size_t num_durations_to_store_;
-  std::vector<int64_t> api_call_durations_;
+  std::vector<double> api_call_durations_;
   const float input_level_;
   bool first_process_call_ = true;
   const ProcessorType processor_type_;

test/testsupport/perf_test.cc

@@ -16,43 +16,24 @@
 
 #include <sstream>
 #include <stdio.h>
+#include <vector>
 
 namespace {
 
-std::string ResultsToString(const std::string& measurement,
+void PrintResultsImpl(const std::string& graph_name,
-                            const std::string& modifier,
+                      const std::string& trace,
-                            const std::string& trace,
+                      const std::string& values,
-                            const std::string& values,
+                      const std::string& units,
-                            const std::string& prefix,
+                      bool important) {
-                            const std::string& suffix,
-                            const std::string& units,
-                            bool important) {
   // <*>RESULT <graph_name>: <trace_name>= <value> <units>
   // <*>RESULT <graph_name>: <trace_name>= {<mean>, <std deviation>} <units>
   // <*>RESULT <graph_name>: <trace_name>= [<value>,value,value,...,] <units>
 
-  // TODO(ajm): Use of a stream here may violate the style guide (depending on
-  // one's definition of "logging"). Consider adding StringPrintf-like
-  // functionality as in the original Chromium implementation.
-  std::ostringstream stream;
   if (important) {
-    stream << "*";
+    printf("*");
   }
-  stream << "RESULT " << measurement << modifier << ": " << trace << "= "
+  printf("RESULT %s: %s= %s %s\n", graph_name.c_str(), trace.c_str(),
-         << prefix << values << suffix << " " << units << std::endl;
+         values.c_str(), units.c_str());
-  return stream.str();
-}
-
-void PrintResultsImpl(const std::string& measurement,
-                      const std::string& modifier,
-                      const std::string& trace,
-                      const std::string& values,
-                      const std::string& prefix,
-                      const std::string& suffix,
-                      const std::string& units,
-                      bool important) {
-  printf("%s", ResultsToString(measurement, modifier, trace, values,
-                               prefix, suffix, units, important).c_str());
 }
 
 }  // namespace
@@ -68,8 +49,8 @@ void PrintResult(const std::string& measurement,
                  bool important) {
   std::ostringstream value_stream;
   value_stream << value;
-  PrintResultsImpl(measurement, modifier, trace, value_stream.str(), "", "",
+  PrintResultsImpl(measurement + modifier, trace, value_stream.str(), units,
-                   units, important);
+                   important);
 }
 
 void PrintResultMeanAndError(const std::string& measurement,
@@ -81,18 +62,30 @@ void PrintResultMeanAndError(const std::string& measurement,
                              bool important) {
   std::ostringstream value_stream;
   value_stream << '{' << mean << ',' << error << '}';
-  PrintResultsImpl(measurement, modifier, trace, value_stream.str(), "", "",
+  PrintResultsImpl(measurement + modifier, trace, value_stream.str(), units,
-                   units, important);
+                   important);
 }
 
 void PrintResultList(const std::string& measurement,
                      const std::string& modifier,
                      const std::string& trace,
-                     const std::string& values,
+                     const std::vector<double>& values,
                      const std::string& units,
                      bool important) {
-  PrintResultsImpl(measurement, modifier, trace, values,
+  std::ostringstream value_stream;
-                   "[", "]", units, important);
+  value_stream << '[';
+  if (!values.empty()) {
+    auto it = values.begin();
+    while (true) {
+      value_stream << *it;
+      if (++it == values.end())
+        break;
+      value_stream << ',';
+    }
+  }
+  value_stream << ']';
+  PrintResultsImpl(measurement + modifier, trace, value_stream.str(), units,
+                   important);
 }
 
 }  // namespace test

test/testsupport/perf_test.h

@@ -17,6 +17,7 @@
 
 #include <sstream>
 #include <string>
+#include <vector>
 
 namespace webrtc {
 namespace test {
@@ -52,6 +53,7 @@ void PrintResultMeanAndError(const std::string& measurement,
                              const std::string& units,
                              bool important);
 
+
 // Like PrintResult(), but prints an entire list of results. The |values|
 // will generally be a list of comma-separated numbers. A typical
 // post-processing step might produce plots of their mean and standard
@@ -59,27 +61,10 @@ void PrintResultMeanAndError(const std::string& measurement,
 void PrintResultList(const std::string& measurement,
                      const std::string& modifier,
                      const std::string& trace,
-                     const std::string& values,
+                     const std::vector<double>& values,
                      const std::string& units,
                      bool important);
 
-// Converts list of values into comma-separated string for PrintResultList.
-template <typename Container>
-std::string ValuesToString(const Container& container) {
-  if (container.empty())
-    return "";
-
-  std::stringstream ss;
-  auto it = container.begin();
-  while (true) {
-    ss << *it;
-    if (++it == container.end())
-      break;
-    ss << ',';
-  }
-  return ss.str();
-}
-
 }  // namespace test
 }  // namespace webrtc
 

test/testsupport/perf_test_unittest.cc

@@ -23,7 +23,13 @@ TEST(PerfTest, AppendResult) {
   PrintResult("measurement", "modifier", "trace", 42, "units", false);
 
   expected += "*RESULT foobar: baz= 7 widgets\n";
-  PrintResult( "foo", "bar", "baz", 7, "widgets", true);
+  PrintResult("foo", "bar", "baz", 7, "widgets", true);
+
+  expected += "RESULT foobar: baz= {1,2} lemurs\n";
+  PrintResultMeanAndError("foo", "bar", "baz", 1, 2, "lemurs", false);
+
+  expected += "RESULT foobar: baz= [1,2,3] units\n";
+  PrintResultList("foo", "bar", "baz", {1, 2, 3}, "units", false);
 
   std::string output = testing::internal::GetCapturedStdout();
   EXPECT_EQ(expected, output);