From 3924ee7b8a6c6427083662c861c0f51be2a38bd9 Mon Sep 17 00:00:00 2001 From: Samuel Panzer Date: Tue, 13 Feb 2018 15:54:46 -0500 Subject: [PATCH] Fixups following addition of KeepRunningBatch (296ec5693) (#526) * Support State::KeepRunningBatch(). State::KeepRunning() can take large amounts of time relative to quick operations (on the order of 1ns, depending on hardware). For such sensitive operations, it is recommended to run batches of repeated operations. This commit simplifies handling of total_iterations_. Rather than predecrementing such that total_iterations_ == 1 signals that KeepRunning() should exit, total_iterations_ == 0 now signals the intention for the benchmark to exit. * Create better fast path in State::KeepRunningBatch() * Replace int parameter with size_t to fix signed mismatch warnings * Ensure benchmark State has been started even on error. * Simplify KeepRunningBatch() * Implement KeepRunning() in terms of KeepRunningBatch(). * Improve codegen by helping the compiler undestand dead code. * Dummy commit for build bots' benefit. --- include/benchmark/benchmark.h | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/include/benchmark/benchmark.h b/include/benchmark/benchmark.h index a7ac9649..1825054b 100644 --- a/include/benchmark/benchmark.h +++ b/include/benchmark/benchmark.h @@ -433,6 +433,7 @@ class State { bool KeepRunning(); // Returns true iff the benchmark should run n more iterations. + // REQUIRES: 'n' > 0. // NOTE: A benchmark must not return from the test until KeepRunningBatch() // has returned false. // NOTE: KeepRunningBatch() may overshoot by up to 'n' iterations. @@ -609,6 +610,9 @@ class State { private: void StartKeepRunning(); + // Implementation of KeepRunning() and KeepRunningBatch(). + // is_batch must be true unless n is 1. + bool KeepRunningInternal(size_t n, bool is_batch); void FinishKeepRunning(); internal::ThreadTimer* timer_; internal::ThreadManager* manager_; @@ -617,28 +621,21 @@ class State { inline BENCHMARK_ALWAYS_INLINE bool State::KeepRunning() { - // total_iterations_ is set to 0 by the constructor, and always set to a - // nonzero value by StartKepRunning(). - if (BENCHMARK_BUILTIN_EXPECT(total_iterations_ != 0, true)) { - --total_iterations_; - return true; - } - if (!started_) { - StartKeepRunning(); - if (!error_occurred_) { - // max_iterations > 0. The first iteration is always valid. - --total_iterations_; - return true; - } - } - FinishKeepRunning(); - return false; + return KeepRunningInternal(1, /*is_batch=*/ false); } inline BENCHMARK_ALWAYS_INLINE bool State::KeepRunningBatch(size_t n) { + return KeepRunningInternal(n, /*is_batch=*/ true); +} + +inline BENCHMARK_ALWAYS_INLINE +bool State::KeepRunningInternal(size_t n, bool is_batch) { // total_iterations_ is set to 0 by the constructor, and always set to a // nonzero value by StartKepRunning(). + assert(n > 0); + // n must be 1 unless is_batch is true. + assert(is_batch || n == 1); if (BENCHMARK_BUILTIN_EXPECT(total_iterations_ >= n, true)) { total_iterations_ -= n; return true; @@ -650,7 +647,8 @@ bool State::KeepRunningBatch(size_t n) { return true; } } - if (total_iterations_ != 0) { + // For non-batch runs, total_iterations_ must be 0 by now. + if (is_batch && total_iterations_ != 0) { batch_leftover_ = n - total_iterations_; total_iterations_ = 0; return true;