From 36b35d528cfcd3428cd3b52f73799523af6f4da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrik=20H=C3=B6glund?= Date: Fri, 27 Mar 2020 13:05:08 +0000 Subject: [PATCH] Reland "Flip histograms to true by default, fix unit in isac_fix_test." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit c59a3049016f697e05437b91ae494c40cede3506. Reason for revert: Other perf tests greening up, can now land this Original change's description: > Revert "Flip histograms to true by default, fix unit in isac_fix_test." > > This reverts commit 7b201012bc7f7cd95249b8314d0d7ebabe966d8b. > > Reason for revert: Seems to work, but need to get low bw tests working first > > Original change's description: > > Flip histograms to true by default, fix unit in isac_fix_test. > > > > Requires downstream changes for all WebRTC perf tests, and > > a corresponding recipe change so isac_fix_test starts using the new > > flow. > > > > Bug: chromium:1029452 > > Change-Id: I8918fca9bef003d365037c1c6bf7c55747dfed99 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/170633 > > Reviewed-by: Mirko Bonadei > > Commit-Queue: Patrik Höglund > > Cr-Commit-Position: refs/heads/master@{#30906} > > TBR=phoglund@webrtc.org,mbonadei@webrtc.org > > Change-Id: I96c2309cd71be14c5a27b515736a32f1b256453c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: chromium:1029452 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171865 > Reviewed-by: Patrik Höglund > Commit-Queue: Patrik Höglund > Cr-Commit-Position: refs/heads/master@{#30913} TBR=phoglund@webrtc.org,mbonadei@webrtc.org Change-Id: If39500beeca74b8e0ed9e97724a55529125a2253 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:1029452 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171876 Reviewed-by: Patrik Höglund Commit-Queue: Patrik Höglund Cr-Commit-Position: refs/heads/master@{#30920} --- modules/audio_coding/codecs/isac/fix/test/kenny.cc | 10 +++++----- test/testsupport/perf_test.cc | 2 +- test/testsupport/perf_test_unittest.cc | 11 ++++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/modules/audio_coding/codecs/isac/fix/test/kenny.cc b/modules/audio_coding/codecs/isac/fix/test/kenny.cc index a0a2dca118..4b431cfdf9 100644 --- a/modules/audio_coding/codecs/isac/fix/test/kenny.cc +++ b/modules/audio_coding/codecs/isac/fix/test/kenny.cc @@ -105,7 +105,7 @@ TEST(IsacFixTest, Kenny) { FILE *inp, *outp, *f_bn, *outbits; int endfile; - const char* chartjson_result_file = NULL; + const char* perf_result_file = NULL; int i; int errtype, h = 0, k, packetLossPercent = 0; @@ -459,7 +459,7 @@ TEST(IsacFixTest, Kenny) { printf("Expected --isolated_script_test_perf_output=/some/filename\n"); exit(1); } - chartjson_result_file = filename_start + 1; + perf_result_file = filename_start + 1; } } @@ -858,10 +858,10 @@ TEST(IsacFixTest, Kenny) { // Record the results with Perf test tools. webrtc::test::PrintResult("isac", "", "time_per_10ms_frame", - (runtime * 10000) / length_file, "us", false); + (runtime * 10) / length_file, "ms", false); - if (chartjson_result_file) { - EXPECT_TRUE(webrtc::test::WritePerfResults(chartjson_result_file)); + if (perf_result_file) { + EXPECT_TRUE(webrtc::test::WritePerfResults(perf_result_file)); } fclose(inp); diff --git a/test/testsupport/perf_test.cc b/test/testsupport/perf_test.cc index ff0f0d9b6b..5b2f7a0479 100644 --- a/test/testsupport/perf_test.cc +++ b/test/testsupport/perf_test.cc @@ -24,7 +24,7 @@ ABSL_FLAG(bool, write_histogram_proto_json, - false, + true, "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 " diff --git a/test/testsupport/perf_test_unittest.cc b/test/testsupport/perf_test_unittest.cc index 1004c6495e..d99014bf80 100644 --- a/test/testsupport/perf_test_unittest.cc +++ b/test/testsupport/perf_test_unittest.cc @@ -97,6 +97,9 @@ TEST_F(PerfTest, MAYBE_TestPrintResult) { } TEST_F(PerfTest, TestGetPerfResultsJSON) { + bool original_flag = absl::GetFlag(FLAGS_write_histogram_proto_json); + absl::SetFlag(&FLAGS_write_histogram_proto_json, false); + PrintResult("measurement", "modifier", "trace", 42, "units", false); PrintResult("foo", "bar", "baz_v", 7, "widgets", true); PrintResultMeanAndError("foo", "bar", "baz_me", 1, 2, "lemurs", false); @@ -104,19 +107,19 @@ TEST_F(PerfTest, TestGetPerfResultsJSON) { PrintResultList("foo", "bar", "baz_vl", kListOfScalars, "units", false); EXPECT_EQ(RemoveSpaces(kJsonExpected), GetPerfResults()); + + absl::SetFlag(&FLAGS_write_histogram_proto_json, original_flag); } TEST_F(PerfTest, TestClearPerfResults) { PrintResult("measurement", "modifier", "trace", 42, "units", false); ClearPerfResults(); - EXPECT_EQ(R"({"format_version":"1.0","charts":{}})", GetPerfResults()); + EXPECT_EQ("", GetPerfResults()); } #if WEBRTC_ENABLE_PROTOBUF TEST_F(PerfTest, TestGetPerfResultsHistograms) { - bool original_flag = absl::GetFlag(FLAGS_write_histogram_proto_json); - absl::SetFlag(&FLAGS_write_histogram_proto_json, true); PrintResult("measurement", "_modifier", "story_1", 42, "ms", false); PrintResult("foo", "bar", "story_1", 7, "sigma", true); // Note: the error will be ignored, not supported by histograms. @@ -153,8 +156,6 @@ TEST_F(PerfTest, TestGetPerfResultsHistograms) { EXPECT_EQ(hist2.name(), "measurement_modifier"); EXPECT_EQ(hist2.unit().unit(), proto::MS_BEST_FIT_FORMAT); - - absl::SetFlag(&FLAGS_write_histogram_proto_json, original_flag); } TEST_F(PerfTest, TestClearPerfResultsHistograms) {