From 21868ea52548cbe66ee66c70e86a71165096b6ee Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Wed, 17 Oct 2018 20:11:05 +0300 Subject: [PATCH 1/3] Aggregates: use non-aggregate count as iteration count. It is incorrect to say that an aggregate is computed over run's iterations, because those iterations already got averaged. Similarly, if there are N repetitions with 1 iterations each, an aggregate will be computed over N measurements, not 1. Thus it is best to simply use the count of separate reports. Fixes #586. --- src/statistics.cc | 12 +-- test/output_test_helper.cc | 8 +- test/reporter_output_test.cc | 123 +++++++++++++++++---------- test/user_counters_thousands_test.cc | 12 +-- 4 files changed, 95 insertions(+), 60 deletions(-) diff --git a/src/statistics.cc b/src/statistics.cc index 8c90a44f01..e705d6df2a 100644 --- a/src/statistics.cc +++ b/src/statistics.cc @@ -95,9 +95,6 @@ std::vector ComputeStats( real_accumulated_time_stat.reserve(reports.size()); cpu_accumulated_time_stat.reserve(reports.size()); - // All repetitions should be run with the same number of iterations so we - // can take this information from the first benchmark. - int64_t const run_iterations = reports.front().iterations; // create stats for user counters struct CounterStat { Counter c; @@ -120,7 +117,6 @@ std::vector ComputeStats( // Populate the accumulators. for (Run const& run : reports) { CHECK_EQ(reports[0].benchmark_name(), run.benchmark_name()); - CHECK_EQ(run_iterations, run.iterations); if (run.error_occurred) continue; real_accumulated_time_stat.emplace_back(run.real_accumulated_time); cpu_accumulated_time_stat.emplace_back(run.cpu_accumulated_time); @@ -148,7 +144,13 @@ std::vector ComputeStats( data.run_type = BenchmarkReporter::Run::RT_Aggregate; data.aggregate_name = Stat.name_; data.report_label = report_label; - data.iterations = run_iterations; + + // It is incorrect to say that an aggregate is computed over + // run's iterations, because those iterations already got averaged. + // Similarly, if there are N repetitions with 1 iterations each, + // an aggregate will be computed over N measurements, not 1. + // Thus it is best to simply use the count of separate reports. + data.iterations = reports.size(); data.real_accumulated_time = Stat.compute_(real_accumulated_time_stat); data.cpu_accumulated_time = Stat.compute_(cpu_accumulated_time_stat); diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index f84bd34521..f2da752e1a 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -44,9 +44,11 @@ SubMap& GetSubstitutions() { {"%hrfloat", "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?[kMGTPEZYmunpfazy]?"}, {"%int", "[ ]*[0-9]+"}, {" %s ", "[ ]+"}, - {"%time", "[ ]*[0-9]{1,6} ns"}, - {"%console_report", "[ ]*[0-9]{1,6} ns [ ]*[0-9]{1,6} ns [ ]*[0-9]+"}, - {"%console_us_report", "[ ]*[0-9] us [ ]*[0-9] us [ ]*[0-9]+"}, + {"%time", "[ ]*[0-9]+ ns"}, + {"%console_report", "[ ]*[0-9]+ ns [ ]*[0-9]+ ns [ ]*[0-9]+"}, + {"%console_time_only_report", "[ ]*[0-9]+ ns [ ]*[0-9]+ ns"}, + {"%console_us_report", "[ ]*[0-9]+ us [ ]*[0-9]+ us [ ]*[0-9]+"}, + {"%console_us_time_only_report", "[ ]*[0-9]+ us [ ]*[0-9]+ us"}, {"%csv_header", "name,iterations,real_time,cpu_time,time_unit,bytes_per_second," "items_per_second,label,error_occurred,error_message"}, diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index 50c1f758ae..1f68593c3f 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -239,11 +239,12 @@ void BM_Repeat(benchmark::State& state) { } // need two repetitions min to be able to output any aggregate output BENCHMARK(BM_Repeat)->Repetitions(2); -ADD_CASES(TC_ConsoleOut, {{"^BM_Repeat/repeats:2 %console_report$"}, - {"^BM_Repeat/repeats:2 %console_report$"}, - {"^BM_Repeat/repeats:2_mean %console_report$"}, - {"^BM_Repeat/repeats:2_median %console_report$"}, - {"^BM_Repeat/repeats:2_stddev %console_report$"}}); +ADD_CASES(TC_ConsoleOut, + {{"^BM_Repeat/repeats:2 %console_report$"}, + {"^BM_Repeat/repeats:2 %console_report$"}, + {"^BM_Repeat/repeats:2_mean %console_time_only_report [ ]*2$"}, + {"^BM_Repeat/repeats:2_median %console_time_only_report [ ]*2$"}, + {"^BM_Repeat/repeats:2_stddev %console_time_only_report [ ]*2$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Repeat/repeats:2\",$"}, {"\"run_name\": \"BM_Repeat/repeats:2\"", MR_Next}, {"\"run_type\": \"iteration\",$", MR_Next}, @@ -254,14 +255,17 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Repeat/repeats:2\",$"}, {"\"run_name\": \"BM_Repeat/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"mean\",$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}, {"\"name\": \"BM_Repeat/repeats:2_median\",$"}, {"\"run_name\": \"BM_Repeat/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"median\",$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}, {"\"name\": \"BM_Repeat/repeats:2_stddev\",$"}, {"\"run_name\": \"BM_Repeat/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"stddev\",$", MR_Next}}); + {"\"aggregate_name\": \"stddev\",$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Repeat/repeats:2\",%csv_report$"}, {"^\"BM_Repeat/repeats:2\",%csv_report$"}, {"^\"BM_Repeat/repeats:2_mean\",%csv_report$"}, @@ -269,12 +273,13 @@ ADD_CASES(TC_CSVOut, {{"^\"BM_Repeat/repeats:2\",%csv_report$"}, {"^\"BM_Repeat/repeats:2_stddev\",%csv_report$"}}); // but for two repetitions, mean and median is the same, so let's repeat.. BENCHMARK(BM_Repeat)->Repetitions(3); -ADD_CASES(TC_ConsoleOut, {{"^BM_Repeat/repeats:3 %console_report$"}, - {"^BM_Repeat/repeats:3 %console_report$"}, - {"^BM_Repeat/repeats:3 %console_report$"}, - {"^BM_Repeat/repeats:3_mean %console_report$"}, - {"^BM_Repeat/repeats:3_median %console_report$"}, - {"^BM_Repeat/repeats:3_stddev %console_report$"}}); +ADD_CASES(TC_ConsoleOut, + {{"^BM_Repeat/repeats:3 %console_report$"}, + {"^BM_Repeat/repeats:3 %console_report$"}, + {"^BM_Repeat/repeats:3 %console_report$"}, + {"^BM_Repeat/repeats:3_mean %console_time_only_report [ ]*3$"}, + {"^BM_Repeat/repeats:3_median %console_time_only_report [ ]*3$"}, + {"^BM_Repeat/repeats:3_stddev %console_time_only_report [ ]*3$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Repeat/repeats:3\",$"}, {"\"run_name\": \"BM_Repeat/repeats:3\",$", MR_Next}, {"\"run_type\": \"iteration\",$", MR_Next}, @@ -288,14 +293,17 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Repeat/repeats:3\",$"}, {"\"run_name\": \"BM_Repeat/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"mean\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"name\": \"BM_Repeat/repeats:3_median\",$"}, {"\"run_name\": \"BM_Repeat/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"median\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"name\": \"BM_Repeat/repeats:3_stddev\",$"}, {"\"run_name\": \"BM_Repeat/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"stddev\",$", MR_Next}}); + {"\"aggregate_name\": \"stddev\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Repeat/repeats:3\",%csv_report$"}, {"^\"BM_Repeat/repeats:3\",%csv_report$"}, {"^\"BM_Repeat/repeats:3\",%csv_report$"}, @@ -304,13 +312,14 @@ ADD_CASES(TC_CSVOut, {{"^\"BM_Repeat/repeats:3\",%csv_report$"}, {"^\"BM_Repeat/repeats:3_stddev\",%csv_report$"}}); // median differs between even/odd number of repetitions, so just to be sure BENCHMARK(BM_Repeat)->Repetitions(4); -ADD_CASES(TC_ConsoleOut, {{"^BM_Repeat/repeats:4 %console_report$"}, - {"^BM_Repeat/repeats:4 %console_report$"}, - {"^BM_Repeat/repeats:4 %console_report$"}, - {"^BM_Repeat/repeats:4 %console_report$"}, - {"^BM_Repeat/repeats:4_mean %console_report$"}, - {"^BM_Repeat/repeats:4_median %console_report$"}, - {"^BM_Repeat/repeats:4_stddev %console_report$"}}); +ADD_CASES(TC_ConsoleOut, + {{"^BM_Repeat/repeats:4 %console_report$"}, + {"^BM_Repeat/repeats:4 %console_report$"}, + {"^BM_Repeat/repeats:4 %console_report$"}, + {"^BM_Repeat/repeats:4 %console_report$"}, + {"^BM_Repeat/repeats:4_mean %console_time_only_report [ ]*4$"}, + {"^BM_Repeat/repeats:4_median %console_time_only_report [ ]*4$"}, + {"^BM_Repeat/repeats:4_stddev %console_time_only_report [ ]*4$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Repeat/repeats:4\",$"}, {"\"run_name\": \"BM_Repeat/repeats:4\",$", MR_Next}, {"\"run_type\": \"iteration\",$", MR_Next}, @@ -327,14 +336,17 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Repeat/repeats:4\",$"}, {"\"run_name\": \"BM_Repeat/repeats:4\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"mean\",$", MR_Next}, + {"\"iterations\": 4,$", MR_Next}, {"\"name\": \"BM_Repeat/repeats:4_median\",$"}, {"\"run_name\": \"BM_Repeat/repeats:4\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"median\",$", MR_Next}, + {"\"iterations\": 4,$", MR_Next}, {"\"name\": \"BM_Repeat/repeats:4_stddev\",$"}, {"\"run_name\": \"BM_Repeat/repeats:4\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"stddev\",$", MR_Next}}); + {"\"aggregate_name\": \"stddev\",$", MR_Next}, + {"\"iterations\": 4,$", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Repeat/repeats:4\",%csv_report$"}, {"^\"BM_Repeat/repeats:4\",%csv_report$"}, {"^\"BM_Repeat/repeats:4\",%csv_report$"}, @@ -362,25 +374,29 @@ void BM_SummaryRepeat(benchmark::State& state) { } } BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->ReportAggregatesOnly(); -ADD_CASES(TC_ConsoleOut, - {{".*BM_SummaryRepeat/repeats:3 ", MR_Not}, - {"^BM_SummaryRepeat/repeats:3_mean %console_report$"}, - {"^BM_SummaryRepeat/repeats:3_median %console_report$"}, - {"^BM_SummaryRepeat/repeats:3_stddev %console_report$"}}); +ADD_CASES( + TC_ConsoleOut, + {{".*BM_SummaryRepeat/repeats:3 ", MR_Not}, + {"^BM_SummaryRepeat/repeats:3_mean %console_time_only_report [ ]*3$"}, + {"^BM_SummaryRepeat/repeats:3_median %console_time_only_report [ ]*3$"}, + {"^BM_SummaryRepeat/repeats:3_stddev %console_time_only_report [ ]*3$"}}); ADD_CASES(TC_JSONOut, {{".*BM_SummaryRepeat/repeats:3 ", MR_Not}, {"\"name\": \"BM_SummaryRepeat/repeats:3_mean\",$"}, {"\"run_name\": \"BM_SummaryRepeat/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"mean\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"name\": \"BM_SummaryRepeat/repeats:3_median\",$"}, {"\"run_name\": \"BM_SummaryRepeat/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"median\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"name\": \"BM_SummaryRepeat/repeats:3_stddev\",$"}, {"\"run_name\": \"BM_SummaryRepeat/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"stddev\",$", MR_Next}}); + {"\"aggregate_name\": \"stddev\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}}); ADD_CASES(TC_CSVOut, {{".*BM_SummaryRepeat/repeats:3 ", MR_Not}, {"^\"BM_SummaryRepeat/repeats:3_mean\",%csv_report$"}, {"^\"BM_SummaryRepeat/repeats:3_median\",%csv_report$"}, @@ -394,25 +410,29 @@ void BM_SummaryDisplay(benchmark::State& state) { } } BENCHMARK(BM_SummaryDisplay)->Repetitions(2)->DisplayAggregatesOnly(); -ADD_CASES(TC_ConsoleOut, - {{".*BM_SummaryDisplay/repeats:2 ", MR_Not}, - {"^BM_SummaryDisplay/repeats:2_mean %console_report$"}, - {"^BM_SummaryDisplay/repeats:2_median %console_report$"}, - {"^BM_SummaryDisplay/repeats:2_stddev %console_report$"}}); +ADD_CASES( + TC_ConsoleOut, + {{".*BM_SummaryDisplay/repeats:2 ", MR_Not}, + {"^BM_SummaryDisplay/repeats:2_mean %console_time_only_report [ ]*2$"}, + {"^BM_SummaryDisplay/repeats:2_median %console_time_only_report [ ]*2$"}, + {"^BM_SummaryDisplay/repeats:2_stddev %console_time_only_report [ ]*2$"}}); ADD_CASES(TC_JSONOut, {{".*BM_SummaryDisplay/repeats:2 ", MR_Not}, {"\"name\": \"BM_SummaryDisplay/repeats:2_mean\",$"}, {"\"run_name\": \"BM_SummaryDisplay/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"mean\",$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}, {"\"name\": \"BM_SummaryDisplay/repeats:2_median\",$"}, {"\"run_name\": \"BM_SummaryDisplay/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"median\",$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}, {"\"name\": \"BM_SummaryDisplay/repeats:2_stddev\",$"}, {"\"run_name\": \"BM_SummaryDisplay/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"stddev\",$", MR_Next}}); + {"\"aggregate_name\": \"stddev\",$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}}); ADD_CASES(TC_CSVOut, {{".*BM_SummaryDisplay/repeats:2 ", MR_Not}, {"^\"BM_SummaryDisplay/repeats:2_mean\",%csv_report$"}, @@ -428,27 +448,33 @@ BENCHMARK(BM_RepeatTimeUnit) ->Repetitions(3) ->ReportAggregatesOnly() ->Unit(benchmark::kMicrosecond); -ADD_CASES(TC_ConsoleOut, - {{".*BM_RepeatTimeUnit/repeats:3 ", MR_Not}, - {"^BM_RepeatTimeUnit/repeats:3_mean %console_us_report$"}, - {"^BM_RepeatTimeUnit/repeats:3_median %console_us_report$"}, - {"^BM_RepeatTimeUnit/repeats:3_stddev %console_us_report$"}}); +ADD_CASES( + TC_ConsoleOut, + {{".*BM_RepeatTimeUnit/repeats:3 ", MR_Not}, + {"^BM_RepeatTimeUnit/repeats:3_mean %console_us_time_only_report [ ]*3$"}, + {"^BM_RepeatTimeUnit/repeats:3_median %console_us_time_only_report [ " + "]*3$"}, + {"^BM_RepeatTimeUnit/repeats:3_stddev %console_us_time_only_report [ " + "]*3$"}}); ADD_CASES(TC_JSONOut, {{".*BM_RepeatTimeUnit/repeats:3 ", MR_Not}, {"\"name\": \"BM_RepeatTimeUnit/repeats:3_mean\",$"}, {"\"run_name\": \"BM_RepeatTimeUnit/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"mean\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"time_unit\": \"us\",?$"}, {"\"name\": \"BM_RepeatTimeUnit/repeats:3_median\",$"}, {"\"run_name\": \"BM_RepeatTimeUnit/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"median\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"time_unit\": \"us\",?$"}, {"\"name\": \"BM_RepeatTimeUnit/repeats:3_stddev\",$"}, {"\"run_name\": \"BM_RepeatTimeUnit/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"stddev\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"time_unit\": \"us\",?$"}}); ADD_CASES(TC_CSVOut, {{".*BM_RepeatTimeUnit/repeats:3 ", MR_Not}, @@ -475,13 +501,14 @@ BENCHMARK(BM_UserStats) // check that user-provided stats is calculated, and is after the default-ones // empty string as name is intentional, it would sort before anything else -ADD_CASES(TC_ConsoleOut, {{"^BM_UserStats/repeats:3 %console_report$"}, - {"^BM_UserStats/repeats:3 %console_report$"}, - {"^BM_UserStats/repeats:3 %console_report$"}, - {"^BM_UserStats/repeats:3_mean %console_report$"}, - {"^BM_UserStats/repeats:3_median %console_report$"}, - {"^BM_UserStats/repeats:3_stddev %console_report$"}, - {"^BM_UserStats/repeats:3_ %console_report$"}}); +ADD_CASES(TC_ConsoleOut, + {{"^BM_UserStats/repeats:3 %console_report$"}, + {"^BM_UserStats/repeats:3 %console_report$"}, + {"^BM_UserStats/repeats:3 %console_report$"}, + {"^BM_UserStats/repeats:3_mean %console_time_only_report [ ]*3$"}, + {"^BM_UserStats/repeats:3_median %console_time_only_report [ ]*3$"}, + {"^BM_UserStats/repeats:3_stddev %console_time_only_report [ ]*3$"}, + {"^BM_UserStats/repeats:3_ %console_report$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_UserStats/repeats:3\",$"}, {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, {"\"run_type\": \"iteration\",$", MR_Next}, @@ -495,18 +522,22 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_UserStats/repeats:3\",$"}, {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"mean\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"name\": \"BM_UserStats/repeats:3_median\",$"}, {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"median\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"name\": \"BM_UserStats/repeats:3_stddev\",$"}, {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"stddev\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, {"\"name\": \"BM_UserStats/repeats:3_\",$"}, {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"\",$", MR_Next}}); + {"\"aggregate_name\": \"\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_UserStats/repeats:3\",%csv_report$"}, {"^\"BM_UserStats/repeats:3\",%csv_report$"}, {"^\"BM_UserStats/repeats:3\",%csv_report$"}, diff --git a/test/user_counters_thousands_test.cc b/test/user_counters_thousands_test.cc index 2c64dc4207..fa0ef97204 100644 --- a/test/user_counters_thousands_test.cc +++ b/test/user_counters_thousands_test.cc @@ -45,9 +45,9 @@ ADD_CASES( "t0_1000000DefaultBase=1000k t1_1000000Base1000=1000k " "t2_1000000Base1024=976.56[23]k t3_1048576Base1000=1048.58k " "t4_1048576Base1024=1024k$"}, - {"^BM_Counters_Thousands/repeats:2_stddev %console_report " - "t0_1000000DefaultBase=0 t1_1000000Base1000=0 t2_1000000Base1024=0 " - "t3_1048576Base1000=0 t4_1048576Base1024=0$"}, + {"^BM_Counters_Thousands/repeats:2_stddev %console_time_only_report [ " + "]*2 t0_1000000DefaultBase=0 t1_1000000Base1000=0 " + "t2_1000000Base1024=0 t3_1048576Base1000=0 t4_1048576Base1024=0$"}, }); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Thousands/repeats:2\",$"}, @@ -82,7 +82,7 @@ ADD_CASES(TC_JSONOut, {"\"run_name\": \"BM_Counters_Thousands/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"mean\",$", MR_Next}, - {"\"iterations\": %int,$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}, {"\"real_time\": %float,$", MR_Next}, {"\"cpu_time\": %float,$", MR_Next}, {"\"time_unit\": \"ns\",$", MR_Next}, @@ -97,7 +97,7 @@ ADD_CASES(TC_JSONOut, {"\"run_name\": \"BM_Counters_Thousands/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"median\",$", MR_Next}, - {"\"iterations\": %int,$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}, {"\"real_time\": %float,$", MR_Next}, {"\"cpu_time\": %float,$", MR_Next}, {"\"time_unit\": \"ns\",$", MR_Next}, @@ -112,7 +112,7 @@ ADD_CASES(TC_JSONOut, {"\"run_name\": \"BM_Counters_Thousands/repeats:2\",$", MR_Next}, {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"stddev\",$", MR_Next}, - {"\"iterations\": %int,$", MR_Next}, + {"\"iterations\": 2,$", MR_Next}, {"\"real_time\": %float,$", MR_Next}, {"\"cpu_time\": %float,$", MR_Next}, {"\"time_unit\": \"ns\",$", MR_Next}, From 28de645bf32cd8745ad0978924bddb45fbf607dc Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Thu, 18 Oct 2018 16:02:35 +0300 Subject: [PATCH 2/3] ComputeStats(): actually properly adjust for changed 'iteration' count. This is a rather huge mess. What is worse, the custom counters are already divided by iteration count by now. And i'm not sure it is in general correct to compute the stats over sum(iterations), and then divide the stat by iteration count. I think we should cleanup everything to divide by iteration count as early as possible. --- src/statistics.cc | 16 +++++ test/reporter_output_test.cc | 120 ++++++++++++++++++++++------------- 2 files changed, 92 insertions(+), 44 deletions(-) diff --git a/src/statistics.cc b/src/statistics.cc index e705d6df2a..e821aec18b 100644 --- a/src/statistics.cc +++ b/src/statistics.cc @@ -95,6 +95,9 @@ std::vector ComputeStats( real_accumulated_time_stat.reserve(reports.size()); cpu_accumulated_time_stat.reserve(reports.size()); + // All repetitions should be run with the same number of iterations so we + // can take this information from the first benchmark. + int64_t const run_iterations = reports.front().iterations; // create stats for user counters struct CounterStat { Counter c; @@ -117,6 +120,7 @@ std::vector ComputeStats( // Populate the accumulators. for (Run const& run : reports) { CHECK_EQ(reports[0].benchmark_name(), run.benchmark_name()); + CHECK_EQ(run_iterations, run.iterations); if (run.error_occurred) continue; real_accumulated_time_stat.emplace_back(run.real_accumulated_time); cpu_accumulated_time_stat.emplace_back(run.cpu_accumulated_time); @@ -137,6 +141,9 @@ std::vector ComputeStats( } } + const double iteration_rescale_factor = + double(reports.size()) / double(run_iterations); + for (const auto& Stat : *reports[0].statistics) { // Get the data from the accumulator to BenchmarkReporter::Run's. Run data; @@ -155,10 +162,19 @@ std::vector ComputeStats( data.real_accumulated_time = Stat.compute_(real_accumulated_time_stat); data.cpu_accumulated_time = Stat.compute_(cpu_accumulated_time_stat); + // We will divide these times by data.iterations when reporting, but the + // data.iterations is not nessesairly the scale of these measurements, + // because in each repetition, these timers are sum over all the iterations. + // And if we want to say that the stats are over N repetitions and not + // M iterations, we need to multiply these by (N/M). + data.real_accumulated_time *= iteration_rescale_factor; + data.cpu_accumulated_time *= iteration_rescale_factor; + data.time_unit = reports[0].time_unit; // user counters for (auto const& kv : counter_stats) { + // Do *NOT* rescale the custom counters. They are already properly scaled. const auto uc_stat = Stat.compute_(kv.second.s); auto c = Counter(uc_stat, counter_stats[kv.first].c.flags, counter_stats[kv.first].c.oneK); diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index 1f68593c3f..126352afb1 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -491,60 +491,92 @@ const auto UserStatistics = [](const std::vector& v) { }; void BM_UserStats(benchmark::State& state) { for (auto _ : state) { + state.SetIterationTime(150 / 10e8); } } // clang-format off BENCHMARK(BM_UserStats) ->Repetitions(3) + ->Iterations(5) + ->UseManualTime() ->ComputeStatistics("", UserStatistics); // clang-format on // check that user-provided stats is calculated, and is after the default-ones // empty string as name is intentional, it would sort before anything else -ADD_CASES(TC_ConsoleOut, - {{"^BM_UserStats/repeats:3 %console_report$"}, - {"^BM_UserStats/repeats:3 %console_report$"}, - {"^BM_UserStats/repeats:3 %console_report$"}, - {"^BM_UserStats/repeats:3_mean %console_time_only_report [ ]*3$"}, - {"^BM_UserStats/repeats:3_median %console_time_only_report [ ]*3$"}, - {"^BM_UserStats/repeats:3_stddev %console_time_only_report [ ]*3$"}, - {"^BM_UserStats/repeats:3_ %console_report$"}}); -ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_UserStats/repeats:3\",$"}, - {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, - {"\"run_type\": \"iteration\",$", MR_Next}, - {"\"name\": \"BM_UserStats/repeats:3\",$"}, - {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, - {"\"run_type\": \"iteration\",$", MR_Next}, - {"\"name\": \"BM_UserStats/repeats:3\",$"}, - {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, - {"\"run_type\": \"iteration\",$", MR_Next}, - {"\"name\": \"BM_UserStats/repeats:3_mean\",$"}, - {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, - {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"mean\",$", MR_Next}, - {"\"iterations\": 3,$", MR_Next}, - {"\"name\": \"BM_UserStats/repeats:3_median\",$"}, - {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, - {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"median\",$", MR_Next}, - {"\"iterations\": 3,$", MR_Next}, - {"\"name\": \"BM_UserStats/repeats:3_stddev\",$"}, - {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, - {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"stddev\",$", MR_Next}, - {"\"iterations\": 3,$", MR_Next}, - {"\"name\": \"BM_UserStats/repeats:3_\",$"}, - {"\"run_name\": \"BM_UserStats/repeats:3\",$", MR_Next}, - {"\"run_type\": \"aggregate\",$", MR_Next}, - {"\"aggregate_name\": \"\",$", MR_Next}, - {"\"iterations\": 3,$", MR_Next}}); -ADD_CASES(TC_CSVOut, {{"^\"BM_UserStats/repeats:3\",%csv_report$"}, - {"^\"BM_UserStats/repeats:3\",%csv_report$"}, - {"^\"BM_UserStats/repeats:3\",%csv_report$"}, - {"^\"BM_UserStats/repeats:3_mean\",%csv_report$"}, - {"^\"BM_UserStats/repeats:3_median\",%csv_report$"}, - {"^\"BM_UserStats/repeats:3_stddev\",%csv_report$"}, - {"^\"BM_UserStats/repeats:3_\",%csv_report$"}}); +ADD_CASES(TC_ConsoleOut, {{"^BM_UserStats/iterations:5/repeats:3/manual_time [ " + "]* 150 ns %time [ ]*5$"}, + {"^BM_UserStats/iterations:5/repeats:3/manual_time [ " + "]* 150 ns %time [ ]*5$"}, + {"^BM_UserStats/iterations:5/repeats:3/manual_time [ " + "]* 150 ns %time [ ]*5$"}, + {"^BM_UserStats/iterations:5/repeats:3/" + "manual_time_mean [ ]* 150 ns %time [ ]*3$"}, + {"^BM_UserStats/iterations:5/repeats:3/" + "manual_time_median [ ]* 150 ns %time [ ]*3$"}, + {"^BM_UserStats/iterations:5/repeats:3/" + "manual_time_stddev [ ]* 0 ns %time [ ]*3$"}, + {"^BM_UserStats/iterations:5/repeats:3/manual_time_ " + "[ ]* 150 ns %time [ ]*3$"}}); +ADD_CASES( + TC_JSONOut, + {{"\"name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$"}, + {"\"run_name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$", + MR_Next}, + {"\"run_type\": \"iteration\",$", MR_Next}, + {"\"iterations\": 5,$", MR_Next}, + {"\"real_time\": 1\\.5(0)*e\\+(0)*2,$", MR_Next}, + {"\"name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$"}, + {"\"run_name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$", + MR_Next}, + {"\"run_type\": \"iteration\",$", MR_Next}, + {"\"iterations\": 5,$", MR_Next}, + {"\"real_time\": 1\\.5(0)*e\\+(0)*2,$", MR_Next}, + {"\"name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$"}, + {"\"run_name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$", + MR_Next}, + {"\"run_type\": \"iteration\",$", MR_Next}, + {"\"iterations\": 5,$", MR_Next}, + {"\"real_time\": 1\\.5(0)*e\\+(0)*2,$", MR_Next}, + {"\"name\": \"BM_UserStats/iterations:5/repeats:3/manual_time_mean\",$"}, + {"\"run_name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$", + MR_Next}, + {"\"run_type\": \"aggregate\",$", MR_Next}, + {"\"aggregate_name\": \"mean\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, + {"\"real_time\": 1\\.5(0)*e\\+(0)*2,$", MR_Next}, + {"\"name\": \"BM_UserStats/iterations:5/repeats:3/manual_time_median\",$"}, + {"\"run_name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$", + MR_Next}, + {"\"run_type\": \"aggregate\",$", MR_Next}, + {"\"aggregate_name\": \"median\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, + {"\"real_time\": 1\\.5(0)*e\\+(0)*2,$", MR_Next}, + {"\"name\": \"BM_UserStats/iterations:5/repeats:3/manual_time_stddev\",$"}, + {"\"run_name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$", + MR_Next}, + {"\"run_type\": \"aggregate\",$", MR_Next}, + {"\"aggregate_name\": \"stddev\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, + {"\"real_time\": (0)*\\.(0)*e\\+(0)*,$", MR_Next}, + {"\"name\": \"BM_UserStats/iterations:5/repeats:3/manual_time_\",$"}, + {"\"run_name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$", + MR_Next}, + {"\"run_type\": \"aggregate\",$", MR_Next}, + {"\"aggregate_name\": \"\",$", MR_Next}, + {"\"iterations\": 3,$", MR_Next}, + {"\"real_time\": 1\\.5(0)*e\\+(0)*2,$", MR_Next}}); +ADD_CASES( + TC_CSVOut, + {{"^\"BM_UserStats/iterations:5/repeats:3/manual_time\",%csv_report$"}, + {"^\"BM_UserStats/iterations:5/repeats:3/manual_time\",%csv_report$"}, + {"^\"BM_UserStats/iterations:5/repeats:3/manual_time\",%csv_report$"}, + {"^\"BM_UserStats/iterations:5/repeats:3/manual_time_mean\",%csv_report$"}, + {"^\"BM_UserStats/iterations:5/repeats:3/" + "manual_time_median\",%csv_report$"}, + {"^\"BM_UserStats/iterations:5/repeats:3/" + "manual_time_stddev\",%csv_report$"}, + {"^\"BM_UserStats/iterations:5/repeats:3/manual_time_\",%csv_report$"}}); // ========================================================================= // // --------------------------- TEST CASES END ------------------------------ // From 769491bd6a3a40f2b76ed4b7f26cd1e293585605 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Thu, 18 Oct 2018 16:58:01 +0300 Subject: [PATCH 3/3] Don't be so sure that stddev in doubles is exactly zero. --- test/reporter_output_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index 126352afb1..80314b3431 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -558,7 +558,7 @@ ADD_CASES( {"\"run_type\": \"aggregate\",$", MR_Next}, {"\"aggregate_name\": \"stddev\",$", MR_Next}, {"\"iterations\": 3,$", MR_Next}, - {"\"real_time\": (0)*\\.(0)*e\\+(0)*,$", MR_Next}, + {"\"real_time\": %float,$", MR_Next}, {"\"name\": \"BM_UserStats/iterations:5/repeats:3/manual_time_\",$"}, {"\"run_name\": \"BM_UserStats/iterations:5/repeats:3/manual_time\",$", MR_Next},