diff --git a/test/BUILD.gn b/test/BUILD.gn index e07ce6f10e..ed4eebefd5 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -252,10 +252,14 @@ rtc_library("perf_test") { "../rtc_base:criticalsection", "../rtc_base:logging", "../rtc_base:rtc_numerics", + "../rtc_base:stringutils", "../rtc_base/synchronization:mutex", "../test:fileutils", ] - absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] + absl_deps = [ + "//third_party/abseil-cpp/absl/strings", + "//third_party/abseil-cpp/absl/types:optional", + ] if (rtc_enable_protobuf) { sources += [ "testsupport/perf_test_histogram_writer.cc" ] deps += [ @@ -542,6 +546,8 @@ if (rtc_include_tests && !build_with_chromium) { "scenario:scenario_unittests", "time_controller:time_controller", "time_controller:time_controller_unittests", + ] + absl_deps = [ "//third_party/abseil-cpp/absl/flags:flag", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", diff --git a/test/testsupport/perf_result_reporter.cc b/test/testsupport/perf_result_reporter.cc index e4c98e7446..158f1cd768 100644 --- a/test/testsupport/perf_result_reporter.cc +++ b/test/testsupport/perf_result_reporter.cc @@ -12,6 +12,8 @@ #include +#include "absl/strings/string_view.h" + namespace { // These characters mess with either the stdout parsing or the dashboard itself. @@ -21,7 +23,7 @@ const std::vector& InvalidCharacters() { return kInvalidCharacters; } -void CheckForInvalidCharacters(const std::string& str) { +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 << "'"; @@ -76,8 +78,8 @@ std::string UnitToString(Unit unit) { } // namespace -PerfResultReporter::PerfResultReporter(const std::string& metric_basename, - const std::string& story_name) +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_); @@ -85,19 +87,20 @@ PerfResultReporter::PerfResultReporter(const std::string& metric_basename, PerfResultReporter::~PerfResultReporter() = default; -void PerfResultReporter::RegisterMetric(const std::string& metric_suffix, +void PerfResultReporter::RegisterMetric(absl::string_view metric_suffix, Unit unit) { RegisterMetric(metric_suffix, unit, ImproveDirection::kNone); } -void PerfResultReporter::RegisterMetric(const std::string& metric_suffix, +void PerfResultReporter::RegisterMetric(absl::string_view 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}}); + std::string metric(metric_suffix); + RTC_CHECK(metric_map_.count(metric) == 0); + metric_map_.insert({std::move(metric), {unit, improve_direction}}); } -void PerfResultReporter::AddResult(const std::string& metric_suffix, +void PerfResultReporter::AddResult(absl::string_view metric_suffix, size_t value) const { auto info = GetMetricInfoOrFail(metric_suffix); @@ -105,7 +108,7 @@ void PerfResultReporter::AddResult(const std::string& metric_suffix, UnitToString(info.unit), kNotImportant, info.improve_direction); } -void PerfResultReporter::AddResult(const std::string& metric_suffix, +void PerfResultReporter::AddResult(absl::string_view metric_suffix, double value) const { auto info = GetMetricInfoOrFail(metric_suffix); @@ -114,7 +117,7 @@ void PerfResultReporter::AddResult(const std::string& metric_suffix, } void PerfResultReporter::AddResultList( - const std::string& metric_suffix, + absl::string_view metric_suffix, rtc::ArrayView values) const { auto info = GetMetricInfoOrFail(metric_suffix); @@ -123,7 +126,7 @@ void PerfResultReporter::AddResultList( info.improve_direction); } -void PerfResultReporter::AddResultMeanAndError(const std::string& metric_suffix, +void PerfResultReporter::AddResultMeanAndError(absl::string_view metric_suffix, const double mean, const double error) { auto info = GetMetricInfoOrFail(metric_suffix); @@ -134,8 +137,8 @@ void PerfResultReporter::AddResultMeanAndError(const std::string& metric_suffix, } absl::optional PerfResultReporter::GetMetricInfo( - const std::string& metric_suffix) const { - auto iter = metric_map_.find(metric_suffix); + absl::string_view metric_suffix) const { + auto iter = metric_map_.find(std::string(metric_suffix)); if (iter == metric_map_.end()) { return absl::optional(); } @@ -144,7 +147,7 @@ absl::optional PerfResultReporter::GetMetricInfo( } MetricInfo PerfResultReporter::GetMetricInfoOrFail( - const std::string& metric_suffix) const { + absl::string_view metric_suffix) const { absl::optional info = GetMetricInfo(metric_suffix); RTC_CHECK(info.has_value()) << "Attempted to use unregistered metric " << metric_suffix; diff --git a/test/testsupport/perf_result_reporter.h b/test/testsupport/perf_result_reporter.h index c8028574aa..aeb1786824 100644 --- a/test/testsupport/perf_result_reporter.h +++ b/test/testsupport/perf_result_reporter.h @@ -14,6 +14,7 @@ #include #include +#include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/array_view.h" #include "test/testsupport/perf_test.h" @@ -61,34 +62,34 @@ struct MetricInfo { // as separate subtests (e.g. next to bwe_15s). class PerfResultReporter { public: - PerfResultReporter(const std::string& metric_basename, - const std::string& story_name); + PerfResultReporter(absl::string_view metric_basename, + absl::string_view story_name); ~PerfResultReporter(); - void RegisterMetric(const std::string& metric_suffix, Unit unit); - void RegisterMetric(const std::string& metric_suffix, + void RegisterMetric(absl::string_view metric_suffix, Unit unit); + void RegisterMetric(absl::string_view 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 AddResult(absl::string_view metric_suffix, size_t value) const; + void AddResult(absl::string_view metric_suffix, double value) const; - void AddResultList(const std::string& metric_suffix, + 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(const std::string& metric_suffix, + void AddResultMeanAndError(absl::string_view 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; + absl::string_view metric_suffix) const; private: - MetricInfo GetMetricInfoOrFail(const std::string& metric_suffix) const; + MetricInfo GetMetricInfoOrFail(absl::string_view metric_suffix) const; std::string metric_basename_; std::string story_name_; diff --git a/test/testsupport/perf_test.cc b/test/testsupport/perf_test.cc index b68eaa46a1..c3800b93d0 100644 --- a/test/testsupport/perf_test.cc +++ b/test/testsupport/perf_test.cc @@ -17,7 +17,9 @@ #include #include +#include "absl/strings/string_view.h" #include "rtc_base/checks.h" +#include "rtc_base/strings/string_builder.h" #include "rtc_base/synchronization/mutex.h" #include "test/testsupport/file_utils.h" #include "test/testsupport/perf_test_histogram_writer.h" @@ -28,15 +30,15 @@ namespace test { namespace { std::string UnitWithDirection( - const std::string& units, + absl::string_view units, webrtc::test::ImproveDirection improve_direction) { switch (improve_direction) { case webrtc::test::ImproveDirection::kNone: - return units; + return std::string(units); case webrtc::test::ImproveDirection::kSmallerIsBetter: - return units + "_smallerIsBetter"; + return std::string(units) + "_smallerIsBetter"; case webrtc::test::ImproveDirection::kBiggerIsBetter: - return units + "_biggerIsBetter"; + return std::string(units) + "_biggerIsBetter"; } } @@ -65,12 +67,14 @@ class PlottableCounterPrinter { output_ = output; } - void AddCounter(const std::string& graph_name, - const std::string& trace_name, + void AddCounter(absl::string_view graph_name, + absl::string_view trace_name, const webrtc::SamplesStatsCounter& counter, - const std::string& units) { + absl::string_view units) { MutexLock lock(&mutex_); - plottable_counters_.push_back({graph_name, trace_name, counter, units}); + plottable_counters_.push_back({std::string(graph_name), + std::string(trace_name), counter, + std::string(units)}); } void Print(const std::vector& desired_graphs_raw) const { @@ -128,10 +132,10 @@ class ResultsLinePrinter { output_ = output; } - void PrintResult(const std::string& graph_name, - const std::string& trace_name, + void PrintResult(absl::string_view graph_name, + absl::string_view trace_name, const double value, - const std::string& units, + absl::string_view units, bool important, ImproveDirection improve_direction) { std::ostringstream value_stream; @@ -143,11 +147,11 @@ class ResultsLinePrinter { important); } - void PrintResultMeanAndError(const std::string& graph_name, - const std::string& trace_name, + void PrintResultMeanAndError(absl::string_view graph_name, + absl::string_view trace_name, const double mean, const double error, - const std::string& units, + absl::string_view units, bool important, ImproveDirection improve_direction) { std::ostringstream value_stream; @@ -157,10 +161,10 @@ class ResultsLinePrinter { UnitWithDirection(units, improve_direction), important); } - void PrintResultList(const std::string& graph_name, - const std::string& trace_name, + void PrintResultList(absl::string_view graph_name, + absl::string_view trace_name, const rtc::ArrayView values, - const std::string& units, + absl::string_view units, const bool important, webrtc::test::ImproveDirection improve_direction) { std::ostringstream value_stream; @@ -171,20 +175,21 @@ class ResultsLinePrinter { } private: - void PrintResultImpl(const std::string& graph_name, - const std::string& trace_name, - const std::string& values, - const std::string& prefix, - const std::string& suffix, - const std::string& units, + void PrintResultImpl(absl::string_view graph_name, + absl::string_view trace_name, + absl::string_view values, + absl::string_view prefix, + absl::string_view suffix, + absl::string_view units, bool important) { MutexLock lock(&mutex_); + rtc::StringBuilder message; + message << (important ? "*" : "") << "RESULT " << graph_name << ": " + << trace_name << "= " << prefix << values << suffix << " " << units; // <*>RESULT : = // <*>RESULT : = {, } // <*>RESULT : = [,value,value,...,] - fprintf(output_, "%sRESULT %s: %s= %s%s%s %s\n", important ? "*" : "", - graph_name.c_str(), trace_name.c_str(), prefix.c_str(), - values.c_str(), suffix.c_str(), units.c_str()); + fprintf(output_, "%s\n", message.str().c_str()); } Mutex mutex_; @@ -241,32 +246,35 @@ bool WritePerfResults(const std::string& output_path) { return true; } -void PrintResult(const std::string& measurement, - const std::string& modifier, - const std::string& trace, +void PrintResult(absl::string_view measurement, + absl::string_view modifier, + absl::string_view trace, const double value, - const std::string& units, + absl::string_view units, bool important, ImproveDirection improve_direction) { - std::string graph_name = measurement + modifier; + rtc::StringBuilder graph_name; + graph_name << measurement << modifier; RTC_CHECK(std::isfinite(value)) - << "Expected finite value for graph " << graph_name << ", trace name " - << trace << ", units " << units << ", got " << value; - GetPerfWriter().LogResult(graph_name, trace, value, units, important, + << "Expected finite value for graph " << graph_name.str() + << ", trace name " << trace << ", units " << units << ", got " << value; + GetPerfWriter().LogResult(graph_name.str(), trace, value, units, important, improve_direction); - GetResultsLinePrinter().PrintResult(graph_name, trace, value, units, + GetResultsLinePrinter().PrintResult(graph_name.str(), trace, value, units, important, improve_direction); } -void PrintResult(const std::string& measurement, - const std::string& modifier, - const std::string& trace, +void PrintResult(absl::string_view measurement, + absl::string_view modifier, + absl::string_view trace, const SamplesStatsCounter& counter, - const std::string& units, + absl::string_view units, const bool important, ImproveDirection improve_direction) { - std::string graph_name = measurement + modifier; - GetPlottableCounterPrinter().AddCounter(graph_name, trace, counter, units); + rtc::StringBuilder graph_name; + graph_name << measurement << modifier; + GetPlottableCounterPrinter().AddCounter(graph_name.str(), trace, counter, + units); double mean = counter.IsEmpty() ? 0 : counter.GetAverage(); double error = counter.IsEmpty() ? 0 : counter.GetStandardDeviation(); @@ -274,40 +282,43 @@ void PrintResult(const std::string& measurement, important, improve_direction); } -void PrintResultMeanAndError(const std::string& measurement, - const std::string& modifier, - const std::string& trace, +void PrintResultMeanAndError(absl::string_view measurement, + absl::string_view modifier, + absl::string_view trace, const double mean, const double error, - const std::string& units, + absl::string_view units, bool important, ImproveDirection improve_direction) { RTC_CHECK(std::isfinite(mean)); RTC_CHECK(std::isfinite(error)); - std::string graph_name = measurement + modifier; - GetPerfWriter().LogResultMeanAndError(graph_name, trace, mean, error, units, - important, improve_direction); - GetResultsLinePrinter().PrintResultMeanAndError( - graph_name, trace, mean, error, units, important, improve_direction); + rtc::StringBuilder graph_name; + graph_name << measurement << modifier; + GetPerfWriter().LogResultMeanAndError(graph_name.str(), trace, mean, error, + units, important, improve_direction); + GetResultsLinePrinter().PrintResultMeanAndError(graph_name.str(), trace, mean, + error, units, important, + improve_direction); } -void PrintResultList(const std::string& measurement, - const std::string& modifier, - const std::string& trace, +void PrintResultList(absl::string_view measurement, + absl::string_view modifier, + absl::string_view trace, const rtc::ArrayView values, - const std::string& units, + absl::string_view units, bool important, ImproveDirection improve_direction) { for (double v : values) { RTC_CHECK(std::isfinite(v)); } - std::string graph_name = measurement + modifier; - GetPerfWriter().LogResultList(graph_name, trace, values, units, important, - improve_direction); - GetResultsLinePrinter().PrintResultList(graph_name, trace, values, units, - important, improve_direction); + rtc::StringBuilder graph_name; + graph_name << measurement << modifier; + GetPerfWriter().LogResultList(graph_name.str(), trace, values, units, + important, improve_direction); + GetResultsLinePrinter().PrintResultList(graph_name.str(), trace, values, + units, important, improve_direction); } } // namespace test diff --git a/test/testsupport/perf_test.h b/test/testsupport/perf_test.h index 25535bce82..41380241c3 100644 --- a/test/testsupport/perf_test.h +++ b/test/testsupport/perf_test.h @@ -15,6 +15,7 @@ #include #include +#include "absl/strings/string_view.h" #include "api/array_view.h" #include "api/numerics/samples_stats_counter.h" @@ -45,11 +46,11 @@ enum class ImproveDirection { // // The binary this runs in must be hooked up as a perf test in the WebRTC // recipes for this to actually be uploaded to chromeperf.appspot.com. -void PrintResult(const std::string& measurement, - const std::string& modifier, - const std::string& user_story, +void PrintResult(absl::string_view measurement, + absl::string_view modifier, + absl::string_view user_story, const double value, - const std::string& units, + absl::string_view units, bool important, ImproveDirection improve_direction = ImproveDirection::kNone); @@ -58,12 +59,12 @@ void PrintResult(const std::string& measurement, // standard deviation (or other error metric) of the measurement. // DEPRECATED: soon unsupported. void PrintResultMeanAndError( - const std::string& measurement, - const std::string& modifier, - const std::string& user_story, + absl::string_view measurement, + absl::string_view modifier, + absl::string_view user_story, const double mean, const double error, - const std::string& units, + absl::string_view units, bool important, ImproveDirection improve_direction = ImproveDirection::kNone); @@ -72,21 +73,21 @@ void PrintResultMeanAndError( // post-processing step might produce plots of their mean and standard // deviation. void PrintResultList( - const std::string& measurement, - const std::string& modifier, - const std::string& user_story, + absl::string_view measurement, + absl::string_view modifier, + absl::string_view user_story, rtc::ArrayView values, - const std::string& units, + absl::string_view units, bool important, ImproveDirection improve_direction = ImproveDirection::kNone); // Like PrintResult(), but prints a (mean, standard deviation) from stats // counter. Also add specified metric to the plotable metrics output. -void PrintResult(const std::string& measurement, - const std::string& modifier, - const std::string& user_story, +void PrintResult(absl::string_view measurement, + absl::string_view modifier, + absl::string_view user_story, const SamplesStatsCounter& counter, - const std::string& units, + absl::string_view units, const bool important, ImproveDirection improve_direction = ImproveDirection::kNone); diff --git a/test/testsupport/perf_test_histogram_writer.cc b/test/testsupport/perf_test_histogram_writer.cc index a4f86dc5f0..c1514566fc 100644 --- a/test/testsupport/perf_test_histogram_writer.cc +++ b/test/testsupport/perf_test_histogram_writer.cc @@ -15,7 +15,9 @@ #include #include +#include "absl/strings/string_view.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/string_builder.h" #include "rtc_base/synchronization/mutex.h" #include "third_party/catapult/tracing/tracing/value/diagnostics/reserved_infos.h" #include "third_party/catapult/tracing/tracing/value/histogram.h" @@ -39,20 +41,20 @@ class PerfTestHistogramWriter : public PerfTestResultWriter { histograms_.clear(); } - void LogResult(const std::string& graph_name, - const std::string& trace_name, + void LogResult(absl::string_view graph_name, + absl::string_view trace_name, const double value, - const std::string& units, + absl::string_view units, const bool important, ImproveDirection improve_direction) override { (void)important; AddSample(graph_name, trace_name, value, units, improve_direction); } - void LogResultMeanAndError(const std::string& graph_name, - const std::string& trace_name, + void LogResultMeanAndError(absl::string_view graph_name, + absl::string_view trace_name, const double mean, const double error, - const std::string& units, + absl::string_view units, const bool important, ImproveDirection improve_direction) override { RTC_LOG(LS_WARNING) << "Discarding stddev, not supported by histograms"; @@ -61,10 +63,10 @@ class PerfTestHistogramWriter : public PerfTestResultWriter { AddSample(graph_name, trace_name, mean, units, improve_direction); } - void LogResultList(const std::string& graph_name, - const std::string& trace_name, + void LogResultList(absl::string_view graph_name, + absl::string_view trace_name, const rtc::ArrayView values, - const std::string& units, + absl::string_view units, const bool important, ImproveDirection improve_direction) override { (void)important; @@ -88,14 +90,14 @@ class PerfTestHistogramWriter : public PerfTestResultWriter { } private: - void AddSample(const std::string& original_graph_name, - const std::string& trace_name, + void AddSample(absl::string_view original_graph_name, + absl::string_view trace_name, const double value, - const std::string& units, + absl::string_view units, ImproveDirection improve_direction) { // WebRTC annotates the units into the metric name when they are not // supported by the Histogram API. - std::string graph_name = original_graph_name; + std::string graph_name(original_graph_name); if (units == "dB") { graph_name += "_dB"; } else if (units == "fps") { @@ -107,9 +109,10 @@ class PerfTestHistogramWriter : public PerfTestResultWriter { // Lookup on graph name + trace name (or measurement + story in catapult // parlance). There should be several histograms with the same measurement // if they're for different stories. - std::string measurement_and_story = graph_name + trace_name; + rtc::StringBuilder measurement_and_story; + measurement_and_story << graph_name << trace_name; MutexLock lock(&mutex_); - if (histograms_.count(measurement_and_story) == 0) { + if (histograms_.count(measurement_and_story.str()) == 0) { proto::UnitAndDirection unit = ParseUnit(units, improve_direction); std::unique_ptr builder = std::make_unique(graph_name, unit); @@ -117,24 +120,24 @@ class PerfTestHistogramWriter : public PerfTestResultWriter { // Set all summary options as false - we don't want to generate // metric_std, metric_count, and so on for all metrics. builder->SetSummaryOptions(proto::SummaryOptions()); - histograms_[measurement_and_story] = std::move(builder); + histograms_[measurement_and_story.str()] = std::move(builder); proto::Diagnostic stories; proto::GenericSet* generic_set = stories.mutable_generic_set(); - generic_set->add_values(AsJsonString(trace_name)); - histograms_[measurement_and_story]->AddDiagnostic( + generic_set->add_values(AsJsonString(std::string(trace_name))); + histograms_[measurement_and_story.str()]->AddDiagnostic( catapult::kStoriesDiagnostic, stories); } if (units == "bps") { // Bps has been interpreted as bits per second in WebRTC tests. - histograms_[measurement_and_story]->AddSample(value / 8); + histograms_[measurement_and_story.str()]->AddSample(value / 8); } else { - histograms_[measurement_and_story]->AddSample(value); + histograms_[measurement_and_story.str()]->AddSample(value); } } - proto::UnitAndDirection ParseUnit(const std::string& units, + proto::UnitAndDirection ParseUnit(absl::string_view units, ImproveDirection improve_direction) { RTC_DCHECK(units.find('_') == std::string::npos) << "The unit_bigger|smallerIsBetter syntax isn't supported in WebRTC, " @@ -155,7 +158,7 @@ class PerfTestHistogramWriter : public PerfTestResultWriter { } else if (units == "%") { result.set_unit(proto::UNITLESS); } else { - proto::Unit unit = catapult::UnitFromJsonUnit(units); + proto::Unit unit = catapult::UnitFromJsonUnit(std::string(units)); // UnitFromJsonUnit returns UNITLESS if it doesn't recognize the unit. if (unit == proto::UNITLESS && units != "unitless") { diff --git a/test/testsupport/perf_test_result_writer.h b/test/testsupport/perf_test_result_writer.h index d5d7011749..e7342c137f 100644 --- a/test/testsupport/perf_test_result_writer.h +++ b/test/testsupport/perf_test_result_writer.h @@ -12,8 +12,10 @@ #define TEST_TESTSUPPORT_PERF_TEST_RESULT_WRITER_H_ #include + #include +#include "absl/strings/string_view.h" #include "test/testsupport/perf_test.h" namespace webrtc { @@ -25,25 +27,25 @@ class PerfTestResultWriter { virtual ~PerfTestResultWriter() = default; virtual void ClearResults() = 0; - virtual void LogResult(const std::string& graph_name, - const std::string& trace_name, + virtual void LogResult(absl::string_view graph_name, + absl::string_view trace_name, const double value, - const std::string& units, + absl::string_view units, const bool important, webrtc::test::ImproveDirection improve_direction) = 0; virtual void LogResultMeanAndError( - const std::string& graph_name, - const std::string& trace_name, + absl::string_view graph_name, + absl::string_view trace_name, const double mean, const double error, - const std::string& units, + absl::string_view units, const bool important, webrtc::test::ImproveDirection improve_direction) = 0; virtual void LogResultList( - const std::string& graph_name, - const std::string& trace_name, + absl::string_view graph_name, + absl::string_view trace_name, const rtc::ArrayView values, - const std::string& units, + absl::string_view units, const bool important, webrtc::test::ImproveDirection improve_direction) = 0;