State: Initialize counters with kAvgIteration in constructor (#1652)

* State: Initialize counters with kAvgIteration in constructor

Previously, `counters` was updated in `PauseTiming()` with
`counters[name] += Counter(measurement, kAvgIteration)`.

The first `counters[name]` call inserts a counter with no flags.

There is no `operator+=` for `Counter`, so the insertion is done
by converting the `Counter` to a `double`, then constructing a
`Counter` to insert from the `double`, which drops the flags.

Pre-insert the `Counter` with the correct flags, then only
update `Counter::value`.

Introduced in 1c64a36 ([perf-counters] Fix pause/resume (#1643)).

* perf_counters_test.cc: Don't divide by iterations

Perf counters are now divided by iterations, so dividing again
in the test is wrong.

* State: Fix shadowed param error

* benchmark.cc: Fix clang-tidy error

---------

Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
This commit is contained in:
Jesse Rosenstock 2023-08-21 16:35:42 +02:00 committed by GitHub
parent e441a8cb11
commit e73915667c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 14 deletions

View File

@ -179,6 +179,17 @@ State::State(std::string name, IterationCount max_iters,
BM_CHECK_LT(thread_index_, threads_) BM_CHECK_LT(thread_index_, threads_)
<< "thread_index must be less than threads"; << "thread_index must be less than threads";
// Add counters with correct flag now. If added with `counters[name]` in
// `PauseTiming`, a new `Counter` will be inserted the first time, which
// won't have the flag. Inserting them now also reduces the allocations
// during the benchmark.
if (perf_counters_measurement_) {
for (const std::string& counter_name :
perf_counters_measurement_->names()) {
counters[counter_name] = Counter(0.0, Counter::kAvgIterations);
}
}
// Note: The use of offsetof below is technically undefined until C++17 // Note: The use of offsetof below is technically undefined until C++17
// because State is not a standard layout type. However, all compilers // because State is not a standard layout type. However, all compilers
// currently provide well-defined behavior as an extension (which is // currently provide well-defined behavior as an extension (which is
@ -227,9 +238,11 @@ void State::PauseTiming() {
BM_CHECK(false) << "Perf counters read the value failed."; BM_CHECK(false) << "Perf counters read the value failed.";
} }
for (const auto& name_and_measurement : measurements) { for (const auto& name_and_measurement : measurements) {
auto name = name_and_measurement.first; const std::string& name = name_and_measurement.first;
auto measurement = name_and_measurement.second; const double measurement = name_and_measurement.second;
counters[name] += Counter(measurement, Counter::kAvgIterations); // Counter was inserted with `kAvgIterations` flag by the constructor.
assert(counters.find(name) != counters.end());
counters[name].value += measurement;
} }
} }
} }

View File

@ -66,22 +66,17 @@ static void CheckSimple(Results const& e) {
double withoutPauseResumeInstrCount = 0.0; double withoutPauseResumeInstrCount = 0.0;
double withPauseResumeInstrCount = 0.0; double withPauseResumeInstrCount = 0.0;
static void CheckInstrCount(double* counter, Results const& e) { static void SaveInstrCountWithoutResume(Results const& e) {
BM_CHECK_GT(e.NumIterations(), 0); withoutPauseResumeInstrCount = e.GetAs<double>("INSTRUCTIONS");
*counter = e.GetAs<double>("INSTRUCTIONS") / e.NumIterations();
} }
static void CheckInstrCountWithoutResume(Results const& e) { static void SaveInstrCountWithResume(Results const& e) {
CheckInstrCount(&withoutPauseResumeInstrCount, e); withPauseResumeInstrCount = e.GetAs<double>("INSTRUCTIONS");
}
static void CheckInstrCountWithResume(Results const& e) {
CheckInstrCount(&withPauseResumeInstrCount, e);
} }
CHECK_BENCHMARK_RESULTS("BM_Simple", &CheckSimple); CHECK_BENCHMARK_RESULTS("BM_Simple", &CheckSimple);
CHECK_BENCHMARK_RESULTS("BM_WithoutPauseResume", &CheckInstrCountWithoutResume); CHECK_BENCHMARK_RESULTS("BM_WithoutPauseResume", &SaveInstrCountWithoutResume);
CHECK_BENCHMARK_RESULTS("BM_WithPauseResume", &CheckInstrCountWithResume); CHECK_BENCHMARK_RESULTS("BM_WithPauseResume", &SaveInstrCountWithResume);
int main(int argc, char* argv[]) { int main(int argc, char* argv[]) {
if (!benchmark::internal::PerfCounters::kSupported) { if (!benchmark::internal::PerfCounters::kSupported) {