Skip to content

Commit 507c06e

Browse files
authored
Aggregates: use non-aggregate count as iteration count. (#706)
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.
1 parent 99d1356 commit 507c06e

4 files changed

Lines changed: 170 additions & 87 deletions

File tree

src/statistics.cc

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,22 +141,40 @@ std::vector<BenchmarkReporter::Run> ComputeStats(
141141
}
142142
}
143143

144+
const double iteration_rescale_factor =
145+
double(reports.size()) / double(run_iterations);
146+
144147
for (const auto& Stat : *reports[0].statistics) {
145148
// Get the data from the accumulator to BenchmarkReporter::Run's.
146149
Run data;
147150
data.run_name = reports[0].benchmark_name();
148151
data.run_type = BenchmarkReporter::Run::RT_Aggregate;
149152
data.aggregate_name = Stat.name_;
150153
data.report_label = report_label;
151-
data.iterations = run_iterations;
154+
155+
// It is incorrect to say that an aggregate is computed over
156+
// run's iterations, because those iterations already got averaged.
157+
// Similarly, if there are N repetitions with 1 iterations each,
158+
// an aggregate will be computed over N measurements, not 1.
159+
// Thus it is best to simply use the count of separate reports.
160+
data.iterations = reports.size();
152161

153162
data.real_accumulated_time = Stat.compute_(real_accumulated_time_stat);
154163
data.cpu_accumulated_time = Stat.compute_(cpu_accumulated_time_stat);
155164

165+
// We will divide these times by data.iterations when reporting, but the
166+
// data.iterations is not nessesairly the scale of these measurements,
167+
// because in each repetition, these timers are sum over all the iterations.
168+
// And if we want to say that the stats are over N repetitions and not
169+
// M iterations, we need to multiply these by (N/M).
170+
data.real_accumulated_time *= iteration_rescale_factor;
171+
data.cpu_accumulated_time *= iteration_rescale_factor;
172+
156173
data.time_unit = reports[0].time_unit;
157174

158175
// user counters
159176
for (auto const& kv : counter_stats) {
177+
// Do *NOT* rescale the custom counters. They are already properly scaled.
160178
const auto uc_stat = Stat.compute_(kv.second.s);
161179
auto c = Counter(uc_stat, counter_stats[kv.first].c.flags,
162180
counter_stats[kv.first].c.oneK);

test/output_test_helper.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@ SubMap& GetSubstitutions() {
4444
{"%hrfloat", "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?[kMGTPEZYmunpfazy]?"},
4545
{"%int", "[ ]*[0-9]+"},
4646
{" %s ", "[ ]+"},
47-
{"%time", "[ ]*[0-9]{1,6} ns"},
48-
{"%console_report", "[ ]*[0-9]{1,6} ns [ ]*[0-9]{1,6} ns [ ]*[0-9]+"},
49-
{"%console_us_report", "[ ]*[0-9] us [ ]*[0-9] us [ ]*[0-9]+"},
47+
{"%time", "[ ]*[0-9]+ ns"},
48+
{"%console_report", "[ ]*[0-9]+ ns [ ]*[0-9]+ ns [ ]*[0-9]+"},
49+
{"%console_time_only_report", "[ ]*[0-9]+ ns [ ]*[0-9]+ ns"},
50+
{"%console_us_report", "[ ]*[0-9]+ us [ ]*[0-9]+ us [ ]*[0-9]+"},
51+
{"%console_us_time_only_report", "[ ]*[0-9]+ us [ ]*[0-9]+ us"},
5052
{"%csv_header",
5153
"name,iterations,real_time,cpu_time,time_unit,bytes_per_second,"
5254
"items_per_second,label,error_occurred,error_message"},

0 commit comments

Comments
 (0)