diff options
author | Jesse Rosenstock <jmr@google.com> | 2023-08-21 16:35:42 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-08-21 15:35:42 +0100 |
commit | e73915667c21faccd7019c6da8ab083b0264db13 (patch) | |
tree | 6d64d56aee89942389a85364ae87ff1d6c45fd81 | |
parent | e441a8cb112a3c13629749ec8c3d65b2d170b10e (diff) | |
download | google-benchmark-e73915667c21faccd7019c6da8ab083b0264db13.tar.gz |
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>
-rw-r--r-- | src/benchmark.cc | 19 | ||||
-rw-r--r-- | test/perf_counters_test.cc | 17 |
2 files changed, 22 insertions, 14 deletions
diff --git a/src/benchmark.cc b/src/benchmark.cc index 974cde6..6139e59 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -179,6 +179,17 @@ State::State(std::string name, IterationCount max_iters, BM_CHECK_LT(thread_index_, 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 // because State is not a standard layout type. However, all compilers // 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."; } for (const auto& name_and_measurement : measurements) { - auto name = name_and_measurement.first; - auto measurement = name_and_measurement.second; - counters[name] += Counter(measurement, Counter::kAvgIterations); + const std::string& name = name_and_measurement.first; + const double measurement = name_and_measurement.second; + // Counter was inserted with `kAvgIterations` flag by the constructor. + assert(counters.find(name) != counters.end()); + counters[name].value += measurement; } } } diff --git a/test/perf_counters_test.cc b/test/perf_counters_test.cc index f2ef9be..b0a3ab0 100644 --- a/test/perf_counters_test.cc +++ b/test/perf_counters_test.cc @@ -66,22 +66,17 @@ static void CheckSimple(Results const& e) { double withoutPauseResumeInstrCount = 0.0; double withPauseResumeInstrCount = 0.0; -static void CheckInstrCount(double* counter, Results const& e) { - BM_CHECK_GT(e.NumIterations(), 0); - *counter = e.GetAs<double>("INSTRUCTIONS") / e.NumIterations(); +static void SaveInstrCountWithoutResume(Results const& e) { + withoutPauseResumeInstrCount = e.GetAs<double>("INSTRUCTIONS"); } -static void CheckInstrCountWithoutResume(Results const& e) { - CheckInstrCount(&withoutPauseResumeInstrCount, e); -} - -static void CheckInstrCountWithResume(Results const& e) { - CheckInstrCount(&withPauseResumeInstrCount, e); +static void SaveInstrCountWithResume(Results const& e) { + withPauseResumeInstrCount = e.GetAs<double>("INSTRUCTIONS"); } CHECK_BENCHMARK_RESULTS("BM_Simple", &CheckSimple); -CHECK_BENCHMARK_RESULTS("BM_WithoutPauseResume", &CheckInstrCountWithoutResume); -CHECK_BENCHMARK_RESULTS("BM_WithPauseResume", &CheckInstrCountWithResume); +CHECK_BENCHMARK_RESULTS("BM_WithoutPauseResume", &SaveInstrCountWithoutResume); +CHECK_BENCHMARK_RESULTS("BM_WithPauseResume", &SaveInstrCountWithResume); int main(int argc, char* argv[]) { if (!benchmark::internal::PerfCounters::kSupported) { |