From 8ae6448cc7ec6353e3491a2a15f972f9735f124b Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Fri, 10 Mar 2017 17:52:02 -0700 Subject: [PATCH 01/60] Fix std::string detection hack for SetLabel. Previously benchmark_api.h wasn't allowed to include standard library headers. For this reason SetLabel had a hack to accept std::string without including . The hack worked by attempting to detect the injected class name `basic_string`. However Clang has changed it's behavior regarding injected class names so this hack no longer works. This patch removes the hack and replaces it with a function that actually names std::string. However we still cannot pass std::string across the dylib boundary because of libstdc++'s dual C++11 ABI. --- include/benchmark/benchmark_api.h | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/include/benchmark/benchmark_api.h b/include/benchmark/benchmark_api.h index da3e7f9e..f72a64a2 100644 --- a/include/benchmark/benchmark_api.h +++ b/include/benchmark/benchmark_api.h @@ -203,19 +203,6 @@ class Benchmark; class BenchmarkImp; class BenchmarkFamilies; -template -struct Voider { - typedef void type; -}; - -template -struct EnableIfString {}; - -template -struct EnableIfString::type> { - typedef int type; -}; - void UseCharPointer(char const volatile*); // Take ownership of the pointer and register the benchmark. Return the @@ -432,13 +419,7 @@ class State { // REQUIRES: a benchmark has exited its KeepRunning loop. void SetLabel(const char* label); - // Allow the use of std::string without actually including . - // This function does not participate in overload resolution unless StringType - // has the nested typename `basic_string`. This typename should be provided - // as an injected class name in the case of std::string. - template - void SetLabel(StringType const& str, - typename internal::EnableIfString::type = 1) { + void BENCHMARK_ALWAYS_INLINE SetLabel(const std::string& str) { this->SetLabel(str.c_str()); } From f682f7e9a41a88468c72ce5f7f6abe76913b82ad Mon Sep 17 00:00:00 2001 From: Eric Date: Fri, 10 Mar 2017 18:47:39 -0700 Subject: [PATCH 02/60] Implement ClobberMemory() and fix DoNotOptimize on MSVC. (#352) I recently learned Windows provides a function called _ReadWriteBarrier which is literally ClobberMemory under a different name. This patch uses it to implement ClobberMemory under MSVC. --- README.md | 2 +- include/benchmark/benchmark_api.h | 23 +++++++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 456b0a6c..44466379 100644 --- a/README.md +++ b/README.md @@ -365,7 +365,7 @@ static void BM_vector_push_back(benchmark::State& state) { } ``` -Note that `ClobberMemory()` is only available for GNU based compilers. +Note that `ClobberMemory()` is only available for GNU or MSVC based compilers. ### Set time unit manually If a benchmark runs a few milliseconds it may be hard to visually compare the diff --git a/include/benchmark/benchmark_api.h b/include/benchmark/benchmark_api.h index f72a64a2..8cde61ba 100644 --- a/include/benchmark/benchmark_api.h +++ b/include/benchmark/benchmark_api.h @@ -165,6 +165,10 @@ BENCHMARK(BM_test)->Unit(benchmark::kMillisecond); #include #endif +#if defined(_MSC_VER) +#include // for _ReadWriteBarrier +#endif + namespace benchmark { class BenchmarkReporter; @@ -215,11 +219,16 @@ BENCHMARK_UNUSED static int stream_init_anchor = InitializeStreams(); } // end namespace internal + +#if !defined(__GNUC__) || defined(__pnacl__) || defined(EMSCRIPTN) +# define BENCHMARK_HAS_NO_INLINE_ASSEMBLY +#endif + // The DoNotOptimize(...) function can be used to prevent a value or // expression from being optimized away by the compiler. This function is // intended to add little to no overhead. // See: https://youtu.be/nXaxk27zwlk?t=2441 -#if defined(__GNUC__) && !defined(__pnacl__) && !defined(EMSCRIPTEN) +#ifndef BENCHMARK_HAS_NO_INLINE_ASSEMBLY template inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp const& value) { asm volatile("" : : "g"(value) : "memory"); @@ -229,12 +238,22 @@ inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp const& value) { inline BENCHMARK_ALWAYS_INLINE void ClobberMemory() { asm volatile("" : : : "memory"); } +#elif defined(_MSC_VER) +template +inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp const& value) { + internal::UseCharPointer(&reinterpret_cast(value)); + _ReadWriteBarrier(); +} + +inline BENCHMARK_ALWAYS_INLINE void ClobberMemory() { + _ReadWriteBarrier(); +} #else template inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp const& value) { internal::UseCharPointer(&reinterpret_cast(value)); } -// FIXME Add ClobberMemory() for non-gnu compilers +// FIXME Add ClobberMemory() for non-gnu and non-msvc compilers #endif From f5ff6d0e0d3d00cf07bb8306548b637e98b13720 Mon Sep 17 00:00:00 2001 From: Yasushi Saito Date: Mon, 13 Mar 2017 19:30:19 -0700 Subject: [PATCH 03/60] Include cstdlib for timespec. Clang modules demands that. (#353) --- src/sleep.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sleep.cc b/src/sleep.cc index 918abc48..1449339e 100644 --- a/src/sleep.cc +++ b/src/sleep.cc @@ -15,6 +15,7 @@ #include "sleep.h" #include +#include #include #include "internal_macros.h" From 9b92ed76a8620354403b5f1d089560048b6843bb Mon Sep 17 00:00:00 2001 From: rolandschulz Date: Mon, 27 Mar 2017 17:30:54 -0700 Subject: [PATCH 04/60] Fix ICC compiler warnings (#358) fixes #354 The build fails with ICC17 because of warnings and Werror. What is the correct solution to fix it? Should a patch disable Werror for ICC (or maybe all non known compilers) disable the false postive warnings for all files. This could be done using: add_cxx_compiler_flag(-wd2102) #ICC17u2: Many false positives for Wstrict-aliasing add_cxx_compiler_flag(-wd2259) #ICC17u2: non-pointer conversion from "long" to "int" may lose significant bits (even for explicit static cast, sleep.cc(44)) add_cxx_compiler_flag(-wd654) #ICC17u2: overloaded virtual function "benchmark::Fixture::SetUp" is only partially overridden (because of deprecated overload) disable warnings at file level or some other granularity --- CMakeLists.txt | 7 ++++++- README.md | 1 + test/benchmark_test.cc | 2 +- test/output_test_helper.cc | 12 ++++++------ 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e3abf0fc..713d22b6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -92,8 +92,13 @@ else() add_cxx_compiler_flag(-Wzero-as-null-pointer-constant) endif() if (HAVE_CXX_FLAG_FSTRICT_ALIASING) - add_cxx_compiler_flag(-Wstrict-aliasing) + if (NOT CMAKE_CXX_COMPILER_ID STREQUAL "Intel") #ICC17u2: Many false positives for Wstrict-aliasing + add_cxx_compiler_flag(-Wstrict-aliasing) + endif() endif() + # ICC17u2: overloaded virtual function "benchmark::Fixture::SetUp" is only partially overridden + # (because of deprecated overload) + add_cxx_compiler_flag(-wd654) add_cxx_compiler_flag(-Wthread-safety) if (HAVE_CXX_FLAG_WTHREAD_SAFETY) cxx_feature_check(THREAD_SAFETY_ATTRIBUTES) diff --git a/README.md b/README.md index 44466379..f16a9d79 100644 --- a/README.md +++ b/README.md @@ -643,6 +643,7 @@ The following minimum versions are strongly recommended build the library: * GCC 4.8 * Clang 3.4 * Visual Studio 2013 +* Intel 2015 Update 1 Anything older *may* work. diff --git a/test/benchmark_test.cc b/test/benchmark_test.cc index dfcf0920..57731331 100644 --- a/test/benchmark_test.cc +++ b/test/benchmark_test.cc @@ -150,7 +150,7 @@ static void BM_LongTest(benchmark::State& state) { BENCHMARK(BM_LongTest)->Range(1 << 16, 1 << 28); static void BM_ParallelMemset(benchmark::State& state) { - int size = state.range(0) / sizeof(int); + int size = state.range(0) / static_cast(sizeof(int)); int thread_size = size / state.threads; int from = thread_size * state.thread_index; int to = from + thread_size; diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 721d39f9..54c028a6 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -31,7 +31,7 @@ TestCaseList& GetTestCaseList(TestCaseID ID) { SubMap& GetSubstitutions() { // Don't use 'dec_re' from header because it may not yet be initialized. - static std::string dec_re = "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?"; + static std::string safe_dec_re = "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?"; static SubMap map = { {"%float", "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?"}, {"%int", "[ ]*[0-9]+"}, @@ -39,13 +39,13 @@ SubMap& GetSubstitutions() { {"%time", "[ ]*[0-9]{1,5} ns"}, {"%console_report", "[ ]*[0-9]{1,5} ns [ ]*[0-9]{1,5} ns [ ]*[0-9]+"}, {"%console_us_report", "[ ]*[0-9] us [ ]*[0-9] us [ ]*[0-9]+"}, - {"%csv_report", "[0-9]+," + dec_re + "," + dec_re + ",ns,,,,,"}, - {"%csv_us_report", "[0-9]+," + dec_re + "," + dec_re + ",us,,,,,"}, + {"%csv_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns,,,,,"}, + {"%csv_us_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",us,,,,,"}, {"%csv_bytes_report", - "[0-9]+," + dec_re + "," + dec_re + ",ns," + dec_re + ",,,,"}, + "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns," + safe_dec_re + ",,,,"}, {"%csv_items_report", - "[0-9]+," + dec_re + "," + dec_re + ",ns,," + dec_re + ",,,"}, - {"%csv_label_report_begin", "[0-9]+," + dec_re + "," + dec_re + ",ns,,,"}, + "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns,," + safe_dec_re + ",,,"}, + {"%csv_label_report_begin", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns,,,"}, {"%csv_label_report_end", ",,"}}; return map; } From 94c512c0439aa9b625d9e179767ce870df9f76a8 Mon Sep 17 00:00:00 2001 From: Eric Date: Mon, 27 Mar 2017 18:32:12 -0600 Subject: [PATCH 05/60] Replace int64_t usages with 'int' instead. (#359) Previously the constants used for converting between different units of time were declared using int64_t. However we should only use explicitly sized integer types when they are required, and should use 'int' everwhere else, and there is no good reason to use int64_t here. For that reason this patch changes the type of the constants. This should help address issue #354 as well. --- src/sleep.cc | 2 +- src/sleep.h | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/sleep.cc b/src/sleep.cc index 1449339e..54aa04a4 100644 --- a/src/sleep.cc +++ b/src/sleep.cc @@ -41,7 +41,7 @@ void SleepForMicroseconds(int microseconds) { } void SleepForMilliseconds(int milliseconds) { - SleepForMicroseconds(static_cast(milliseconds) * kNumMicrosPerMilli); + SleepForMicroseconds(milliseconds * kNumMicrosPerMilli); } void SleepForSeconds(double seconds) { diff --git a/src/sleep.h b/src/sleep.h index f1e515ca..f98551af 100644 --- a/src/sleep.h +++ b/src/sleep.h @@ -1,14 +1,12 @@ #ifndef BENCHMARK_SLEEP_H_ #define BENCHMARK_SLEEP_H_ -#include - namespace benchmark { -const int64_t kNumMillisPerSecond = 1000LL; -const int64_t kNumMicrosPerMilli = 1000LL; -const int64_t kNumMicrosPerSecond = kNumMillisPerSecond * 1000LL; -const int64_t kNumNanosPerMicro = 1000LL; -const int64_t kNumNanosPerSecond = kNumNanosPerMicro * kNumMicrosPerSecond; +const int kNumMillisPerSecond = 1000; +const int kNumMicrosPerMilli = 1000; +const int kNumMicrosPerSecond = kNumMillisPerSecond * 1000; +const int kNumNanosPerMicro = 1000; +const int kNumNanosPerSecond = kNumNanosPerMicro * kNumMicrosPerSecond; void SleepForMilliseconds(int milliseconds); void SleepForSeconds(double seconds); From ec15860da5808a0b0fa43ab74a5c4404dbcc7ac5 Mon Sep 17 00:00:00 2001 From: MVafin Date: Tue, 28 Mar 2017 09:35:17 +0300 Subject: [PATCH 06/60] Fix CPU frequency parsing on Linux (#355) (#356) * Fix reading CPU info from file Macro CHECK do nothing for release mode, meaning it doesn't invoke the arguments * Add myself to AUTHORS and CONTRIBUTORS --- AUTHORS | 1 + CONTRIBUTORS | 1 + src/sysinfo.cc | 4 +++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 8c15b87f..391434ca 100644 --- a/AUTHORS +++ b/AUTHORS @@ -25,6 +25,7 @@ Jussi Knuuttila Kaito Udagawa Lei Xu Matt Clarkson +Maxim Vafin Nick Hutchinson Oleksandr Sochka Paul Redmond diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 96cd15a0..0f7f6ed1 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -41,6 +41,7 @@ Kaito Udagawa Kai Wolf Lei Xu Matt Clarkson +Maxim Vafin Nick Hutchinson Oleksandr Sochka Pascal Leroy diff --git a/src/sysinfo.cc b/src/sysinfo.cc index 34f9871c..7feb79e6 100644 --- a/src/sysinfo.cc +++ b/src/sysinfo.cc @@ -75,7 +75,9 @@ bool ReadIntFromFile(const char* file, long* value) { char line[1024]; char* err; memset(line, '\0', sizeof(line)); - CHECK(read(fd, line, sizeof(line) - 1)); + ssize_t read_err = read(fd, line, sizeof(line) - 1); + ((void)read_err); // prevent unused warning + CHECK(read_err >= 0); const long temp_value = strtol(line, &err, 10); if (line[0] != '\0' && (*err == '\n' || *err == '\0')) { *value = temp_value; From 0dbcdf56a0d0ed817a7fccf8f622259ee8dafa18 Mon Sep 17 00:00:00 2001 From: Eric Date: Tue, 28 Mar 2017 01:43:42 -0600 Subject: [PATCH 07/60] Add BENCHMARK_BUILD_32_BITS option and add builders to test it (#360) * Add BENCHMARK_BUILD_32_BITS option and add builders to test it * Attempt to fix travis configuration * Make add_required_cxx_compiler_flag cause an error when the flag isn't supported * add gcc-multilib dependancy on travis * attempt to fix travis.yml parsing error * Require g++-multilib instead of gcc-multilib * Add 32 bit release configurations * Attempt to fix libc++ travis build w/ 32 bits * Work around CMake configuration failure on Travis --- .travis-libcxx-setup.sh | 6 ++++ .travis.yml | 50 +++++++++++++++++++++++++++++++++- CMakeLists.txt | 6 ++++ cmake/AddCXXCompilerFlag.cmake | 35 +++++++++++++++++++++--- 4 files changed, 92 insertions(+), 5 deletions(-) diff --git a/.travis-libcxx-setup.sh b/.travis-libcxx-setup.sh index 1b6b5851..a591743c 100644 --- a/.travis-libcxx-setup.sh +++ b/.travis-libcxx-setup.sh @@ -10,12 +10,18 @@ git clone --depth=1 https://github.com/llvm-mirror/llvm.git llvm-source git clone --depth=1 https://github.com/llvm-mirror/libcxx.git llvm-source/projects/libcxx git clone --depth=1 https://github.com/llvm-mirror/libcxxabi.git llvm-source/projects/libcxxabi +# Setup libc++ options +if [ -z "$BUILD_32_BITS" ]; then + export BUILD_32_BITS=OFF && echo disabling 32 bit build +fi + # Build and install libc++ (Use unstable ABI for better sanitizer coverage) mkdir llvm-build && cd llvm-build cmake -DCMAKE_C_COMPILER=${C_COMPILER} -DCMAKE_CXX_COMPILER=${COMPILER} \ -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_INSTALL_PREFIX=/usr \ -DLIBCXX_ABI_UNSTABLE=ON \ -DLLVM_USE_SANITIZER=${LIBCXX_SANITIZER} \ + -DLLVM_BUILD_32_BITS=${BUILD_32_BITS} \ ../llvm-source make cxx -j2 sudo make install-cxxabi install-cxx diff --git a/.travis.yml b/.travis.yml index 19c68dda..6bbd13bb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,6 +20,18 @@ matrix: env: COMPILER=g++ C_COMPILER=gcc BUILD_TYPE=Debug - compiler: gcc env: COMPILER=g++ C_COMPILER=gcc BUILD_TYPE=Release + - compiler: gcc + addons: + apt: + packages: + - g++-multilib + env: COMPILER=g++ C_COMPILER=gcc BUILD_TYPE=Debug BUILD_32_BITS=ON + - compiler: gcc + addons: + apt: + packages: + - g++-multilib + env: COMPILER=g++ C_COMPILER=gcc BUILD_TYPE=Release BUILD_32_BITS=ON - compiler: gcc addons: apt: @@ -44,6 +56,39 @@ matrix: - COMPILER=clang++-3.8 C_COMPILER=clang-3.8 BUILD_TYPE=Debug - LIBCXX_BUILD=1 - EXTRA_FLAGS="-stdlib=libc++" + - compiler: clang + addons: + apt: + packages: + clang-3.8 + env: + - COMPILER=clang++-3.8 C_COMPILER=clang-3.8 BUILD_TYPE=Release + - LIBCXX_BUILD=1 + - EXTRA_FLAGS="-stdlib=libc++" + # Clang w/ 32bit libc++ + - compiler: clang + addons: + apt: + packages: + - clang-3.8 + - g++-multilib + env: + - COMPILER=clang++-3.8 C_COMPILER=clang-3.8 BUILD_TYPE=Debug + - LIBCXX_BUILD=1 + - BUILD_32_BITS=ON + - EXTRA_FLAGS="-stdlib=libc++ -m32" + # Clang w/ 32bit libc++ + - compiler: clang + addons: + apt: + packages: + - clang-3.8 + - g++-multilib + env: + - COMPILER=clang++-3.8 C_COMPILER=clang-3.8 BUILD_TYPE=Release + - LIBCXX_BUILD=1 + - BUILD_32_BITS=ON + - EXTRA_FLAGS="-stdlib=libc++ -m32" # Clang w/ libc++, ASAN, UBSAN - compiler: clang addons: @@ -77,6 +122,9 @@ matrix: - EXTRA_FLAGS="-stdlib=libc++ -g -O2 -fno-omit-frame-pointer -fsanitize=thread -fno-sanitize-recover=all" before_script: + - if [ -z "$BUILD_32_BITS" ]; then + export BUILD_32_BITS=OFF && echo disabling 32 bit build; + fi - if [ -n "${LIBCXX_BUILD}" ]; then source .travis-libcxx-setup.sh; fi @@ -90,7 +138,7 @@ install: fi script: - - cmake -DCMAKE_C_COMPILER=${C_COMPILER} -DCMAKE_CXX_COMPILER=${COMPILER} -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DCMAKE_CXX_FLAGS="${EXTRA_FLAGS}" .. + - cmake -DCMAKE_C_COMPILER=${C_COMPILER} -DCMAKE_CXX_COMPILER=${COMPILER} -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DCMAKE_CXX_FLAGS="${EXTRA_FLAGS}" -DBENCHMARK_BUILD_32_BITS=${BUILD_32_BITS} .. - make - make CTEST_OUTPUT_ON_FAILURE=1 test diff --git a/CMakeLists.txt b/CMakeLists.txt index 713d22b6..1ba31331 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,6 +14,8 @@ option(BENCHMARK_ENABLE_TESTING "Enable testing of the benchmark library." ON) option(BENCHMARK_ENABLE_EXCEPTIONS "Enable the use of exceptions in the benchmark library." ON) option(BENCHMARK_ENABLE_LTO "Enable link time optimisation of the benchmark library." OFF) option(BENCHMARK_USE_LIBCXX "Build and test using libc++ as the standard library." OFF) +option(BENCHMARK_BUILD_32_BITS "Build a 32 bit version of the library" OFF) + # Make sure we can import out CMake functions list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") @@ -34,6 +36,10 @@ include(CheckCXXCompilerFlag) include(AddCXXCompilerFlag) include(CXXFeatureCheck) +if (BENCHMARK_BUILD_32_BITS) + add_required_cxx_compiler_flag(-m32) +endif() + if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") # Turn compiler warnings up to 11 string(REGEX REPLACE "[-/]W[1-4]" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") diff --git a/cmake/AddCXXCompilerFlag.cmake b/cmake/AddCXXCompilerFlag.cmake index 9afde84b..0b176ba2 100644 --- a/cmake/AddCXXCompilerFlag.cmake +++ b/cmake/AddCXXCompilerFlag.cmake @@ -19,14 +19,21 @@ set(__add_cxx_compiler_flag INCLUDED) include(CheckCXXCompilerFlag) -function(add_cxx_compiler_flag FLAG) +function(mangle_compiler_flag FLAG OUTPUT) string(TOUPPER "HAVE_CXX_FLAG_${FLAG}" SANITIZED_FLAG) string(REPLACE "+" "X" SANITIZED_FLAG ${SANITIZED_FLAG}) string(REGEX REPLACE "[^A-Za-z_0-9]" "_" SANITIZED_FLAG ${SANITIZED_FLAG}) string(REGEX REPLACE "_+" "_" SANITIZED_FLAG ${SANITIZED_FLAG}) - set(CMAKE_REQUIRED_FLAGS "${FLAG}") - check_cxx_compiler_flag("${FLAG}" ${SANITIZED_FLAG}) - if(${SANITIZED_FLAG}) + set(${OUTPUT} "${SANITIZED_FLAG}" PARENT_SCOPE) +endfunction(mangle_compiler_flag) + +function(add_cxx_compiler_flag FLAG) + mangle_compiler_flag("${FLAG}" MANGLED_FLAG) + set(OLD_CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}") + set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${FLAG}") + check_cxx_compiler_flag("${FLAG}" ${MANGLED_FLAG}) + set(CMAKE_REQUIRED_FLAGS "${OLD_CMAKE_REQUIRED_FLAGS}") + if(${MANGLED_FLAG}) set(VARIANT ${ARGV1}) if(ARGV1) string(TOUPPER "_${VARIANT}" VARIANT) @@ -35,3 +42,23 @@ function(add_cxx_compiler_flag FLAG) endif() endfunction() +function(add_required_cxx_compiler_flag FLAG) + mangle_compiler_flag("${FLAG}" MANGLED_FLAG) + set(OLD_CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}") + set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${FLAG}") + check_cxx_compiler_flag("${FLAG}" ${MANGLED_FLAG}) + set(CMAKE_REQUIRED_FLAGS "${OLD_CMAKE_REQUIRED_FLAGS}") + if(${MANGLED_FLAG}) + set(VARIANT ${ARGV1}) + if(ARGV1) + string(TOUPPER "_${VARIANT}" VARIANT) + endif() + set(CMAKE_CXX_FLAGS${VARIANT} "${CMAKE_CXX_FLAGS${VARIANT}} ${FLAG}" PARENT_SCOPE) + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${FLAG}" PARENT_SCOPE) + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${FLAG}" PARENT_SCOPE) + set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${FLAG}" PARENT_SCOPE) + set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${FLAG}" PARENT_SCOPE) + else() + message(FATAL_ERROR "Required flag '${FLAG}' is not supported by the compiler") + endif() +endfunction() From 17298b2dc0e6dc9f78b149ab9256064d0ac96520 Mon Sep 17 00:00:00 2001 From: Ray Glover Date: Wed, 29 Mar 2017 11:39:18 +0100 Subject: [PATCH 08/60] Python 2/3 compatibility (#361) * [tools] python 2/3 support * update authors/contributors --- AUTHORS | 1 + CONTRIBUTORS | 1 + tools/compare_bench.py | 2 +- tools/gbench/report.py | 2 +- tools/gbench/util.py | 22 +++++++++++----------- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/AUTHORS b/AUTHORS index 391434ca..c4b059df 100644 --- a/AUTHORS +++ b/AUTHORS @@ -18,6 +18,7 @@ Eugene Zhuk Evgeny Safronov Felix Homann Google Inc. +International Business Machines Corporation Ismael Jimenez Martinez Joao Paulo Magalhaes JianXiong Zhou diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 0f7f6ed1..8ca4565a 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -48,6 +48,7 @@ Pascal Leroy Paul Redmond Pierre Phaneuf Radoslav Yovchev +Ray Glover Shuo Chen Yusuke Suzuki Tobias Ulvgård diff --git a/tools/compare_bench.py b/tools/compare_bench.py index 8a7e7991..d54baaa0 100755 --- a/tools/compare_bench.py +++ b/tools/compare_bench.py @@ -59,7 +59,7 @@ def main(): json1 = gbench.util.run_or_load_benchmark(test1, benchmark_options) json2 = gbench.util.run_or_load_benchmark(test2, benchmark_options) output_lines = gbench.report.generate_difference_report(json1, json2) - print 'Comparing %s to %s' % (test1, test2) + print('Comparing %s to %s' % (test1, test2)) for ln in output_lines: print(ln) diff --git a/tools/gbench/report.py b/tools/gbench/report.py index 6776d037..8f1b0fa8 100644 --- a/tools/gbench/report.py +++ b/tools/gbench/report.py @@ -132,7 +132,7 @@ class TestReportDifference(unittest.TestCase): json1, json2 = self.load_results() output_lines_with_header = generate_difference_report(json1, json2, use_color=False) output_lines = output_lines_with_header[2:] - print "\n".join(output_lines_with_header) + print("\n".join(output_lines_with_header)) self.assertEqual(len(output_lines), len(expect_lines)) for i in xrange(0, len(output_lines)): parts = [x for x in output_lines[i].split(' ') if x] diff --git a/tools/gbench/util.py b/tools/gbench/util.py index bfce376c..07c23772 100644 --- a/tools/gbench/util.py +++ b/tools/gbench/util.py @@ -20,21 +20,21 @@ def is_executable_file(filename): """ if not os.path.isfile(filename): return False - with open(filename, 'r') as f: + with open(filename, mode='rb') as f: magic_bytes = f.read(_num_magic_bytes) if sys.platform == 'darwin': return magic_bytes in [ - '\xfe\xed\xfa\xce', # MH_MAGIC - '\xce\xfa\xed\xfe', # MH_CIGAM - '\xfe\xed\xfa\xcf', # MH_MAGIC_64 - '\xcf\xfa\xed\xfe', # MH_CIGAM_64 - '\xca\xfe\xba\xbe', # FAT_MAGIC - '\xbe\xba\xfe\xca' # FAT_CIGAM + b'\xfe\xed\xfa\xce', # MH_MAGIC + b'\xce\xfa\xed\xfe', # MH_CIGAM + b'\xfe\xed\xfa\xcf', # MH_MAGIC_64 + b'\xcf\xfa\xed\xfe', # MH_CIGAM_64 + b'\xca\xfe\xba\xbe', # FAT_MAGIC + b'\xbe\xba\xfe\xca' # FAT_CIGAM ] elif sys.platform.startswith('win'): - return magic_bytes == 'MZ' + return magic_bytes == b'MZ' else: - return magic_bytes == '\x7FELF' + return magic_bytes == b'\x7FELF' def is_json_file(filename): @@ -68,7 +68,7 @@ def classify_input_file(filename): elif is_json_file(filename): ftype = IT_JSON else: - err_msg = "'%s' does not name a valid benchmark executable or JSON file" + err_msg = "'%s' does not name a valid benchmark executable or JSON file" % filename return ftype, err_msg @@ -80,7 +80,7 @@ def check_input_file(filename): """ ftype, msg = classify_input_file(filename) if ftype == IT_Invalid: - print "Invalid input file: %s" % msg + print("Invalid input file: %s" % msg) sys.exit(1) return ftype From ea26e62a0dcaa2df529c5b0e901745a1b914063d Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 1 Apr 2017 17:04:37 +0100 Subject: [PATCH 09/60] Fixes #357: broken RMS values when time unit is set. --- src/complexity.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/complexity.cc b/src/complexity.cc index 015db4ce..02adbef6 100644 --- a/src/complexity.cc +++ b/src/complexity.cc @@ -295,6 +295,11 @@ std::vector ComputeBigO( big_o.report_big_o = true; big_o.complexity = result_cpu.complexity; + // All the time results are reported after being multiplied by the + // time unit multiplier. But since RMS is a relative quantity it + // should not be multiplied at all. So, here, we _divide_ it by the + // multiplier so that when it is multiplied later the result is the + // correct one. double multiplier = GetTimeUnitMultiplier(reports[0].time_unit); // Only add label to mean/stddev if it is same for all runs @@ -307,6 +312,9 @@ std::vector ComputeBigO( rms.cpu_accumulated_time = result_cpu.rms / multiplier; rms.report_rms = true; rms.complexity = result_cpu.complexity; + // don't forget to keep the time unit, or we won't be able to + // recover the correct value. + rms.time_unit = reports[0].time_unit; results.push_back(big_o); results.push_back(rms); From 824bbb818e2d34e546c77d719687ab732264603c Mon Sep 17 00:00:00 2001 From: Giuseppe Roberti Date: Mon, 3 Apr 2017 01:46:59 +0200 Subject: [PATCH 10/60] Add CMake Package Config files during install - Remove target_include_directories of ${PROJECT_SOURCE_DIR}/include to fix error: Target "benchmark" INTERFACE_INCLUDE_DIRECTORIES property contains path which is prefixed in the source directory. --- cmake/Config.cmake.in | 1 + src/CMakeLists.txt | 41 ++++++++++++++++++++++++++++++++++------- 2 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 cmake/Config.cmake.in diff --git a/cmake/Config.cmake.in b/cmake/Config.cmake.in new file mode 100644 index 00000000..6e9256ee --- /dev/null +++ b/cmake/Config.cmake.in @@ -0,0 +1 @@ +include("${CMAKE_CURRENT_LIST_DIR}/@targets_export_name@.cmake") diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 40388751..0472b824 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -27,18 +27,45 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Windows") target_link_libraries(benchmark Shlwapi) endif() -# Expose public API -target_include_directories(benchmark PUBLIC ${PROJECT_SOURCE_DIR}/include) +set(include_install_dir "include/doctest/") +set(lib_install_dir "lib/") +set(bin_install_dir "bin/") +set(config_install_dir "lib/cmake/${PROJECT_NAME}") + +set(generated_dir "${CMAKE_CURRENT_BINARY_DIR}/generated") + +set(version_config "${generated_dir}/${PROJECT_NAME}ConfigVersion.cmake") +set(project_config "${generated_dir}/${PROJECT_NAME}Config.cmake") +set(targets_export_name "${PROJECT_NAME}Targets") + +set(namespace "${PROJECT_NAME}::") + +include(CMakePackageConfigHelpers) +write_basic_package_version_file( + "${version_config}" VERSION ${GIT_VERSION} COMPATIBILITY SameMajorVersion +) + +configure_file("${CMAKE_SOURCE_DIR}/cmake/Config.cmake.in" "${project_config}" @ONLY) # Install target (will install the library to specified CMAKE_INSTALL_PREFIX variable) install( TARGETS benchmark - ARCHIVE DESTINATION lib - LIBRARY DESTINATION lib - RUNTIME DESTINATION bin - COMPONENT library) + EXPORT ${targets_export_name} + ARCHIVE DESTINATION ${lib_install_dir} + LIBRARY DESTINATION ${lib_install_dir} + RUNTIME DESTINATION ${bin_install_dir} + INCLUDES DESTINATION ${include_install_dir}) install( DIRECTORY "${PROJECT_SOURCE_DIR}/include/benchmark" - DESTINATION include + DESTINATION ${include_install_dir} FILES_MATCHING PATTERN "*.*h") + +install( + FILES "${project_config}" "${version_config}" + DESTINATION "${config_install_dir}") + +install( + EXPORT "${targets_export_name}" + NAMESPACE "${namespace}" + DESTINATION "${config_install_dir}") From 128fe25025dcf75baa2e1c533b1aaf90a5f1c7fc Mon Sep 17 00:00:00 2001 From: Giuseppe Roberti Date: Tue, 4 Apr 2017 02:13:20 +0200 Subject: [PATCH 11/60] Fix ${include_install_dir} --- src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0472b824..b403bbdc 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -27,7 +27,7 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Windows") target_link_libraries(benchmark Shlwapi) endif() -set(include_install_dir "include/doctest/") +set(include_install_dir "include/benchmark") set(lib_install_dir "lib/") set(bin_install_dir "bin/") set(config_install_dir "lib/cmake/${PROJECT_NAME}") From 858581ea766794aa9f37bc047adffdcef912c04a Mon Sep 17 00:00:00 2001 From: Dominic Hamon Date: Tue, 4 Apr 2017 08:39:02 -0700 Subject: [PATCH 12/60] Remove unnecessary benchmark subfolder --- src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a50e3cde..d517b3cc 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -31,7 +31,7 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Windows") target_link_libraries(benchmark Shlwapi) endif() -set(include_install_dir "include/benchmark") +set(include_install_dir "include") set(lib_install_dir "lib/") set(bin_install_dir "bin/") set(config_install_dir "lib/cmake/${PROJECT_NAME}") From 9a5072d1bf9187b32ce9a88842dffa31ef416442 Mon Sep 17 00:00:00 2001 From: jpmag Date: Tue, 4 Apr 2017 20:31:28 +0100 Subject: [PATCH 13/60] Fixes #357: broken RMS values when time unit is set. (#362) --- src/complexity.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/complexity.cc b/src/complexity.cc index 015db4ce..02adbef6 100644 --- a/src/complexity.cc +++ b/src/complexity.cc @@ -295,6 +295,11 @@ std::vector ComputeBigO( big_o.report_big_o = true; big_o.complexity = result_cpu.complexity; + // All the time results are reported after being multiplied by the + // time unit multiplier. But since RMS is a relative quantity it + // should not be multiplied at all. So, here, we _divide_ it by the + // multiplier so that when it is multiplied later the result is the + // correct one. double multiplier = GetTimeUnitMultiplier(reports[0].time_unit); // Only add label to mean/stddev if it is same for all runs @@ -307,6 +312,9 @@ std::vector ComputeBigO( rms.cpu_accumulated_time = result_cpu.rms / multiplier; rms.report_rms = true; rms.complexity = result_cpu.complexity; + // don't forget to keep the time unit, or we won't be able to + // recover the correct value. + rms.time_unit = reports[0].time_unit; results.push_back(big_o); results.push_back(rms); From 312d9d0ac5c280e81aee0689045a10ae441b4db1 Mon Sep 17 00:00:00 2001 From: Daniel Varga Date: Mon, 10 Apr 2017 17:43:05 +0200 Subject: [PATCH 14/60] Fix cmake file not to use CMAKE_SOURCE_DIR (#367) Using CMAKE_SOURCE_DIR benchmark cannot be built as part of a project. This change allows to add benchmark using add_subdirectory to a bigger project. --- src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d517b3cc..77077739 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -49,7 +49,7 @@ write_basic_package_version_file( "${version_config}" VERSION ${GIT_VERSION} COMPATIBILITY SameMajorVersion ) -configure_file("${CMAKE_SOURCE_DIR}/cmake/Config.cmake.in" "${project_config}" @ONLY) +configure_file("${PROJECT_SOURCE_DIR}/cmake/Config.cmake.in" "${project_config}" @ONLY) # Install target (will install the library to specified CMAKE_INSTALL_PREFIX variable) install( From 7f87c98d36279b1819e9c8ee0dc93c6a8ea64aee Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Mon, 17 Apr 2017 20:49:51 -0600 Subject: [PATCH 15/60] Enable by removing -DNDEBUG when running the tests. In non-debug builds CMake automatically adds -DNDEBUG, this means that uses of `assert` in the tests are disabled for non-debug builds. Obviously we want these tests to run, regardless of configuration. This patch strips -DNDEBUG during non-debug builds and adds -UNDEBUG just to be sure. --- test/CMakeLists.txt | 19 +++++++++++++++++++ test/diagnostics_test.cc | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 87245984..14ba7a6e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -2,6 +2,25 @@ find_package(Threads REQUIRED) +# NOTE: Some tests use `` to perform the test. Therefore we must +# strip -DNDEBUG from the default CMake flags in DEBUG mode. +string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE) +if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" ) + add_definitions( -UNDEBUG ) + add_definitions(-DTEST_BENCHMARK_LIBRARY_HAS_NO_ASSERTIONS) + # Also remove /D NDEBUG to avoid MSVC warnings about conflicting defines. + foreach (flags_var_to_scrub + CMAKE_CXX_FLAGS_RELEASE + CMAKE_CXX_FLAGS_RELWITHDEBINFO + CMAKE_CXX_FLAGS_MINSIZEREL + CMAKE_C_FLAGS_RELEASE + CMAKE_C_FLAGS_RELWITHDEBINFO + CMAKE_C_FLAGS_MINSIZEREL) + string (REGEX REPLACE "(^| )[/-]D *NDEBUG($| )" " " + "${flags_var_to_scrub}" "${${flags_var_to_scrub}}") + endforeach() +endif() + # NOTE: These flags must be added after find_package(Threads REQUIRED) otherwise # they will break the configuration check. if (DEFINED BENCHMARK_CXX_LINKER_FLAGS) diff --git a/test/diagnostics_test.cc b/test/diagnostics_test.cc index c6c235d0..1046730b 100644 --- a/test/diagnostics_test.cc +++ b/test/diagnostics_test.cc @@ -26,7 +26,7 @@ void TestHandler() { } void try_invalid_pause_resume(benchmark::State& state) { -#if !defined(NDEBUG) && !defined(TEST_HAS_NO_EXCEPTIONS) +#if !defined(TEST_BENCHMARK_LIBRARY_HAS_NO_ASSERTIONS) && !defined(TEST_HAS_NO_EXCEPTIONS) try { state.PauseTiming(); std::abort(); From 74b24058ad4914b837200d0341050657ba154e4a Mon Sep 17 00:00:00 2001 From: Eric Date: Mon, 17 Apr 2017 22:29:28 -0600 Subject: [PATCH 16/60] Add Benchmark::Iterations for explicit iteration count control - Fixes #370 (#373) * Add Benchmark::Iterations for explicitly specifying the number of iterations to use. * Document that benchmark::Iterations should not be used to limit benchmark runtimes --- include/benchmark/benchmark_api.h | 11 ++++++++++- src/benchmark.cc | 6 ++++-- src/benchmark_api_internal.h | 1 + src/benchmark_register.cc | 33 +++++++++++++++++++++---------- test/options_test.cc | 24 +++++++++++++++++++++- 5 files changed, 61 insertions(+), 14 deletions(-) diff --git a/include/benchmark/benchmark_api.h b/include/benchmark/benchmark_api.h index 8cde61ba..1e853e2c 100644 --- a/include/benchmark/benchmark_api.h +++ b/include/benchmark/benchmark_api.h @@ -577,9 +577,17 @@ class Benchmark { // Set the minimum amount of time to use when running this benchmark. This // option overrides the `benchmark_min_time` flag. - // REQUIRES: `t > 0` + // REQUIRES: `t > 0` and `Iterations` has not been called on this benchmark. Benchmark* MinTime(double t); + // Specify the amount of iterations that should be run by this benchmark. + // REQUIRES: 'n > 0' and `MinTime` has not been called on this benchmark. + // + // NOTE: This function should only be used when *exact* iteration control is + // needed and never to control or limit how long a benchmark runs, where + // `--benchmark_min_time=N` or `MinTime(...)` should be used instead. + Benchmark* Iterations(size_t n); + // Specify the amount of times to repeat this benchmark. This option overrides // the `benchmark_repetitions` flag. // REQUIRES: `n > 0` @@ -668,6 +676,7 @@ class Benchmark { TimeUnit time_unit_; int range_multiplier_; double min_time_; + size_t iterations_; int repetitions_; bool use_real_time_; bool use_manual_time_; diff --git a/src/benchmark.cc b/src/benchmark.cc index a93a83de..4adb97ac 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -285,7 +285,8 @@ std::vector RunBenchmark( std::vector* complexity_reports) { std::vector reports; // return value - size_t iters = 1; + const bool has_explicit_iteration_count = b.iterations != 0; + size_t iters = has_explicit_iteration_count ? b.iterations : 1; std::unique_ptr manager; std::vector pool(b.threads - 1); const int repeats = @@ -333,7 +334,8 @@ std::vector RunBenchmark( !IsZero(b.min_time) ? b.min_time : FLAGS_benchmark_min_time; // If this was the first run, was elapsed time or cpu time large enough? // If this is not the first run, go with the current value of iter. - if ((i > 0) || results.has_error_ || (iters >= kMaxIterations) || + if ((i > 0) || has_explicit_iteration_count || results.has_error_ || + (iters >= kMaxIterations) || (seconds >= min_time) || (results.real_time_used >= 5 * min_time)) { BenchmarkReporter::Run report = CreateRunReport(b, results, iters, seconds); diff --git a/src/benchmark_api_internal.h b/src/benchmark_api_internal.h index 978fc0a0..828ed121 100644 --- a/src/benchmark_api_internal.h +++ b/src/benchmark_api_internal.h @@ -28,6 +28,7 @@ struct Benchmark::Instance { bool last_benchmark_instance; int repetitions; double min_time; + size_t iterations; int threads; // Number of concurrent threads to us }; diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index cf78bbf4..fe373204 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -143,6 +143,7 @@ bool BenchmarkFamilies::FindBenchmarks( instance.time_unit = family->time_unit_; instance.range_multiplier = family->range_multiplier_; instance.min_time = family->min_time_; + instance.iterations = family->iterations_; instance.repetitions = family->repetitions_; instance.use_real_time = family->use_real_time_; instance.use_manual_time = family->use_manual_time_; @@ -167,12 +168,13 @@ bool BenchmarkFamilies::FindBenchmarks( ++arg_i; } - if (!IsZero(family->min_time_)) { + if (!IsZero(family->min_time_)) instance.name += StringPrintF("/min_time:%0.3f", family->min_time_); - } - if (family->repetitions_ != 0) { + if (family->iterations_ != 0) + instance.name += StringPrintF("/iterations:%d", family->iterations_); + if (family->repetitions_ != 0) instance.name += StringPrintF("/repeats:%d", family->repetitions_); - } + if (family->use_manual_time_) { instance.name += "/manual_time"; } else if (family->use_real_time_) { @@ -219,6 +221,7 @@ Benchmark::Benchmark(const char* name) time_unit_(kNanosecond), range_multiplier_(kRangeMultiplier), min_time_(0), + iterations_(0), repetitions_(0), use_real_time_(false), use_manual_time_(false), @@ -344,6 +347,22 @@ Benchmark* Benchmark::RangeMultiplier(int multiplier) { return this; } + +Benchmark* Benchmark::MinTime(double t) { + CHECK(t > 0.0); + CHECK(iterations_ == 0); + min_time_ = t; + return this; +} + + +Benchmark* Benchmark::Iterations(size_t n) { + CHECK(n > 0); + CHECK(IsZero(min_time_)); + iterations_ = n; + return this; +} + Benchmark* Benchmark::Repetitions(int n) { CHECK(n > 0); repetitions_ = n; @@ -355,12 +374,6 @@ Benchmark* Benchmark::ReportAggregatesOnly(bool value) { return this; } -Benchmark* Benchmark::MinTime(double t) { - CHECK(t > 0.0); - min_time_ = t; - return this; -} - Benchmark* Benchmark::UseRealTime() { CHECK(!use_manual_time_) << "Cannot set UseRealTime and UseManualTime simultaneously."; diff --git a/test/options_test.cc b/test/options_test.cc index bedb1cc3..bbbed288 100644 --- a/test/options_test.cc +++ b/test/options_test.cc @@ -1,8 +1,12 @@ #include "benchmark/benchmark_api.h" - #include #include +#if defined(NDEBUG) +#undef NDEBUG +#endif +#include + void BM_basic(benchmark::State& state) { while (state.KeepRunning()) { } @@ -40,4 +44,22 @@ void CustomArgs(benchmark::internal::Benchmark* b) { BENCHMARK(BM_basic)->Apply(CustomArgs); +void BM_explicit_iteration_count(benchmark::State& st) { + // Test that benchmarks specified with an explicit iteration count are + // only run once. + static bool invoked_before = false; + assert(!invoked_before); + invoked_before = true; + + // Test that the requested iteration count is respected. + assert(st.max_iterations == 42); + size_t actual_iterations = 0; + while (st.KeepRunning()) + ++actual_iterations; + assert(st.iterations() == st.max_iterations); + assert(st.iterations() == 42); + +} +BENCHMARK(BM_explicit_iteration_count)->Iterations(42); + BENCHMARK_MAIN() From 46afd8e69339b546526706056da9dd5009fa01f1 Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Tue, 18 Apr 2017 00:13:18 -0600 Subject: [PATCH 17/60] Don't limit benchmarks with manual timers to 5x the elapsed real time. When using CPU time to determine the correct number of iterations the library additionally checks if the benchmark has consumed 5x the minimum required time according to the wall clock. This prevents benchmarks with low CPU usage from running for much longer than actually intended. However when a benchmark uses a manual timer this heuristic isn't helpful and likely isn't correct since we don't know what the manual timer actually measures. This patch removes the above restriction when a benchmark specifies a manual timer. --- src/benchmark.cc | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/benchmark.cc b/src/benchmark.cc index 4adb97ac..00ffa07f 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -296,7 +296,7 @@ std::vector RunBenchmark( (b.report_mode == internal::RM_Unspecified ? FLAGS_benchmark_report_aggregates_only : b.report_mode == internal::RM_ReportAggregatesOnly); - for (int i = 0; i < repeats; i++) { + for (int repetition_num = 0; repetition_num < repeats; repetition_num++) { for (;;) { // Try benchmark VLOG(2) << "Running " << b.name << " for " << iters << "\n"; @@ -332,11 +332,20 @@ std::vector RunBenchmark( const double min_time = !IsZero(b.min_time) ? b.min_time : FLAGS_benchmark_min_time; - // If this was the first run, was elapsed time or cpu time large enough? - // If this is not the first run, go with the current value of iter. - if ((i > 0) || has_explicit_iteration_count || results.has_error_ || - (iters >= kMaxIterations) || - (seconds >= min_time) || (results.real_time_used >= 5 * min_time)) { + + // Determine if this run should be reported; Either it has + // run for a sufficient amount of time or because an error was reported. + const bool should_report = repetition_num > 0 + || has_explicit_iteration_count // An exact iteration count was requested + || results.has_error_ + || iters >= kMaxIterations + || seconds >= min_time // the elapsed time is large enough + // CPU time is specified but the elapsed real time greatly exceeds the + // minimum time. Note that user provided timers are except from this + // sanity check. + || ((results.real_time_used >= 5 * min_time) && !b.use_manual_time); + + if (should_report) { BenchmarkReporter::Run report = CreateRunReport(b, results, iters, seconds); if (!report.error_occurred && b.complexity != oNone) From 09b93ccc6a9aed84c269b6f5b8130c878e518ebb Mon Sep 17 00:00:00 2001 From: Dmitry Trifonov Date: Tue, 18 Apr 2017 08:48:07 -0700 Subject: [PATCH 18/60] fix android compilation (#372) * fix android compilation * checking __GLIBCXX__ and __GLIBCPP__ macro in addition to __ANDROID__ * using vsnprintf instead of std::vsnprintf to compile on Android * removed __GLIBCPP__ check on Android * StringPrintF instead of std::to_string for Android --- src/benchmark_register.cc | 5 +++-- src/colorprint.cc | 4 ++-- test/register_benchmark_test.cc | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index fe373204..c95da987 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -31,6 +31,7 @@ #include #include #include +#include #include #include "check.h" @@ -163,8 +164,8 @@ bool BenchmarkFamilies::FindBenchmarks( StringPrintF("%s:", family->arg_names_[arg_i].c_str()); } } - - instance.name += std::to_string(arg); + + instance.name += StringPrintF("%d", arg); ++arg_i; } diff --git a/src/colorprint.cc b/src/colorprint.cc index 513376b1..2dec4a8b 100644 --- a/src/colorprint.cc +++ b/src/colorprint.cc @@ -89,7 +89,7 @@ std::string FormatString(const char* msg, va_list args) { std::size_t size = 256; char local_buff[256]; - auto ret = std::vsnprintf(local_buff, size, msg, args_cp); + auto ret = vsnprintf(local_buff, size, msg, args_cp); va_end(args_cp); @@ -104,7 +104,7 @@ std::string FormatString(const char* msg, va_list args) { // we did not provide a long enough buffer on our first attempt. size = (size_t)ret + 1; // + 1 for the null byte std::unique_ptr buff(new char[size]); - ret = std::vsnprintf(buff.get(), size, msg, args); + ret = vsnprintf(buff.get(), size, msg, args); CHECK(ret > 0 && ((size_t)ret) < size); return buff.get(); } diff --git a/test/register_benchmark_test.cc b/test/register_benchmark_test.cc index e9f8ea53..c0ecad7d 100644 --- a/test/register_benchmark_test.cc +++ b/test/register_benchmark_test.cc @@ -114,14 +114,14 @@ void TestRegistrationAtRuntime() { #endif #ifndef BENCHMARK_HAS_NO_VARIADIC_REGISTER_BENCHMARK { - int x = 42; + const char* x = "42"; auto capturing_lam = [=](benchmark::State& st) { while (st.KeepRunning()) { } - st.SetLabel(std::to_string(x)); + st.SetLabel(x); }; benchmark::RegisterBenchmark("lambda_benchmark", capturing_lam); - AddCases({{"lambda_benchmark", "42"}}); + AddCases({{"lambda_benchmark", x}}); } #endif } From 7a74b74856bae690a0998c967c7807dd2272af82 Mon Sep 17 00:00:00 2001 From: Dmitry Trifonov Date: Fri, 21 Apr 2017 05:07:52 +0300 Subject: [PATCH 19/60] fix for android NDK r10e (#375) --- src/complexity.cc | 4 ++-- test/complexity_test.cc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/complexity.cc b/src/complexity.cc index 02adbef6..a4c57c43 100644 --- a/src/complexity.cc +++ b/src/complexity.cc @@ -35,9 +35,9 @@ BigOFunc* FittingCurve(BigO complexity) { case oNCubed: return [](int n) -> double { return std::pow(n, 3); }; case oLogN: - return [](int n) { return std::log2(n); }; + return [](int n) { return log2(n); }; case oNLogN: - return [](int n) { return n * std::log2(n); }; + return [](int n) { return n * log2(n); }; case o1: default: return [](int) { return 1.0; }; diff --git a/test/complexity_test.cc b/test/complexity_test.cc index 14e03b06..62d1154d 100644 --- a/test/complexity_test.cc +++ b/test/complexity_test.cc @@ -141,7 +141,7 @@ BENCHMARK(BM_Complexity_O_N_log_N) BENCHMARK(BM_Complexity_O_N_log_N) ->RangeMultiplier(2) ->Range(1 << 10, 1 << 16) - ->Complexity([](int n) { return n * std::log2(n); }); + ->Complexity([](int n) { return n * log2(n); }); BENCHMARK(BM_Complexity_O_N_log_N) ->RangeMultiplier(2) ->Range(1 << 10, 1 << 16) From 3336ea00d892fc312715c3a00d33a9568261e86a Mon Sep 17 00:00:00 2001 From: Arkady Shapkin Date: Mon, 24 Apr 2017 20:45:24 +0300 Subject: [PATCH 20/60] Support VS2017 on AppVeyor (#376) --- appveyor.yml | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 204f30de..e084f386 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,16 +1,18 @@ version: '{build}' +image: Visual Studio 2017 + configuration: - Debug - Release environment: matrix: - - compiler: msvc-12-seh - generator: "Visual Studio 12 2013" + - compiler: msvc-15-seh + generator: "Visual Studio 15 2017" - - compiler: msvc-12-seh - generator: "Visual Studio 12 2013 Win64" + - compiler: msvc-15-seh + generator: "Visual Studio 15 2017 Win64" - compiler: msvc-14-seh generator: "Visual Studio 14 2015" @@ -18,9 +20,16 @@ environment: - compiler: msvc-14-seh generator: "Visual Studio 14 2015 Win64" + - compiler: msvc-12-seh + generator: "Visual Studio 12 2013" + + - compiler: msvc-12-seh + generator: "Visual Studio 12 2013 Win64" + - compiler: gcc-5.3.0-posix generator: "MinGW Makefiles" cxx_path: 'C:\mingw-w64\i686-5.3.0-posix-dwarf-rt_v4-rev0\mingw32\bin' + APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015 matrix: fast_finish: true @@ -30,12 +39,6 @@ install: - if "%generator%"=="MinGW Makefiles" (set "PATH=%PATH:C:\Program Files\Git\usr\bin;=%") - if not "%cxx_path%"=="" (set "PATH=%PATH%;%cxx_path%") -# TODO Remove this. This is a hack to work around bogus warning messages -# See http://goo.gl/euguBI for more information. -before_build: - - del "C:\Program Files (x86)\MSBuild\14.0\Microsoft.Common.targets\ImportAfter\Xamarin.Common.targets" - - del "C:\Program Files (x86)\MSBuild\12.0\Microsoft.Common.targets\ImportAfter\Xamarin.Common.targets" - build_script: - md _build -Force - cd _build @@ -51,4 +54,3 @@ artifacts: name: logs - path: '_build/Testing/**/*.xml' name: test_results - From 2d1a34626fec7db18e7198aad9e2b2439e1bf186 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Thu, 27 Apr 2017 13:16:49 +0100 Subject: [PATCH 21/60] Fixes #378 (hopefully). Unit tests for counters to follow. The problem was that the call to Finish() the user counters was lost in a big merge. If I had already written the tests for the user counters, this would probably have been catched earlier. --- src/benchmark.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/benchmark.cc b/src/benchmark.cc index 00ffa07f..8d3f406c 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -252,6 +252,7 @@ BenchmarkReporter::Run CreateRunReport( report.complexity = b.complexity; report.complexity_lambda = b.complexity_lambda; report.counters = results.counters; + internal::Finish(&report.counters, seconds, b.threads); } return report; } From 1295ce8f23717d910caf602ee4458212e9a3e746 Mon Sep 17 00:00:00 2001 From: vladoovt Date: Thu, 27 Apr 2017 09:56:43 -0600 Subject: [PATCH 22/60] Fixes #378 coercion to double was causing counter to forget its flags, changed it so that its value is updated directly --- src/console_reporter.cc | 8 +++++++- src/counter.cc | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/console_reporter.cc b/src/console_reporter.cc index 3f3de029..6e2a9c6f 100644 --- a/src/console_reporter.cc +++ b/src/console_reporter.cc @@ -134,7 +134,13 @@ void ConsoleReporter::PrintRunData(const Run& result) { for (auto& c : result.counters) { auto const& s = HumanReadableNumber(c.second.value); - printer(Out, COLOR_DEFAULT, " %s=%s", c.first.c_str(), s.c_str()); + if (c.second.flags & Counter::Flags::kIsRate) { + std::string counter_rate = StrCat(" ", HumanReadableNumber(c.second.value), " ", c.first.c_str(), "/s"); + printer(Out, COLOR_DEFAULT, " %*s", 13, counter_rate.c_str()); + } + else { + printer(Out, COLOR_DEFAULT, " %s=%s", c.first.c_str(), s.c_str()); + } } if (!rate.empty()) { diff --git a/src/counter.cc b/src/counter.cc index 307863d3..2e9d918b 100644 --- a/src/counter.cc +++ b/src/counter.cc @@ -30,7 +30,7 @@ double Finish(Counter const& c, double cpu_time, double num_threads) { void Finish(UserCounters *l, double cpu_time, double num_threads) { for (auto &c : *l) { - c.second = Finish(c.second, cpu_time, num_threads); + c.second.value = Finish(c.second, cpu_time, num_threads); } } @@ -39,7 +39,7 @@ void Increment(UserCounters *l, UserCounters const& r) { for (auto &c : *l) { auto it = r.find(c.first); if (it != r.end()) { - c.second = c.second + it->second; + c.second.value = c.second + it->second; } } // add counters present in r, but not in *l From 409f35da50c3f2db5a8187ea1fb98342bddaa317 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Thu, 27 Apr 2017 19:22:36 +0100 Subject: [PATCH 23/60] User counters: fix misplaced newline in console reporter header. --- src/console_reporter.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/console_reporter.cc b/src/console_reporter.cc index 3f3de029..e1044cd4 100644 --- a/src/console_reporter.cc +++ b/src/console_reporter.cc @@ -53,11 +53,10 @@ bool ConsoleReporter::ReportContext(const Context& context) { void ConsoleReporter::PrintHeader(const Run& run) { std::string str = - FormatString("%-*s %13s %13s %10s\n", static_cast(name_field_width_), - "Benchmark", "Time", "CPU", "Iterations"); - if(!run.counters.empty()) { - str += " UserCounters..."; - } + FormatString("%-*s %13s %13s %10s%s\n", static_cast(name_field_width_), + "Benchmark", "Time", "CPU", "Iterations", + (run.counters.empty() ? "" : " UserCounters...") + ); std::string line = std::string(str.length(), '-'); GetOutputStream() << line << "\n" << str << line << "\n"; } From b273d9b7d5e49d29e60b98776d5cb8fc161952bf Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Thu, 27 Apr 2017 19:24:06 +0100 Subject: [PATCH 24/60] Reporter tests: reuse csv header. --- test/output_test_helper.cc | 3 +++ test/reporter_output_test.cc | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 54c028a6..4f676682 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -39,6 +39,9 @@ SubMap& GetSubstitutions() { {"%time", "[ ]*[0-9]{1,5} ns"}, {"%console_report", "[ ]*[0-9]{1,5} ns [ ]*[0-9]{1,5} ns [ ]*[0-9]+"}, {"%console_us_report", "[ ]*[0-9] us [ ]*[0-9] us [ ]*[0-9]+"}, + {"%csv_header", + "name,iterations,real_time,cpu_time,time_unit,bytes_per_second," + "items_per_second,label,error_occurred,error_message"}, {"%csv_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns,,,,,"}, {"%csv_us_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",us,,,,,"}, {"%csv_bytes_report", diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index cb52aec0..4a481433 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -13,9 +13,7 @@ ADD_CASES(TC_ConsoleOut, {{"^[-]+$", MR_Next}, {"^Benchmark %s Time %s CPU %s Iterations$", MR_Next}, {"^[-]+$", MR_Next}}); -ADD_CASES(TC_CSVOut, - {{"name,iterations,real_time,cpu_time,time_unit,bytes_per_second," - "items_per_second,label,error_occurred,error_message"}}); +ADD_CASES(TC_CSVOut, {{"%csv_header"}}); // ========================================================================= // // ------------------------ Testing Basic Output --------------------------- // From 3c2d7f5348ed0e0a6b0dfda242a8b0211858ebe5 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Thu, 27 Apr 2017 19:25:20 +0100 Subject: [PATCH 25/60] User counter tests: first version. --- test/CMakeLists.txt | 3 ++ test/output_test_helper.cc | 3 ++ test/user_counters_test.cc | 75 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 test/user_counters_test.cc diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 14ba7a6e..8d644f90 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -92,6 +92,9 @@ add_test(multiple_ranges_test multiple_ranges_test --benchmark_min_time=0.01) compile_output_test(reporter_output_test) add_test(reporter_output_test reporter_output_test --benchmark_min_time=0.01) +compile_output_test(user_counters_test) +add_test(user_counters_test user_counters_test --benchmark_min_time=0.01) + check_cxx_compiler_flag(-std=c++03 BENCHMARK_HAS_CXX03_FLAG) if (BENCHMARK_HAS_CXX03_FLAG) set(CXX03_FLAGS "${CMAKE_CXX_FLAGS}") diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 4f676682..0acb09bc 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -48,6 +48,9 @@ SubMap& GetSubstitutions() { "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns," + safe_dec_re + ",,,,"}, {"%csv_items_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns,," + safe_dec_re + ",,,"}, + {"%csv_bytes_items_report", + "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns," + safe_dec_re + + "," + safe_dec_re + ",,,"}, {"%csv_label_report_begin", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns,,,"}, {"%csv_label_report_end", ",,"}}; return map; diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc new file mode 100644 index 00000000..1e75fbae --- /dev/null +++ b/test/user_counters_test.cc @@ -0,0 +1,75 @@ + +#undef NDEBUG +#include + +#include "benchmark/benchmark.h" +#include "output_test.h" + +// ========================================================================= // +// ---------------------- Testing Prologue Output -------------------------- // +// ========================================================================= // + +ADD_CASES(TC_ConsoleOut, + {{"^[-]+$", MR_Next}, + {"^Benchmark %s Time %s CPU %s Iterations UserCounters...$", MR_Next}, + {"^[-]+$", MR_Next}}); +ADD_CASES(TC_CSVOut, {{"%csv_header,\"bar\",\"foo\""}}); + +// ========================================================================= // +// ------------------------- Basic Counters Output ------------------------- // +// ========================================================================= // + +void BM_Counters_Simple(benchmark::State& state) { + while (state.KeepRunning()) { + } + state.counters["foo"] = 1; + state.counters["bar"] = 2; + //state.SetItemsProcessed(150); + //state.SetBytesProcessed(364); +} +BENCHMARK(BM_Counters_Simple);//->ThreadRange(1, 32); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Simple %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Simple\",$"}, + {"\"iterations\": %int,$", MR_Next}, + {"\"real_time\": %int,$", MR_Next}, + {"\"cpu_time\": %int,$", MR_Next}, + {"\"time_unit\": \"ns\",$", MR_Next}, + {"\"bar\": %float,$", MR_Next}, + {"\"foo\": %float$", MR_Next}, + {"}", MR_Next}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Simple\",%csv_report,%float,%float$"}}); + +// ========================================================================= // +// ------------------------- Basic Counters Output ------------------------- // +// ========================================================================= // + +void BM_Counters_WithBytesAndItemsPSec(benchmark::State& state) { + while (state.KeepRunning()) { + } + state.counters["foo"] = 1; + state.counters["bar"] = 2; + state.SetItemsProcessed(150); + state.SetBytesProcessed(364); +} +BENCHMARK(BM_Counters_WithBytesAndItemsPSec);//->ThreadRange(1, 32); +ADD_CASES(TC_ConsoleOut, + {{"^BM_Counters_WithBytesAndItemsPSec %console_report " + "bar=%float foo=%float +%floatB/s +%float items/s$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_WithBytesAndItemsPSec\",$"}, + {"\"iterations\": %int,$", MR_Next}, + {"\"real_time\": %int,$", MR_Next}, + {"\"cpu_time\": %int,$", MR_Next}, + {"\"time_unit\": \"ns\",$", MR_Next}, + {"\"bytes_per_second\": %int,$", MR_Next}, + {"\"items_per_second\": %int,$", MR_Next}, + {"\"bar\": %float,$", MR_Next}, + {"\"foo\": %float$", MR_Next}, + {"}", MR_Next}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_WithBytesAndItemsPSec\"," + "%csv_bytes_items_report,%float,%float$"}}); + +// ========================================================================= // +// --------------------------- TEST CASES END ------------------------------ // +// ========================================================================= // + +int main(int argc, char* argv[]) { RunOutputTests(argc, argv); } From 693a43013d5e7344e5f627bc773aff7fe0151391 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Thu, 27 Apr 2017 22:11:40 +0100 Subject: [PATCH 26/60] User counters: add more unit tests. ... The tests are still missing a way to check actual validity of numerical results; this will be done next. As they currently are, the tests pass, but the problem detected with #378 is still standing and the results with non-standard counters are wrong. --- test/benchmark_test.cc | 17 ------- test/user_counters_test.cc | 98 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 22 deletions(-) diff --git a/test/benchmark_test.cc b/test/benchmark_test.cc index 57731331..7a16466e 100644 --- a/test/benchmark_test.cc +++ b/test/benchmark_test.cc @@ -213,23 +213,6 @@ void BM_non_template_args(benchmark::State& state, int, double) { } BENCHMARK_CAPTURE(BM_non_template_args, basic_test, 0, 0); -static void BM_UserCounter(benchmark::State& state) { - static const int depth = 1024; - while (state.KeepRunning()) { - benchmark::DoNotOptimize(CalculatePi(depth)); - } - state.counters["Foo"] = 1; - state.counters["Bar"] = 2; - state.counters["Baz"] = 3; - state.counters["Bat"] = 5; -#ifdef BENCHMARK_HAS_CXX11 - state.counters.insert({{"Foo", 2}, {"Bar", 3}, {"Baz", 5}, {"Bat", 6}}); -#endif -} -BENCHMARK(BM_UserCounter)->Threads(8); -BENCHMARK(BM_UserCounter)->ThreadRange(1, 32); -BENCHMARK(BM_UserCounter)->ThreadPerCpu(); - #endif // __cplusplus >= 201103L static void BM_DenseThreadRanges(benchmark::State& st) { diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index 1e75fbae..df30d926 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -1,6 +1,5 @@ #undef NDEBUG -#include #include "benchmark/benchmark.h" #include "output_test.h" @@ -16,7 +15,7 @@ ADD_CASES(TC_ConsoleOut, ADD_CASES(TC_CSVOut, {{"%csv_header,\"bar\",\"foo\""}}); // ========================================================================= // -// ------------------------- Basic Counters Output ------------------------- // +// ------------------------- Simple Counters Output ------------------------ // // ========================================================================= // void BM_Counters_Simple(benchmark::State& state) { @@ -24,8 +23,6 @@ void BM_Counters_Simple(benchmark::State& state) { } state.counters["foo"] = 1; state.counters["bar"] = 2; - //state.SetItemsProcessed(150); - //state.SetBytesProcessed(364); } BENCHMARK(BM_Counters_Simple);//->ThreadRange(1, 32); ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Simple %console_report bar=%float foo=%float$"}}); @@ -40,7 +37,7 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Simple\",$"}, ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Simple\",%csv_report,%float,%float$"}}); // ========================================================================= // -// ------------------------- Basic Counters Output ------------------------- // +// --------------------- Counters+Items+Bytes/s Output --------------------- // // ========================================================================= // void BM_Counters_WithBytesAndItemsPSec(benchmark::State& state) { @@ -68,6 +65,97 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_WithBytesAndItemsPSec\",$"}, ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_WithBytesAndItemsPSec\"," "%csv_bytes_items_report,%float,%float$"}}); +// ========================================================================= // +// ------------------------- Rate Counters Output -------------------------- // +// ========================================================================= // + +void BM_Counters_Rate(benchmark::State& state) { + while (state.KeepRunning()) { + } + namespace bm = benchmark; + state.counters["foo"] = bm::Counter{1, bm::Counter::kIsRate}; + state.counters["bar"] = bm::Counter{2, bm::Counter::kIsRate}; +} +BENCHMARK(BM_Counters_Rate);//->ThreadRange(1, 32); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Rate %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Rate\",$"}, + {"\"iterations\": %int,$", MR_Next}, + {"\"real_time\": %int,$", MR_Next}, + {"\"cpu_time\": %int,$", MR_Next}, + {"\"time_unit\": \"ns\",$", MR_Next}, + {"\"bar\": %float,$", MR_Next}, + {"\"foo\": %float$", MR_Next}, + {"}", MR_Next}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Rate\",%csv_report,%float,%float$"}}); + +// ========================================================================= // +// ------------------------- Thread Counters Output ------------------------ // +// ========================================================================= // + +void BM_Counters_Threads(benchmark::State& state) { + while (state.KeepRunning()) { + } + state.counters["foo"] = 1; + state.counters["bar"] = 2; +} +BENCHMARK(BM_Counters_Threads)->ThreadRange(1, 8); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Threads/threads:%int %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Threads/threads:%int\",$"}, + {"\"iterations\": %int,$", MR_Next}, + {"\"real_time\": %int,$", MR_Next}, + {"\"cpu_time\": %int,$", MR_Next}, + {"\"time_unit\": \"ns\",$", MR_Next}, + {"\"bar\": %float,$", MR_Next}, + {"\"foo\": %float$", MR_Next}, + {"}", MR_Next}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Threads/threads:%int\",%csv_report,%float,%float$"}}); + +// ========================================================================= // +// ---------------------- ThreadAvg Counters Output ------------------------ // +// ========================================================================= // + +void BM_Counters_AvgThreads(benchmark::State& state) { + while (state.KeepRunning()) { + } + namespace bm = benchmark; + state.counters["foo"] = bm::Counter{1, bm::Counter::kAvgThreads}; + state.counters["bar"] = bm::Counter{2, bm::Counter::kAvgThreads}; +} +BENCHMARK(BM_Counters_AvgThreads)->ThreadRange(1, 8); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_AvgThreads/threads:%int %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreads/threads:%int\",$"}, + {"\"iterations\": %int,$", MR_Next}, + {"\"real_time\": %int,$", MR_Next}, + {"\"cpu_time\": %int,$", MR_Next}, + {"\"time_unit\": \"ns\",$", MR_Next}, + {"\"bar\": %float,$", MR_Next}, + {"\"foo\": %float$", MR_Next}, + {"}", MR_Next}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreads/threads:%int\",%csv_report,%float,%float$"}}); + +// ========================================================================= // +// ---------------------- ThreadAvg Counters Output ------------------------ // +// ========================================================================= // + +void BM_Counters_AvgThreadsRate(benchmark::State& state) { + while (state.KeepRunning()) { + } + namespace bm = benchmark; + state.counters["foo"] = bm::Counter{1, bm::Counter::kAvgThreadsRate}; + state.counters["bar"] = bm::Counter{2, bm::Counter::kAvgThreadsRate}; +} +BENCHMARK(BM_Counters_AvgThreadsRate)->ThreadRange(1, 8); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_AvgThreadsRate/threads:%int %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreadsRate/threads:%int\",$"}, + {"\"iterations\": %int,$", MR_Next}, + {"\"real_time\": %int,$", MR_Next}, + {"\"cpu_time\": %int,$", MR_Next}, + {"\"time_unit\": \"ns\",$", MR_Next}, + {"\"bar\": %float,$", MR_Next}, + {"\"foo\": %float$", MR_Next}, + {"}", MR_Next}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreadsRate/threads:%int\",%csv_report,%float,%float$"}}); + // ========================================================================= // // --------------------------- TEST CASES END ------------------------------ // // ========================================================================= // From 6452883027b6642157a54cb5ec61c55c34f4948e Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Fri, 28 Apr 2017 15:02:27 +0100 Subject: [PATCH 27/60] Unit testing: add facilities to check benchmark results. This is needed for examining the values of user counters (needed for #348). It is also needed for checking the values of standard benchmark results like items_processed or complexities (for example, checking the standard deviation is needed for unit testing #357 as discussed in #362). --- test/output_test.h | 73 +++++++++++++++++++ test/output_test_helper.cc | 139 +++++++++++++++++++++++++++++++++++++ test/user_counters_test.cc | 28 +++++--- 3 files changed, 232 insertions(+), 8 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 57d4397a..0bc2c7ba 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -58,6 +58,79 @@ int SetSubstitutions( // Run all output tests. void RunOutputTests(int argc, char* argv[]); +// ========================================================================= // +// ------------------------- Results checking ------------------------------ // +// ========================================================================= // + +struct ResultsCheckerEntry; +typedef std::function< void(ResultsCheckerEntry const&) > ResultsCheckFn; + +// Class to test the results of a benchmark. +// It inspects the results by looking at the CSV output of a subscribed +// benchmark. +struct ResultsCheckerEntry { + std::string name; + std::map< std::string, std::string > values; + ResultsCheckFn check_fn; + + // int NumThreads() const; // TODO + // double duration_real_time() const {} // TODO + // double duration_cpu_time() const {} // TODO + + // get the string for a result by name, or nullptr if the name + // is not found + const std::string* Get(std::string const& entry_name) const { + auto it = values.find(entry_name); + if(it == values.end()) return nullptr; + return &it->second; + } + + // get a result by name, parsed as a specific type. + // For counters, use GetCounterAs instead. + template< class T > T GetAs(std::string const& entry_name) const { + auto *sv = Get(entry_name); + CHECK(sv != nullptr && !sv->empty()); + std::stringstream ss; + ss << *sv; + T out; + ss >> out; + CHECK(!ss.fail()); + return out; + } + + // counters are written as doubles, so they have to be read first + // as a double, and only then converted to the asked type. + template< class T > T GetCounterAs(std::string const& entry_name) const { + double dval = GetAs< double >(entry_name); + T tval = static_cast< T >(dval); + return tval; + } + + +}; + + +#define _CHECK_RESULT_VALUE(entry, getfn, var_type, var_name, relationship, value) \ + CONCAT(CHECK_, relationship)(entry.getfn< var_type >(var_name), (value)) \ + << "\n" << __FILE__ << ":" << __LINE__ << ": " \ + << entry.name << ": expected (" << #var_type << ")" \ + << var_name << "=" << entry.GetAs< var_type >(var_name) \ + << " to be " #relationship " to " << (value); + +#define CHECK_RESULT_VALUE(entry, var_type, var_name, relationship, value) \ + _CHECK_RESULT_VALUE(entry, GetAs, var_type, var_name, relationship, value) + +#define CHECK_COUNTER_VALUE(entry, var_type, var_name, relationship, value) \ + _CHECK_RESULT_VALUE(entry, GetCounterAs, var_type, var_name, relationship, value) + +#define CHECK_BENCHMARK_RESULTS(bm_name, checker_function) \ + size_t CONCAT(dummy, __LINE__) = AddChecker(bm_name, checker_function) + +// Add a function to check the (CSV) results of a benchmark. These +// functions will be called only after the output was successfully +// checked. +size_t AddChecker(const char* bm_name, ResultsCheckFn fn); + // ========================================================================= // // --------------------------- Misc Utilities ------------------------------ // // ========================================================================= // diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 0acb09bc..3209d90c 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -34,6 +34,8 @@ SubMap& GetSubstitutions() { static std::string safe_dec_re = "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?"; static SubMap map = { {"%float", "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?"}, + // human-readable float + {"%hrfloat", "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?[kMGTPEZYmunpfazy]?"}, {"%int", "[ ]*[0-9]+"}, {" %s ", "[ ]+"}, {"%time", "[ ]*[0-9]{1,5} ns"}, @@ -146,8 +148,139 @@ class TestReporter : public benchmark::BenchmarkReporter { std::vector reporters_; }; } + } // end namespace internal +// ========================================================================= // +// -------------------------- Results checking ----------------------------- // +// ========================================================================= // + +namespace internal { + +// Utility class to manage subscribers for checking benchmark results. +// It works by parsing the CSV output to read the results. +class ResultsChecker { + public: + + std::map< std::string, ResultsCheckerEntry > results; + std::vector< std::string > result_names; + + void Add(const std::string& entry_name, ResultsCheckFn fn); + + void CheckResults(std::stringstream& output); + + private: + + ResultsCheckerEntry* Find_(const std::string& entry_name); + + void SetHeader_(const std::string& csv_header); + void SetValues_(const std::string& entry_csv_line); + + std::vector< std::string > SplitCsv_(std::string const& line); + +}; + +// store the static ResultsChecker in a function to prevent initialization +// order problems +ResultsChecker& GetResultsChecker() { + static ResultsChecker rc; + return rc; +} + +// add a results checker for a benchmark +void ResultsChecker::Add(const std::string& entry_name, ResultsCheckFn fn) { + results[entry_name] = {entry_name, {}, fn}; +} + +// check the results of all subscribed benchmarks +void ResultsChecker::CheckResults(std::stringstream& output) +{ + // first reset the stream to the start + { + auto start = std::ios::streampos(0); + // clear before calling tellg() + output.clear(); + // seek to zero only when needed + if(output.tellg() > start) output.seekg(start); + // and just in case + output.clear(); + } + // now go over every line and publish it to the ResultsChecker + std::string line; + bool on_first = true; + while (output.eof() == false) { + CHECK(output.good()); + std::getline(output, line); + if (on_first) { + SetHeader_(line); // this is important + on_first = false; + continue; + } + SetValues_(line); + } + // finally we can call the subscribed check functions + for(const auto& p : results) { + CHECK(p.second.check_fn); + p.second.check_fn(p.second); + } +} + +// prepare for the names in this header +void ResultsChecker::SetHeader_(const std::string& csv_header) { + result_names = SplitCsv_(csv_header); +} + +// set the values for subscribed benchmarks, and silently ignore all others +void ResultsChecker::SetValues_(const std::string& entry_csv_line) { + CHECK(!result_names.empty()); + auto vals = SplitCsv_(entry_csv_line); + if(vals.empty()) return; + CHECK_EQ(vals.size(), result_names.size()); + ResultsCheckerEntry* entry = Find_(vals[0]); + if(!entry) return; + for (size_t i = 1, e = vals.size(); i < e; ++i) { + entry->values[result_names[i]] = vals[i]; + } +} + +// find a subscribed benchmark, or return null +ResultsCheckerEntry* ResultsChecker::Find_(const std::string& entry_name) { + auto it = results.find(entry_name); + if(it == results.end()) return nullptr; + return &it->second; +} + +// a quick'n'dirty csv splitter (eliminating quotes) +std::vector< std::string > ResultsChecker::SplitCsv_(std::string const& line) { + std::vector< std::string > out; + if(line.empty()) return out; + if(!result_names.empty()) out.reserve(result_names.size()); + size_t prev = 0, pos = line.find_first_of(','), curr = pos; + while(pos != line.npos) { + CHECK(curr > 0); + if(line[prev] == '"') ++prev; + if(line[curr-1] == '"') --curr; + out.push_back(line.substr(prev, curr-prev)); + prev = pos + 1; + pos = line.find_first_of(',', pos + 1); + curr = pos; + } + curr = line.size(); + if(line[prev] == '"') ++prev; + if(line[curr-1] == '"') --curr; + out.push_back(line.substr(prev, curr-prev)); + return out; +} + +} // end namespace internal + +size_t AddChecker(const char* bm_name, ResultsCheckFn fn) +{ + auto &rc = internal::GetResultsChecker(); + rc.Add(bm_name, fn); + return rc.results.size(); +} + // ========================================================================= // // -------------------------- Public API Definitions------------------------ // // ========================================================================= // @@ -237,4 +370,10 @@ void RunOutputTests(int argc, char* argv[]) { std::cout << "\n"; } + + // now that we know the output is as expected, we can dispatch + // the checks to subscribees. + auto &csv = TestCases[2]; + CHECK(strcmp(csv.name, "CSVReporter") == 0); // would use == but gcc spits a warning + internal::GetResultsChecker().CheckResults(csv.out_stream); } diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index df30d926..84736b78 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -25,7 +25,7 @@ void BM_Counters_Simple(benchmark::State& state) { state.counters["bar"] = 2; } BENCHMARK(BM_Counters_Simple);//->ThreadRange(1, 32); -ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Simple %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Simple %console_report bar=%hrfloat foo=%hrfloat$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Simple\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, @@ -35,23 +35,28 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Simple\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Simple\",%csv_report,%float,%float$"}}); +CHECK_BENCHMARK_RESULTS("BM_Counters_Simple", [](ResultsCheckerEntry const& e) { + CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); + CHECK_COUNTER_VALUE(e, int, "bar", EQ, 2); +}); // ========================================================================= // // --------------------- Counters+Items+Bytes/s Output --------------------- // // ========================================================================= // +namespace { int num_calls1 = 0; } void BM_Counters_WithBytesAndItemsPSec(benchmark::State& state) { while (state.KeepRunning()) { } state.counters["foo"] = 1; - state.counters["bar"] = 2; - state.SetItemsProcessed(150); + state.counters["bar"] = ++num_calls1; state.SetBytesProcessed(364); + state.SetItemsProcessed(150); } BENCHMARK(BM_Counters_WithBytesAndItemsPSec);//->ThreadRange(1, 32); ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_WithBytesAndItemsPSec %console_report " - "bar=%float foo=%float +%floatB/s +%float items/s$"}}); + "bar=%hrfloat foo=%hrfloat +%floatB/s +%float items/s$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_WithBytesAndItemsPSec\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, @@ -64,6 +69,13 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_WithBytesAndItemsPSec\",$"}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_WithBytesAndItemsPSec\"," "%csv_bytes_items_report,%float,%float$"}}); +CHECK_BENCHMARK_RESULTS("BM_Counters_WithBytesAndItemsPSec", + [](ResultsCheckerEntry const& e) { + CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); + CHECK_COUNTER_VALUE(e, int, "bar", EQ, num_calls1); + //TODO CHECK_RESULT_VALUE(e, int, "bytes_per_second", EQ, 364 / e.duration_cpu_time()); + //TODO CHECK_RESULT_VALUE(e, int, "items_per_second", EQ, 150 / e.duration_cpu_time()); +}); // ========================================================================= // // ------------------------- Rate Counters Output -------------------------- // @@ -77,7 +89,7 @@ void BM_Counters_Rate(benchmark::State& state) { state.counters["bar"] = bm::Counter{2, bm::Counter::kIsRate}; } BENCHMARK(BM_Counters_Rate);//->ThreadRange(1, 32); -ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Rate %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Rate %console_report bar=%hrfloat foo=%hrfloat$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Rate\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, @@ -99,7 +111,7 @@ void BM_Counters_Threads(benchmark::State& state) { state.counters["bar"] = 2; } BENCHMARK(BM_Counters_Threads)->ThreadRange(1, 8); -ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Threads/threads:%int %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Threads/threads:%int %console_report bar=%hrfloat foo=%hrfloat$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Threads/threads:%int\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, @@ -122,7 +134,7 @@ void BM_Counters_AvgThreads(benchmark::State& state) { state.counters["bar"] = bm::Counter{2, bm::Counter::kAvgThreads}; } BENCHMARK(BM_Counters_AvgThreads)->ThreadRange(1, 8); -ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_AvgThreads/threads:%int %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_AvgThreads/threads:%int %console_report bar=%hrfloat foo=%hrfloat$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreads/threads:%int\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, @@ -145,7 +157,7 @@ void BM_Counters_AvgThreadsRate(benchmark::State& state) { state.counters["bar"] = bm::Counter{2, bm::Counter::kAvgThreadsRate}; } BENCHMARK(BM_Counters_AvgThreadsRate)->ThreadRange(1, 8); -ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_AvgThreadsRate/threads:%int %console_report bar=%float foo=%float$"}}); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_AvgThreadsRate/threads:%int %console_report bar=%hrfloat foo=%hrfloat$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreadsRate/threads:%int\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, From e869e3749a2a75654f92a9dfc5258530920e862a Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Fri, 28 Apr 2017 15:38:21 +0100 Subject: [PATCH 28/60] Remove some whitespace. --- test/output_test.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 0bc2c7ba..f6aa9911 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -105,11 +105,8 @@ struct ResultsCheckerEntry { T tval = static_cast< T >(dval); return tval; } - - }; - #define _CHECK_RESULT_VALUE(entry, getfn, var_type, var_name, relationship, value) \ CONCAT(CHECK_, relationship)(entry.getfn< var_type >(var_name), (value)) \ << "\n" << __FILE__ << ":" << __LINE__ << ": " \ From 8adf59d7620616f435cbe37907a5eec3faba9712 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Fri, 28 Apr 2017 20:40:21 +0100 Subject: [PATCH 29/60] Add epsilon check macros for float comparison. --- src/check.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/check.h b/src/check.h index 6f1fe0cf..86383596 100644 --- a/src/check.h +++ b/src/check.h @@ -3,6 +3,7 @@ #include #include +#include #include "internal_macros.h" #include "log.h" @@ -68,4 +69,11 @@ class CheckHandler { #define CHECK_GT(a, b) CHECK((a) > (b)) #define CHECK_LT(a, b) CHECK((a) < (b)) +#define CHECK_EQ_EPS(a, b, eps) CHECK(std::fabs((a) - (b)) < eps) +#define CHECK_NE_EPS(a, b, eps) CHECK(std::fabs((a) - (b)) >= eps) +#define CHECK_GE_EPS(a, b, eps) CHECK((a) - (b) > -eps) +#define CHECK_LE_EPS(a, b, eps) CHECK((b) - (a) > -eps) +#define CHECK_GT_EPS(a, b, eps) CHECK((a) - (b) > eps) +#define CHECK_LT_EPS(a, b, eps) CHECK((b) - (a) > eps) + #endif // CHECK_H_ From 2a8d0dd1b1c128d3058898bbbadc3099a0434915 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Fri, 28 Apr 2017 20:42:28 +0100 Subject: [PATCH 30/60] Use const char* instead of std::string in entry name lookup. --- test/output_test.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index f6aa9911..066eb53c 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -79,7 +79,7 @@ struct ResultsCheckerEntry { // get the string for a result by name, or nullptr if the name // is not found - const std::string* Get(std::string const& entry_name) const { + const std::string* Get(const char* entry_name) const { auto it = values.find(entry_name); if(it == values.end()) return nullptr; return &it->second; @@ -87,7 +87,7 @@ struct ResultsCheckerEntry { // get a result by name, parsed as a specific type. // For counters, use GetCounterAs instead. - template< class T > T GetAs(std::string const& entry_name) const { + template< class T > T GetAs(const char* entry_name) const { auto *sv = Get(entry_name); CHECK(sv != nullptr && !sv->empty()); std::stringstream ss; @@ -100,7 +100,7 @@ struct ResultsCheckerEntry { // counters are written as doubles, so they have to be read first // as a double, and only then converted to the asked type. - template< class T > T GetCounterAs(std::string const& entry_name) const { + template< class T > T GetCounterAs(const char* entry_name) const { double dval = GetAs< double >(entry_name); T tval = static_cast< T >(dval); return tval; From 1826feb164f0f935900e1819d70b60106f56ffc3 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Fri, 28 Apr 2017 20:43:44 +0100 Subject: [PATCH 31/60] ResultsCheckerEntry: add more getter functions. --- test/output_test.h | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 066eb53c..57330a7b 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -73,9 +73,26 @@ struct ResultsCheckerEntry { std::map< std::string, std::string > values; ResultsCheckFn check_fn; - // int NumThreads() const; // TODO - // double duration_real_time() const {} // TODO - // double duration_cpu_time() const {} // TODO + int NumThreads() const { + auto pos = name.find_last_of("/threads:"); + if(pos == name.npos) return 1; + auto end = name.find_last_of('/', pos + 9); + std::stringstream ss; + ss << name.substr(pos + 9, end); + int num = 1; + ss >> num; + CHECK(!ss.fail()); + return num; + } + + // get the real_time duration of the benchmark in seconds + double DurationRealTime() const { + return GetAs< double >("iterations") * GetTime("real_time"); + } + // get the cpu_time duration of the benchmark in seconds + double DurationCPUTime() const { + return GetAs< double >("iterations") * GetTime("cpu_time"); + } // get the string for a result by name, or nullptr if the name // is not found @@ -105,6 +122,25 @@ struct ResultsCheckerEntry { T tval = static_cast< T >(dval); return tval; } + + // get cpu_time or real_time in seconds + double GetTime(const char* which) const { + double val = GetAs< double >(which); + auto unit = Get("time_unit"); + CHECK(unit); + if(*unit == "ns") { + return val * 1.e-9; + } else if(*unit == "us") { + return val * 1.e-6; + } else if(*unit == "ms") { + return val * 1.e-3; + } else if(*unit == "s") { + return val; + } else { + CHECK(1 == 0) << "unknown time unit: " << *unit; + return 0; + } + } }; #define _CHECK_RESULT_VALUE(entry, getfn, var_type, var_name, relationship, value) \ From 8c757a3bb9891064172d7b1ec52965a05d0fdf8a Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Fri, 28 Apr 2017 20:44:27 +0100 Subject: [PATCH 32/60] Results check: add checks with epsilon. --- test/output_test.h | 54 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 57330a7b..072af359 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -143,27 +143,49 @@ struct ResultsCheckerEntry { } }; -#define _CHECK_RESULT_VALUE(entry, getfn, var_type, var_name, relationship, value) \ - CONCAT(CHECK_, relationship)(entry.getfn< var_type >(var_name), (value)) \ - << "\n" << __FILE__ << ":" << __LINE__ << ": " \ - << entry.name << ": expected (" << #var_type << ")" \ - << var_name << "=" << entry.GetAs< var_type >(var_name) \ - << " to be " #relationship " to " << (value); - -#define CHECK_RESULT_VALUE(entry, var_type, var_name, relationship, value) \ - _CHECK_RESULT_VALUE(entry, GetAs, var_type, var_name, relationship, value) - -#define CHECK_COUNTER_VALUE(entry, var_type, var_name, relationship, value) \ - _CHECK_RESULT_VALUE(entry, GetCounterAs, var_type, var_name, relationship, value) - -#define CHECK_BENCHMARK_RESULTS(bm_name, checker_function) \ - size_t CONCAT(dummy, __LINE__) = AddChecker(bm_name, checker_function) - // Add a function to check the (CSV) results of a benchmark. These // functions will be called only after the output was successfully // checked. size_t AddChecker(const char* bm_name, ResultsCheckFn fn); +#ifdef __clang__ + /* NOTE: using , ## __VA_ARGS__ to deal with zero-args calls to + * variadic macros is not portable, but works in clang, gcc, msvc, icc. + * clang requires switching off compiler warnings for pedantic mode. + * @see http://stackoverflow.com/questions/32047685/variadic-macro-without-arguments */ +# pragma clang diagnostic push + // warning: token pasting of ',' and __VA_ARGS__ is a GNU extension +# pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" +#elif defined(__GNUC__) + /* GCC also issues a warning for zero-args calls to variadic macros. + * This warning is switched on with -pedantic and apparently there is no + * easy way to turn it off as with clang. But marking this as a system + * header works. + * @see https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html + * @see http://stackoverflow.com/questions/35587137/ */ +# pragma GCC system_header +#endif + +#define _CHECK_RESULT_VALUE(entry, getfn, var_type, var_name, relationship, value, ...) \ + CONCAT(CHECK_, relationship)(entry.getfn< var_type >(var_name), (value), ## __VA_ARGS__) \ + << "\n" << __FILE__ << ":" << __LINE__ << ": " \ + << entry.name << ": expected (" << #var_type << ")" \ + << var_name << "=" << entry.getfn< var_type >(var_name) \ + << " to be " #relationship " to " << (value); + +#define CHECK_RESULT_VALUE(entry, var_type, var_name, relationship, value, ...) \ + _CHECK_RESULT_VALUE(entry, GetAs, var_type, var_name, relationship, value, ## __VA_ARGS__) + +#define CHECK_COUNTER_VALUE(entry, var_type, var_name, relationship, value, ...) \ + _CHECK_RESULT_VALUE(entry, GetCounterAs, var_type, var_name, relationship, value, ## __VA_ARGS__) + +#ifdef __clang__ +# pragma clang diagnostic pop +#endif + +#define CHECK_BENCHMARK_RESULTS(bm_name, checker_function) \ + size_t CONCAT(dummy, __LINE__) = AddChecker(bm_name, checker_function) + // ========================================================================= // // --------------------------- Misc Utilities ------------------------------ // // ========================================================================= // From da69e5de4579df378f1148ae4d5edd47ded57531 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Fri, 28 Apr 2017 20:45:30 +0100 Subject: [PATCH 33/60] User counters: add more tests. --- test/user_counters_test.cc | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index 84736b78..38415005 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -22,7 +22,7 @@ void BM_Counters_Simple(benchmark::State& state) { while (state.KeepRunning()) { } state.counters["foo"] = 1; - state.counters["bar"] = 2; + state.counters["bar"] = 2 * state.iterations(); } BENCHMARK(BM_Counters_Simple);//->ThreadRange(1, 32); ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Simple %console_report bar=%hrfloat foo=%hrfloat$"}}); @@ -36,8 +36,10 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Simple\",$"}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Simple\",%csv_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_Simple", [](ResultsCheckerEntry const& e) { + double its = e.GetAs< double >("iterations"); CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); - CHECK_COUNTER_VALUE(e, int, "bar", EQ, 2); + // check that the value of bar is within 0.1% of the expected value + CHECK_COUNTER_VALUE(e, double, "bar", EQ_EPS, 2. * its, 0.001 * its); }); // ========================================================================= // @@ -73,8 +75,10 @@ CHECK_BENCHMARK_RESULTS("BM_Counters_WithBytesAndItemsPSec", [](ResultsCheckerEntry const& e) { CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); CHECK_COUNTER_VALUE(e, int, "bar", EQ, num_calls1); - //TODO CHECK_RESULT_VALUE(e, int, "bytes_per_second", EQ, 364 / e.duration_cpu_time()); - //TODO CHECK_RESULT_VALUE(e, int, "items_per_second", EQ, 150 / e.duration_cpu_time()); + // check that the values are within 0.1% of the expected values + double t = e.DurationCPUTime(); // this (and not real time) is the time used + CHECK_RESULT_VALUE(e, double, "bytes_per_second", EQ_EPS, 364. / t, 0.001 * t); + CHECK_RESULT_VALUE(e, double, "items_per_second", EQ_EPS, 150. / t, 0.001 * t); }); // ========================================================================= // @@ -99,6 +103,13 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Rate\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Rate\",%csv_report,%float,%float$"}}); +CHECK_BENCHMARK_RESULTS("BM_Counters_Rate", + [](ResultsCheckerEntry const& e) { + // check that the values are within 0.1% of the expected values + double t = e.DurationCPUTime(); // this (and not real time) is the time used + CHECK_COUNTER_VALUE(e, double, "foo", EQ_EPS, 5. / t, 0.001 * t); + CHECK_COUNTER_VALUE(e, double, "bar", EQ_EPS, 2. / t, 0.001 * t); +}); // ========================================================================= // // ------------------------- Thread Counters Output ------------------------ // From 1ce286f632fb9d741a6d6828c7582de09207ebb0 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 18:26:30 +0100 Subject: [PATCH 34/60] Avoid compiler-specific pragmas in result check macros. - Epsilon is now understood as relative to expected value. - Improve error messages for epsilon checks. --- test/output_test.h | 70 ++++++++++++++++++++++---------------- test/user_counters_test.cc | 22 ++++++------ 2 files changed, 51 insertions(+), 41 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 072af359..247f7005 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -148,40 +148,50 @@ struct ResultsCheckerEntry { // checked. size_t AddChecker(const char* bm_name, ResultsCheckFn fn); -#ifdef __clang__ - /* NOTE: using , ## __VA_ARGS__ to deal with zero-args calls to - * variadic macros is not portable, but works in clang, gcc, msvc, icc. - * clang requires switching off compiler warnings for pedantic mode. - * @see http://stackoverflow.com/questions/32047685/variadic-macro-without-arguments */ -# pragma clang diagnostic push - // warning: token pasting of ',' and __VA_ARGS__ is a GNU extension -# pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" -#elif defined(__GNUC__) - /* GCC also issues a warning for zero-args calls to variadic macros. - * This warning is switched on with -pedantic and apparently there is no - * easy way to turn it off as with clang. But marking this as a system - * header works. - * @see https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html - * @see http://stackoverflow.com/questions/35587137/ */ -# pragma GCC system_header -#endif +//---------------------------------- +// Macros to help in result checking. Do not use them with arguments causing +// side-effects. -#define _CHECK_RESULT_VALUE(entry, getfn, var_type, var_name, relationship, value, ...) \ - CONCAT(CHECK_, relationship)(entry.getfn< var_type >(var_name), (value), ## __VA_ARGS__) \ - << "\n" << __FILE__ << ":" << __LINE__ << ": " \ - << entry.name << ": expected (" << #var_type << ")" \ - << var_name << "=" << entry.getfn< var_type >(var_name) \ - << " to be " #relationship " to " << (value); +#define _CHECK_RESULT_VALUE(entry, getfn, var_type, var_name, relationship, value) \ + CONCAT(CHECK_, relationship) \ + (entry.getfn< var_type >(var_name), (value)) << "\n" \ + << __FILE__ << ":" << __LINE__ << ": " << (entry).name << ":\n" \ + << __FILE__ << ":" << __LINE__ << ": " \ + << "expected (" << #var_type << ")" << (var_name) \ + << "=" << (entry).getfn< var_type >(var_name) \ + << " to be " #relationship " to " << (value) << "\n" -#define CHECK_RESULT_VALUE(entry, var_type, var_name, relationship, value, ...) \ - _CHECK_RESULT_VALUE(entry, GetAs, var_type, var_name, relationship, value, ## __VA_ARGS__) +// check with tolerance. eps_factor is the tolerance window, which will be +// interpreted relative to value. +#define _CHECK_RESULT_VALUE_EPS(entry, getfn, var_type, var_name, relationship, value, eps_factor) \ + CONCAT(CHECK_, relationship) \ + (entry.getfn< var_type >(var_name), (value), (eps_factor) * (value)) << "\n" \ + << __FILE__ << ":" << __LINE__ << ": " << (entry).name << ":\n" \ + << __FILE__ << ":" << __LINE__ << ": " \ + << "expected (" << #var_type << ")" << (var_name) \ + << "=" << (entry).getfn< var_type >(var_name) \ + << " to be " #relationship " to " << (value) << "\n" \ + << __FILE__ << ":" << __LINE__ << ": " \ + << "with tolerance of " << (eps_factor) * (value) \ + << " (" << (eps_factor)*100. << "%), " \ + << "but delta was " << ((entry).getfn< var_type >(var_name) - (value)) \ + << " (" << (((entry).getfn< var_type >(var_name) - (value)) \ + / \ + ((value) > 1.e-5 || value < -1.e-5 ? value : 1.e-5)*100.) \ + << "%)" -#define CHECK_COUNTER_VALUE(entry, var_type, var_name, relationship, value, ...) \ - _CHECK_RESULT_VALUE(entry, GetCounterAs, var_type, var_name, relationship, value, ## __VA_ARGS__) +#define CHECK_RESULT_VALUE(entry, var_type, var_name, relationship, value) \ + _CHECK_RESULT_VALUE(entry, GetAs, var_type, var_name, relationship, value) + +#define CHECK_COUNTER_VALUE(entry, var_type, var_name, relationship, value) \ + _CHECK_RESULT_VALUE(entry, GetCounterAs, var_type, var_name, relationship, value) + +#define CHECK_RESULT_VALUE_EPS(entry, var_name, relationship, value, eps_factor) \ + _CHECK_RESULT_VALUE_EPS(entry, GetAs, double, var_name, relationship, value, eps_factor) + +#define CHECK_COUNTER_VALUE_EPS(entry, var_name, relationship, value, eps_factor) \ + _CHECK_RESULT_VALUE_EPS(entry, GetCounterAs, double, var_name, relationship, value, eps_factor) -#ifdef __clang__ -# pragma clang diagnostic pop -#endif #define CHECK_BENCHMARK_RESULTS(bm_name, checker_function) \ size_t CONCAT(dummy, __LINE__) = AddChecker(bm_name, checker_function) diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index 38415005..159e05fe 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -24,7 +24,7 @@ void BM_Counters_Simple(benchmark::State& state) { state.counters["foo"] = 1; state.counters["bar"] = 2 * state.iterations(); } -BENCHMARK(BM_Counters_Simple);//->ThreadRange(1, 32); +BENCHMARK(BM_Counters_Simple); ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Simple %console_report bar=%hrfloat foo=%hrfloat$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Simple\",$"}, {"\"iterations\": %int,$", MR_Next}, @@ -39,7 +39,7 @@ CHECK_BENCHMARK_RESULTS("BM_Counters_Simple", [](ResultsCheckerEntry const& e) { double its = e.GetAs< double >("iterations"); CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); // check that the value of bar is within 0.1% of the expected value - CHECK_COUNTER_VALUE(e, double, "bar", EQ_EPS, 2. * its, 0.001 * its); + CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2.*its, 0.001); }); // ========================================================================= // @@ -55,10 +55,10 @@ void BM_Counters_WithBytesAndItemsPSec(benchmark::State& state) { state.SetBytesProcessed(364); state.SetItemsProcessed(150); } -BENCHMARK(BM_Counters_WithBytesAndItemsPSec);//->ThreadRange(1, 32); +BENCHMARK(BM_Counters_WithBytesAndItemsPSec); ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_WithBytesAndItemsPSec %console_report " - "bar=%hrfloat foo=%hrfloat +%floatB/s +%float items/s$"}}); + "bar=%hrfloat foo=%hrfloat +%hrfloatB/s +%hrfloat items/s$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_WithBytesAndItemsPSec\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, @@ -73,12 +73,12 @@ ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_WithBytesAndItemsPSec\"," "%csv_bytes_items_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_WithBytesAndItemsPSec", [](ResultsCheckerEntry const& e) { + double t = e.DurationCPUTime(); // this (and not real time) is the time used CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); CHECK_COUNTER_VALUE(e, int, "bar", EQ, num_calls1); // check that the values are within 0.1% of the expected values - double t = e.DurationCPUTime(); // this (and not real time) is the time used - CHECK_RESULT_VALUE(e, double, "bytes_per_second", EQ_EPS, 364. / t, 0.001 * t); - CHECK_RESULT_VALUE(e, double, "items_per_second", EQ_EPS, 150. / t, 0.001 * t); + CHECK_RESULT_VALUE_EPS(e, "bytes_per_second", EQ_EPS, 364./t, 0.001); + CHECK_RESULT_VALUE_EPS(e, "items_per_second", EQ_EPS, 150./t, 0.001); }); // ========================================================================= // @@ -92,7 +92,7 @@ void BM_Counters_Rate(benchmark::State& state) { state.counters["foo"] = bm::Counter{1, bm::Counter::kIsRate}; state.counters["bar"] = bm::Counter{2, bm::Counter::kIsRate}; } -BENCHMARK(BM_Counters_Rate);//->ThreadRange(1, 32); +BENCHMARK(BM_Counters_Rate); ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Rate %console_report bar=%hrfloat foo=%hrfloat$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Rate\",$"}, {"\"iterations\": %int,$", MR_Next}, @@ -105,10 +105,10 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Rate\",$"}, ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Rate\",%csv_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_Rate", [](ResultsCheckerEntry const& e) { - // check that the values are within 0.1% of the expected values double t = e.DurationCPUTime(); // this (and not real time) is the time used - CHECK_COUNTER_VALUE(e, double, "foo", EQ_EPS, 5. / t, 0.001 * t); - CHECK_COUNTER_VALUE(e, double, "bar", EQ_EPS, 2. / t, 0.001 * t); + // check that the values are within 0.1% of the expected values + CHECK_COUNTER_VALUE_EPS(e, "foo", EQ_EPS, 1./t, 0.001); + CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2./t, 0.001); }); // ========================================================================= // From 738fcd9e6a947f979667e431ed126cf3ae5ee268 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 18:30:28 +0100 Subject: [PATCH 35/60] Add log of the benchmark name when checking results. --- test/output_test_helper.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 3209d90c..6e463365 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -220,8 +220,10 @@ void ResultsChecker::CheckResults(std::stringstream& output) } // finally we can call the subscribed check functions for(const auto& p : results) { + VLOG(1) << "Checking results of " << p.second.name << ": ... \n"; CHECK(p.second.check_fn); p.second.check_fn(p.second); + VLOG(1) << "Checking results of " << p.second.name << ": OK.\n"; } } From 92034a8b84fd97306b1abe2ed1d3b733b3d03d5d Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 19:02:07 +0100 Subject: [PATCH 36/60] Make result checkers execute on all regex-matching benchmarks. --- test/output_test.h | 3 +- test/output_test_helper.cc | 58 ++++++++++++++++++++++---------------- test/user_counters_test.cc | 4 +++ 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 247f7005..7872326e 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -71,7 +71,8 @@ typedef std::function< void(ResultsCheckerEntry const&) > ResultsCheckFn; struct ResultsCheckerEntry { std::string name; std::map< std::string, std::string > values; - ResultsCheckFn check_fn; + + ResultsCheckerEntry(std::string const& n) : name(n) {} int NumThreads() const { auto pos = name.find_last_of("/threads:"); diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 6e463365..2b681fa0 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -162,10 +162,17 @@ namespace internal { class ResultsChecker { public: - std::map< std::string, ResultsCheckerEntry > results; - std::vector< std::string > result_names; + struct PatternAndFn : public TestCase { // reusing TestCase for its regexes + PatternAndFn(const std::string& regex, ResultsCheckFn fn_) + : TestCase(regex), fn(fn_) {} + ResultsCheckFn fn; + }; - void Add(const std::string& entry_name, ResultsCheckFn fn); + std::vector< PatternAndFn > check_patterns; + std::vector< ResultsCheckerEntry > results; + std::vector< std::string > field_names; + + void Add(const std::string& entry_pattern, ResultsCheckFn fn); void CheckResults(std::stringstream& output); @@ -188,8 +195,8 @@ ResultsChecker& GetResultsChecker() { } // add a results checker for a benchmark -void ResultsChecker::Add(const std::string& entry_name, ResultsCheckFn fn) { - results[entry_name] = {entry_name, {}, fn}; +void ResultsChecker::Add(const std::string& entry_pattern, ResultsCheckFn fn) { + check_patterns.emplace_back(entry_pattern, fn); } // check the results of all subscribed benchmarks @@ -219,44 +226,47 @@ void ResultsChecker::CheckResults(std::stringstream& output) SetValues_(line); } // finally we can call the subscribed check functions - for(const auto& p : results) { - VLOG(1) << "Checking results of " << p.second.name << ": ... \n"; - CHECK(p.second.check_fn); - p.second.check_fn(p.second); - VLOG(1) << "Checking results of " << p.second.name << ": OK.\n"; + for(const auto& p : check_patterns) { + VLOG(2) << "--------------------------------\n"; + VLOG(2) << "checking for benchmarks matching " << p.regex_str << "...\n"; + for(const auto& r : results) { + if(!p.regex->Match(r.name)) { + VLOG(2) << p.regex_str << " is not matched by " << r.name << "\n"; + continue; + } + else { + VLOG(2) << p.regex_str << " is matched by " << r.name << "\n"; + } + VLOG(1) << "Checking results of " << r.name << ": ... \n"; + p.fn(r); + VLOG(1) << "Checking results of " << r.name << ": OK.\n"; + } } } // prepare for the names in this header void ResultsChecker::SetHeader_(const std::string& csv_header) { - result_names = SplitCsv_(csv_header); + field_names = SplitCsv_(csv_header); } // set the values for subscribed benchmarks, and silently ignore all others void ResultsChecker::SetValues_(const std::string& entry_csv_line) { - CHECK(!result_names.empty()); + CHECK(!field_names.empty()); auto vals = SplitCsv_(entry_csv_line); if(vals.empty()) return; - CHECK_EQ(vals.size(), result_names.size()); - ResultsCheckerEntry* entry = Find_(vals[0]); - if(!entry) return; + CHECK_EQ(vals.size(), field_names.size()); + results.emplace_back(vals[0]); // vals[0] is the benchmark name + auto &entry = results.back(); for (size_t i = 1, e = vals.size(); i < e; ++i) { - entry->values[result_names[i]] = vals[i]; + entry.values[field_names[i]] = vals[i]; } } -// find a subscribed benchmark, or return null -ResultsCheckerEntry* ResultsChecker::Find_(const std::string& entry_name) { - auto it = results.find(entry_name); - if(it == results.end()) return nullptr; - return &it->second; -} - // a quick'n'dirty csv splitter (eliminating quotes) std::vector< std::string > ResultsChecker::SplitCsv_(std::string const& line) { std::vector< std::string > out; if(line.empty()) return out; - if(!result_names.empty()) out.reserve(result_names.size()); + if(!field_names.empty()) out.reserve(field_names.size()); size_t prev = 0, pos = line.find_first_of(','), curr = pos; while(pos != line.npos) { CHECK(curr > 0); diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index 159e05fe..071e4bfd 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -132,6 +132,10 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Threads/threads:%int\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Threads/threads:%int\",%csv_report,%float,%float$"}}); +CHECK_BENCHMARK_RESULTS("BM_Counters_Threads/threads:%int", + [](ResultsCheckerEntry const& e) { + std::cout << "BOO: " << e.name << "\n"; +}); // ========================================================================= // // ---------------------- ThreadAvg Counters Output ------------------------ // From 2814e9d8dc5a4bb59397d8f2c58ddeb54a4b109f Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 19:25:51 +0100 Subject: [PATCH 37/60] Fix ResultsCheckerEntry::NumThreads() --- test/output_test.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 7872326e..f9c6e63b 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -75,9 +75,9 @@ struct ResultsCheckerEntry { ResultsCheckerEntry(std::string const& n) : name(n) {} int NumThreads() const { - auto pos = name.find_last_of("/threads:"); + auto pos = name.find("/threads:"); if(pos == name.npos) return 1; - auto end = name.find_last_of('/', pos + 9); + auto end = name.find('/', pos + 9); std::stringstream ss; ss << name.substr(pos + 9, end); int num = 1; From 78548f8c6edf1c2a537c6482badfda1505eda5e3 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 19:26:34 +0100 Subject: [PATCH 38/60] Add (currently failing) tests for user counters with threads. --- test/user_counters_test.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index 071e4bfd..7c07d9b2 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -134,7 +134,8 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Threads/threads:%int\",$"}, ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Threads/threads:%int\",%csv_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_Threads/threads:%int", [](ResultsCheckerEntry const& e) { - std::cout << "BOO: " << e.name << "\n"; + CHECK_COUNTER_VALUE(e, int, "foo", EQ, e.NumThreads()); + CHECK_COUNTER_VALUE(e, int, "bar", EQ, 2 * e.NumThreads()); }); // ========================================================================= // @@ -159,6 +160,11 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreads/threads:%int\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreads/threads:%int\",%csv_report,%float,%float$"}}); +CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreads/threads:%int", + [](ResultsCheckerEntry const& e) { + CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); + CHECK_COUNTER_VALUE(e, int, "bar", EQ, 2); +}); // ========================================================================= // // ---------------------- ThreadAvg Counters Output ------------------------ // @@ -182,6 +188,11 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreadsRate/threads:%int\",$ {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreadsRate/threads:%int\",%csv_report,%float,%float$"}}); +CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreadsRate/threads:%int", + [](ResultsCheckerEntry const& e) { + CHECK_COUNTER_VALUE_EPS(e, "foo", EQ_EPS, 1./e.DurationCPUTime(), 0.001); + CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2./e.DurationCPUTime(), 0.001); +}); // ========================================================================= // // --------------------------- TEST CASES END ------------------------------ // From 921a51abcfd22b4e554e25bb69387b4e3213c6ab Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 19:34:37 +0100 Subject: [PATCH 39/60] Console reporting of user counters: print rates like non-rates. --- src/console_reporter.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/console_reporter.cc b/src/console_reporter.cc index d4dbb8ed..bc60c65f 100644 --- a/src/console_reporter.cc +++ b/src/console_reporter.cc @@ -134,8 +134,8 @@ void ConsoleReporter::PrintRunData(const Run& result) { for (auto& c : result.counters) { auto const& s = HumanReadableNumber(c.second.value); if (c.second.flags & Counter::Flags::kIsRate) { - std::string counter_rate = StrCat(" ", HumanReadableNumber(c.second.value), " ", c.first.c_str(), "/s"); - printer(Out, COLOR_DEFAULT, " %*s", 13, counter_rate.c_str()); + std::string counter_rate = StrCat(HumanReadableNumber(c.second.value), "/s"); + printer(Out, COLOR_DEFAULT, " %s=%s", c.first.c_str(), counter_rate.c_str()); } else { printer(Out, COLOR_DEFAULT, " %s=%s", c.first.c_str(), s.c_str()); From 03b0655d12c6c0102778ed1cdfe3789c5f4c58fe Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 19:35:43 +0100 Subject: [PATCH 40/60] Fix expected values of user counters as rates in unit tests. --- test/user_counters_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index 7c07d9b2..6e7ac565 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -93,7 +93,7 @@ void BM_Counters_Rate(benchmark::State& state) { state.counters["bar"] = bm::Counter{2, bm::Counter::kIsRate}; } BENCHMARK(BM_Counters_Rate); -ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Rate %console_report bar=%hrfloat foo=%hrfloat$"}}); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Rate %console_report bar=%hrfloat/s foo=%hrfloat/s$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Rate\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, @@ -178,7 +178,7 @@ void BM_Counters_AvgThreadsRate(benchmark::State& state) { state.counters["bar"] = bm::Counter{2, bm::Counter::kAvgThreadsRate}; } BENCHMARK(BM_Counters_AvgThreadsRate)->ThreadRange(1, 8); -ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_AvgThreadsRate/threads:%int %console_report bar=%hrfloat foo=%hrfloat$"}}); +ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_AvgThreadsRate/threads:%int %console_report bar=%hrfloat/s foo=%hrfloat/s$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreadsRate/threads:%int\",$"}, {"\"iterations\": %int,$", MR_Next}, {"\"real_time\": %int,$", MR_Next}, From 86249c57a574462cb0b30aefda0f91ae01e2f077 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 19:40:39 +0100 Subject: [PATCH 41/60] Result checking: move some function definitions to source file. --- test/output_test.h | 40 +++++++------------------------------- test/output_test_helper.cc | 30 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index f9c6e63b..c5c3d715 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -65,6 +65,11 @@ void RunOutputTests(int argc, char* argv[]); struct ResultsCheckerEntry; typedef std::function< void(ResultsCheckerEntry const&) > ResultsCheckFn; +// Add a function to check the (CSV) results of a benchmark. These +// functions will be called only after the output was successfully +// checked. +size_t AddChecker(const char* bm_name, ResultsCheckFn fn); + // Class to test the results of a benchmark. // It inspects the results by looking at the CSV output of a subscribed // benchmark. @@ -74,17 +79,7 @@ struct ResultsCheckerEntry { ResultsCheckerEntry(std::string const& n) : name(n) {} - int NumThreads() const { - auto pos = name.find("/threads:"); - if(pos == name.npos) return 1; - auto end = name.find('/', pos + 9); - std::stringstream ss; - ss << name.substr(pos + 9, end); - int num = 1; - ss >> num; - CHECK(!ss.fail()); - return num; - } + int NumThreads() const; // get the real_time duration of the benchmark in seconds double DurationRealTime() const { @@ -125,30 +120,9 @@ struct ResultsCheckerEntry { } // get cpu_time or real_time in seconds - double GetTime(const char* which) const { - double val = GetAs< double >(which); - auto unit = Get("time_unit"); - CHECK(unit); - if(*unit == "ns") { - return val * 1.e-9; - } else if(*unit == "us") { - return val * 1.e-6; - } else if(*unit == "ms") { - return val * 1.e-3; - } else if(*unit == "s") { - return val; - } else { - CHECK(1 == 0) << "unknown time unit: " << *unit; - return 0; - } - } + double GetTime(const char* which) const; }; -// Add a function to check the (CSV) results of a benchmark. These -// functions will be called only after the output was successfully -// checked. -size_t AddChecker(const char* bm_name, ResultsCheckFn fn); - //---------------------------------- // Macros to help in result checking. Do not use them with arguments causing // side-effects. diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 2b681fa0..41a05d99 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -293,6 +293,36 @@ size_t AddChecker(const char* bm_name, ResultsCheckFn fn) return rc.results.size(); } +int ResultsCheckerEntry::NumThreads() const { + auto pos = name.find("/threads:"); + if(pos == name.npos) return 1; + auto end = name.find('/', pos + 9); + std::stringstream ss; + ss << name.substr(pos + 9, end); + int num = 1; + ss >> num; + CHECK(!ss.fail()); + return num; +} + +double ResultsCheckerEntry::GetTime(const char* which) const { + double val = GetAs< double >(which); + auto unit = Get("time_unit"); + CHECK(unit); + if(*unit == "ns") { + return val * 1.e-9; + } else if(*unit == "us") { + return val * 1.e-6; + } else if(*unit == "ms") { + return val * 1.e-3; + } else if(*unit == "s") { + return val; + } else { + CHECK(1 == 0) << "unknown time unit: " << *unit; + return 0; + } +} + // ========================================================================= // // -------------------------- Public API Definitions------------------------ // // ========================================================================= // From f3b82a8edad1e2bb01125804bc80701e8c92b3c6 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 19:44:13 +0100 Subject: [PATCH 42/60] Adopt standard style. --- test/output_test.h | 2 +- test/output_test_helper.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index c5c3d715..be65924a 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -77,7 +77,7 @@ struct ResultsCheckerEntry { std::string name; std::map< std::string, std::string > values; - ResultsCheckerEntry(std::string const& n) : name(n) {} + ResultsCheckerEntry(const std::string& n) : name(n) {} int NumThreads() const; diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 41a05d99..9db09014 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -183,7 +183,7 @@ class ResultsChecker { void SetHeader_(const std::string& csv_header); void SetValues_(const std::string& entry_csv_line); - std::vector< std::string > SplitCsv_(std::string const& line); + std::vector< std::string > SplitCsv_(const std::string& line); }; @@ -263,7 +263,7 @@ void ResultsChecker::SetValues_(const std::string& entry_csv_line) { } // a quick'n'dirty csv splitter (eliminating quotes) -std::vector< std::string > ResultsChecker::SplitCsv_(std::string const& line) { +std::vector< std::string > ResultsChecker::SplitCsv_(const std::string& line) { std::vector< std::string > out; if(line.empty()) return out; if(!field_names.empty()) out.reserve(field_names.size()); From 180719d0d6dedb7a9f892f4f2dab13d95ca5ed4d Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 20:01:18 +0100 Subject: [PATCH 43/60] Rename ResultsCheckerEntry to Results. --- test/output_test.h | 49 ++++++++++++++++++++++---------------- test/output_test_helper.cc | 8 +++---- test/user_counters_test.cc | 12 +++++----- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index be65924a..760a0d00 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -62,22 +62,24 @@ void RunOutputTests(int argc, char* argv[]); // ------------------------- Results checking ------------------------------ // // ========================================================================= // -struct ResultsCheckerEntry; -typedef std::function< void(ResultsCheckerEntry const&) > ResultsCheckFn; +struct Results; +typedef std::function< void(Results const&) > ResultsCheckFn; // Add a function to check the (CSV) results of a benchmark. These // functions will be called only after the output was successfully // checked. -size_t AddChecker(const char* bm_name, ResultsCheckFn fn); +// bm_name_pattern: a name or a regex which will be matched agains +// all the benchmark names. Matching benchmarks +// will be the subject of a call to fn +size_t AddChecker(const char* bm_name_pattern, ResultsCheckFn fn); -// Class to test the results of a benchmark. -// It inspects the results by looking at the CSV output of a subscribed -// benchmark. -struct ResultsCheckerEntry { - std::string name; +// Class to hold the (CSV!) results of a benchmark. +// It is passed in calls to checker functions. +struct Results { + std::string name; // the benchmark name std::map< std::string, std::string > values; - ResultsCheckerEntry(const std::string& n) : name(n) {} + Results(const std::string& n) : name(n) {} int NumThreads() const; @@ -99,21 +101,14 @@ struct ResultsCheckerEntry { } // get a result by name, parsed as a specific type. - // For counters, use GetCounterAs instead. - template< class T > T GetAs(const char* entry_name) const { - auto *sv = Get(entry_name); - CHECK(sv != nullptr && !sv->empty()); - std::stringstream ss; - ss << *sv; - T out; - ss >> out; - CHECK(!ss.fail()); - return out; - } + // NOTE: for counters, use GetCounterAs instead. + template + T GetAs(const char* entry_name) const; // counters are written as doubles, so they have to be read first // as a double, and only then converted to the asked type. - template< class T > T GetCounterAs(const char* entry_name) const { + template + T GetCounterAs(const char* entry_name) const { double dval = GetAs< double >(entry_name); T tval = static_cast< T >(dval); return tval; @@ -123,6 +118,18 @@ struct ResultsCheckerEntry { double GetTime(const char* which) const; }; +template +T Results::GetAs(const char* entry_name) const { + auto *sv = Get(entry_name); + CHECK(sv != nullptr && !sv->empty()); + std::stringstream ss; + ss << *sv; + T out; + ss >> out; + CHECK(!ss.fail()); + return out; +} + //---------------------------------- // Macros to help in result checking. Do not use them with arguments causing // side-effects. diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 9db09014..ef2466c5 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -169,7 +169,7 @@ class ResultsChecker { }; std::vector< PatternAndFn > check_patterns; - std::vector< ResultsCheckerEntry > results; + std::vector< Results > results; std::vector< std::string > field_names; void Add(const std::string& entry_pattern, ResultsCheckFn fn); @@ -178,7 +178,7 @@ class ResultsChecker { private: - ResultsCheckerEntry* Find_(const std::string& entry_name); + Results* Find_(const std::string& entry_name); void SetHeader_(const std::string& csv_header); void SetValues_(const std::string& entry_csv_line); @@ -293,7 +293,7 @@ size_t AddChecker(const char* bm_name, ResultsCheckFn fn) return rc.results.size(); } -int ResultsCheckerEntry::NumThreads() const { +int Results::NumThreads() const { auto pos = name.find("/threads:"); if(pos == name.npos) return 1; auto end = name.find('/', pos + 9); @@ -305,7 +305,7 @@ int ResultsCheckerEntry::NumThreads() const { return num; } -double ResultsCheckerEntry::GetTime(const char* which) const { +double Results::GetTime(const char* which) const { double val = GetAs< double >(which); auto unit = Get("time_unit"); CHECK(unit); diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index 6e7ac565..0f43b1b5 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -35,7 +35,7 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Simple\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Simple\",%csv_report,%float,%float$"}}); -CHECK_BENCHMARK_RESULTS("BM_Counters_Simple", [](ResultsCheckerEntry const& e) { +CHECK_BENCHMARK_RESULTS("BM_Counters_Simple", [](Results const& e) { double its = e.GetAs< double >("iterations"); CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); // check that the value of bar is within 0.1% of the expected value @@ -72,7 +72,7 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_WithBytesAndItemsPSec\",$"}, ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_WithBytesAndItemsPSec\"," "%csv_bytes_items_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_WithBytesAndItemsPSec", - [](ResultsCheckerEntry const& e) { + [](Results const& e) { double t = e.DurationCPUTime(); // this (and not real time) is the time used CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); CHECK_COUNTER_VALUE(e, int, "bar", EQ, num_calls1); @@ -104,7 +104,7 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Rate\",$"}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Rate\",%csv_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_Rate", - [](ResultsCheckerEntry const& e) { + [](Results const& e) { double t = e.DurationCPUTime(); // this (and not real time) is the time used // check that the values are within 0.1% of the expected values CHECK_COUNTER_VALUE_EPS(e, "foo", EQ_EPS, 1./t, 0.001); @@ -133,7 +133,7 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Threads/threads:%int\",$"}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Threads/threads:%int\",%csv_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_Threads/threads:%int", - [](ResultsCheckerEntry const& e) { + [](Results const& e) { CHECK_COUNTER_VALUE(e, int, "foo", EQ, e.NumThreads()); CHECK_COUNTER_VALUE(e, int, "bar", EQ, 2 * e.NumThreads()); }); @@ -161,7 +161,7 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreads/threads:%int\",$"}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreads/threads:%int\",%csv_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreads/threads:%int", - [](ResultsCheckerEntry const& e) { + [](Results const& e) { CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); CHECK_COUNTER_VALUE(e, int, "bar", EQ, 2); }); @@ -189,7 +189,7 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreadsRate/threads:%int\",$ {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreadsRate/threads:%int\",%csv_report,%float,%float$"}}); CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreadsRate/threads:%int", - [](ResultsCheckerEntry const& e) { + [](Results const& e) { CHECK_COUNTER_VALUE_EPS(e, "foo", EQ_EPS, 1./e.DurationCPUTime(), 0.001); CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2./e.DurationCPUTime(), 0.001); }); From 55876610f169b3cf5fdfde98d49d9e9521cccbe8 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 20:02:19 +0100 Subject: [PATCH 44/60] Remove unused prototype. --- test/output_test_helper.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index ef2466c5..63cedb6e 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -178,8 +178,6 @@ class ResultsChecker { private: - Results* Find_(const std::string& entry_name); - void SetHeader_(const std::string& csv_header); void SetValues_(const std::string& entry_csv_line); From b5effb30f9f01ff94d412259eea623a3c511e5e0 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 20:24:26 +0100 Subject: [PATCH 45/60] Add missing include for providing std::function. --- test/output_test.h | 1 + 1 file changed, 1 insertion(+) diff --git a/test/output_test.h b/test/output_test.h index 760a0d00..6b6b70a1 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -7,6 +7,7 @@ #include #include #include +#include #include "../src/re.h" #include "benchmark/benchmark.h" From c81960a899135feff8adbd0a51390ce4d5f211f2 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 20:31:44 +0100 Subject: [PATCH 46/60] Add missing include of stringstream. --- test/output_test.h | 1 + 1 file changed, 1 insertion(+) diff --git a/test/output_test.h b/test/output_test.h index 6b6b70a1..661f9aec 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -8,6 +8,7 @@ #include #include #include +#include #include "../src/re.h" #include "benchmark/benchmark.h" From c16c8fffacb5ee51fe364605db9673b7a0b41b48 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 20:35:02 +0100 Subject: [PATCH 47/60] CHECK_EPS: Use parentheses to wrap macro arguments. --- src/check.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/check.h b/src/check.h index 86383596..fb992560 100644 --- a/src/check.h +++ b/src/check.h @@ -69,11 +69,11 @@ class CheckHandler { #define CHECK_GT(a, b) CHECK((a) > (b)) #define CHECK_LT(a, b) CHECK((a) < (b)) -#define CHECK_EQ_EPS(a, b, eps) CHECK(std::fabs((a) - (b)) < eps) -#define CHECK_NE_EPS(a, b, eps) CHECK(std::fabs((a) - (b)) >= eps) -#define CHECK_GE_EPS(a, b, eps) CHECK((a) - (b) > -eps) -#define CHECK_LE_EPS(a, b, eps) CHECK((b) - (a) > -eps) -#define CHECK_GT_EPS(a, b, eps) CHECK((a) - (b) > eps) -#define CHECK_LT_EPS(a, b, eps) CHECK((b) - (a) > eps) +#define CHECK_EQ_EPS(a, b, eps) CHECK(std::fabs((a) - (b)) < (eps)) +#define CHECK_NE_EPS(a, b, eps) CHECK(std::fabs((a) - (b)) >= (eps)) +#define CHECK_GE_EPS(a, b, eps) CHECK((a) - (b) > -(eps)) +#define CHECK_LE_EPS(a, b, eps) CHECK((b) - (a) > -(eps)) +#define CHECK_GT_EPS(a, b, eps) CHECK((a) - (b) > (eps)) +#define CHECK_LT_EPS(a, b, eps) CHECK((b) - (a) > (eps)) #endif // CHECK_H_ From 8f69e4f6ce05fb53084030017480133446cd9137 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 20:35:25 +0100 Subject: [PATCH 48/60] Remove whitespace. --- test/output_test.h | 1 - 1 file changed, 1 deletion(-) diff --git a/test/output_test.h b/test/output_test.h index 661f9aec..3af371e5 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -176,7 +176,6 @@ T Results::GetAs(const char* entry_name) const { #define CHECK_COUNTER_VALUE_EPS(entry, var_name, relationship, value, eps_factor) \ _CHECK_RESULT_VALUE_EPS(entry, GetCounterAs, double, var_name, relationship, value, eps_factor) - #define CHECK_BENCHMARK_RESULTS(bm_name, checker_function) \ size_t CONCAT(dummy, __LINE__) = AddChecker(bm_name, checker_function) From ef6b4fb8577a7107622026d4e6028db62d21412a Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 20:41:33 +0100 Subject: [PATCH 49/60] Simplify printing of counters in console reporter. --- src/console_reporter.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/console_reporter.cc b/src/console_reporter.cc index bc60c65f..7513acf5 100644 --- a/src/console_reporter.cc +++ b/src/console_reporter.cc @@ -132,14 +132,10 @@ void ConsoleReporter::PrintRunData(const Run& result) { } for (auto& c : result.counters) { - auto const& s = HumanReadableNumber(c.second.value); - if (c.second.flags & Counter::Flags::kIsRate) { - std::string counter_rate = StrCat(HumanReadableNumber(c.second.value), "/s"); - printer(Out, COLOR_DEFAULT, " %s=%s", c.first.c_str(), counter_rate.c_str()); - } - else { - printer(Out, COLOR_DEFAULT, " %s=%s", c.first.c_str(), s.c_str()); - } + std::string s = HumanReadableNumber(c.second.value); + const char* unit = ((c.second.flags & Counter::Flags::kIsRate) ? + "/s" : ""); + printer(Out, COLOR_DEFAULT, " %s=%s%s", c.first.c_str(), s.c_str(), unit); } if (!rate.empty()) { From cdbcaaf2b64fbe113c9fe3b3526f22d2be282c15 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 20:47:32 +0100 Subject: [PATCH 50/60] Fix g++-4.8 compile errors. --- test/output_test_helper.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 63cedb6e..5e515dce 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include "../src/check.h" // NOTE: check.h is for internal use only! #include "../src/re.h" // NOTE: re.h is for internal use only @@ -163,8 +164,8 @@ class ResultsChecker { public: struct PatternAndFn : public TestCase { // reusing TestCase for its regexes - PatternAndFn(const std::string& regex, ResultsCheckFn fn_) - : TestCase(regex), fn(fn_) {} + PatternAndFn(const std::string& rx, ResultsCheckFn fn_) + : TestCase(rx), fn(fn_) {} ResultsCheckFn fn; }; @@ -414,6 +415,7 @@ void RunOutputTests(int argc, char* argv[]) { // now that we know the output is as expected, we can dispatch // the checks to subscribees. auto &csv = TestCases[2]; - CHECK(strcmp(csv.name, "CSVReporter") == 0); // would use == but gcc spits a warning + // would use == but gcc spits a warning + CHECK(std::strcmp(csv.name, "CSVReporter") == 0); internal::GetResultsChecker().CheckResults(csv.out_stream); } From 2a2eb44b3004ec67f694c26dacf488e57c63e568 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 29 Apr 2017 22:27:55 +0100 Subject: [PATCH 51/60] Fix VS2013 quirk. --- test/user_counters_test.cc | 50 +++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index 0f43b1b5..f24f3ade 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -35,12 +35,15 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Simple\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Simple\",%csv_report,%float,%float$"}}); -CHECK_BENCHMARK_RESULTS("BM_Counters_Simple", [](Results const& e) { +// VS2013 does not allow this function to be passed as a lambda argument +// to CHECK_BENCHMARK_RESULTS() +void CheckSimple(Results const& e) { double its = e.GetAs< double >("iterations"); CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); // check that the value of bar is within 0.1% of the expected value CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2.*its, 0.001); -}); +} +CHECK_BENCHMARK_RESULTS("BM_Counters_Simple", &CheckSimple); // ========================================================================= // // --------------------- Counters+Items+Bytes/s Output --------------------- // @@ -71,15 +74,18 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_WithBytesAndItemsPSec\",$"}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_WithBytesAndItemsPSec\"," "%csv_bytes_items_report,%float,%float$"}}); -CHECK_BENCHMARK_RESULTS("BM_Counters_WithBytesAndItemsPSec", - [](Results const& e) { +// VS2013 does not allow this function to be passed as a lambda argument +// to CHECK_BENCHMARK_RESULTS() +void CheckBytesAndItemsPSec(Results const& e) { double t = e.DurationCPUTime(); // this (and not real time) is the time used CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); CHECK_COUNTER_VALUE(e, int, "bar", EQ, num_calls1); // check that the values are within 0.1% of the expected values CHECK_RESULT_VALUE_EPS(e, "bytes_per_second", EQ_EPS, 364./t, 0.001); CHECK_RESULT_VALUE_EPS(e, "items_per_second", EQ_EPS, 150./t, 0.001); -}); +} +CHECK_BENCHMARK_RESULTS("BM_Counters_WithBytesAndItemsPSec", + &CheckBytesAndItemsPSec); // ========================================================================= // // ------------------------- Rate Counters Output -------------------------- // @@ -103,13 +109,15 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Rate\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Rate\",%csv_report,%float,%float$"}}); -CHECK_BENCHMARK_RESULTS("BM_Counters_Rate", - [](Results const& e) { +// VS2013 does not allow this function to be passed as a lambda argument +// to CHECK_BENCHMARK_RESULTS() +void CheckRate(Results const& e) { double t = e.DurationCPUTime(); // this (and not real time) is the time used // check that the values are within 0.1% of the expected values CHECK_COUNTER_VALUE_EPS(e, "foo", EQ_EPS, 1./t, 0.001); CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2./t, 0.001); -}); +} +CHECK_BENCHMARK_RESULTS("BM_Counters_Rate", &CheckRate); // ========================================================================= // // ------------------------- Thread Counters Output ------------------------ // @@ -132,11 +140,13 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_Threads/threads:%int\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Threads/threads:%int\",%csv_report,%float,%float$"}}); -CHECK_BENCHMARK_RESULTS("BM_Counters_Threads/threads:%int", - [](Results const& e) { +// VS2013 does not allow this function to be passed as a lambda argument +// to CHECK_BENCHMARK_RESULTS() +void CheckThreads(Results const& e) { CHECK_COUNTER_VALUE(e, int, "foo", EQ, e.NumThreads()); CHECK_COUNTER_VALUE(e, int, "bar", EQ, 2 * e.NumThreads()); -}); +} +CHECK_BENCHMARK_RESULTS("BM_Counters_Threads/threads:%int", &CheckThreads); // ========================================================================= // // ---------------------- ThreadAvg Counters Output ------------------------ // @@ -160,11 +170,14 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreads/threads:%int\",$"}, {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreads/threads:%int\",%csv_report,%float,%float$"}}); -CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreads/threads:%int", - [](Results const& e) { +// VS2013 does not allow this function to be passed as a lambda argument +// to CHECK_BENCHMARK_RESULTS() +void CheckAvgThreads(Results const& e) { CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); CHECK_COUNTER_VALUE(e, int, "bar", EQ, 2); -}); +} +CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreads/threads:%int", + &CheckAvgThreads); // ========================================================================= // // ---------------------- ThreadAvg Counters Output ------------------------ // @@ -188,11 +201,14 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_Counters_AvgThreadsRate/threads:%int\",$ {"\"foo\": %float$", MR_Next}, {"}", MR_Next}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreadsRate/threads:%int\",%csv_report,%float,%float$"}}); -CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreadsRate/threads:%int", - [](Results const& e) { +// VS2013 does not allow this function to be passed as a lambda argument +// to CHECK_BENCHMARK_RESULTS() +void CheckAvgThreadsRate(Results const& e) { CHECK_COUNTER_VALUE_EPS(e, "foo", EQ_EPS, 1./e.DurationCPUTime(), 0.001); CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2./e.DurationCPUTime(), 0.001); -}); +} +CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreadsRate/threads:%int", + &CheckAvgThreadsRate); // ========================================================================= // // --------------------------- TEST CASES END ------------------------------ // From 47226ccd565fa04a6064fd55cea7164ff8b78c64 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 1 May 2017 21:48:13 +0100 Subject: [PATCH 52/60] CHECK(): rename EPS to FLOAT for consistency with googletest style. --- src/check.h | 12 ++++++------ test/output_test.h | 16 ++++++++-------- test/user_counters_test.cc | 14 +++++++------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/check.h b/src/check.h index fb992560..73bead2f 100644 --- a/src/check.h +++ b/src/check.h @@ -69,11 +69,11 @@ class CheckHandler { #define CHECK_GT(a, b) CHECK((a) > (b)) #define CHECK_LT(a, b) CHECK((a) < (b)) -#define CHECK_EQ_EPS(a, b, eps) CHECK(std::fabs((a) - (b)) < (eps)) -#define CHECK_NE_EPS(a, b, eps) CHECK(std::fabs((a) - (b)) >= (eps)) -#define CHECK_GE_EPS(a, b, eps) CHECK((a) - (b) > -(eps)) -#define CHECK_LE_EPS(a, b, eps) CHECK((b) - (a) > -(eps)) -#define CHECK_GT_EPS(a, b, eps) CHECK((a) - (b) > (eps)) -#define CHECK_LT_EPS(a, b, eps) CHECK((b) - (a) > (eps)) +#define CHECK_FLOAT_EQ(a, b, eps) CHECK(std::fabs((a) - (b)) < (eps)) +#define CHECK_FLOAT_NE(a, b, eps) CHECK(std::fabs((a) - (b)) >= (eps)) +#define CHECK_FLOAT_GE(a, b, eps) CHECK((a) - (b) > -(eps)) +#define CHECK_FLOAT_LE(a, b, eps) CHECK((b) - (a) > -(eps)) +#define CHECK_FLOAT_GT(a, b, eps) CHECK((a) - (b) > (eps)) +#define CHECK_FLOAT_LT(a, b, eps) CHECK((b) - (a) > (eps)) #endif // CHECK_H_ diff --git a/test/output_test.h b/test/output_test.h index 3af371e5..7eb67eda 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -145,10 +145,10 @@ T Results::GetAs(const char* entry_name) const { << "=" << (entry).getfn< var_type >(var_name) \ << " to be " #relationship " to " << (value) << "\n" -// check with tolerance. eps_factor is the tolerance window, which will be -// interpreted relative to value. -#define _CHECK_RESULT_VALUE_EPS(entry, getfn, var_type, var_name, relationship, value, eps_factor) \ - CONCAT(CHECK_, relationship) \ +// check with tolerance. eps_factor is the tolerance window, which is +// interpreted relative to value (eg, 0.1 means 10% of value). +#define _CHECK_FLOAT_RESULT_VALUE(entry, getfn, var_type, var_name, relationship, value, eps_factor) \ + CONCAT(CHECK_FLOAT_, relationship) \ (entry.getfn< var_type >(var_name), (value), (eps_factor) * (value)) << "\n" \ << __FILE__ << ":" << __LINE__ << ": " << (entry).name << ":\n" \ << __FILE__ << ":" << __LINE__ << ": " \ @@ -170,11 +170,11 @@ T Results::GetAs(const char* entry_name) const { #define CHECK_COUNTER_VALUE(entry, var_type, var_name, relationship, value) \ _CHECK_RESULT_VALUE(entry, GetCounterAs, var_type, var_name, relationship, value) -#define CHECK_RESULT_VALUE_EPS(entry, var_name, relationship, value, eps_factor) \ - _CHECK_RESULT_VALUE_EPS(entry, GetAs, double, var_name, relationship, value, eps_factor) +#define CHECK_FLOAT_RESULT_VALUE(entry, var_name, relationship, value, eps_factor) \ + _CHECK_FLOAT_RESULT_VALUE(entry, GetAs, double, var_name, relationship, value, eps_factor) -#define CHECK_COUNTER_VALUE_EPS(entry, var_name, relationship, value, eps_factor) \ - _CHECK_RESULT_VALUE_EPS(entry, GetCounterAs, double, var_name, relationship, value, eps_factor) +#define CHECK_FLOAT_COUNTER_VALUE(entry, var_name, relationship, value, eps_factor) \ + _CHECK_FLOAT_RESULT_VALUE(entry, GetCounterAs, double, var_name, relationship, value, eps_factor) #define CHECK_BENCHMARK_RESULTS(bm_name, checker_function) \ size_t CONCAT(dummy, __LINE__) = AddChecker(bm_name, checker_function) diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index f24f3ade..f337c137 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -41,7 +41,7 @@ void CheckSimple(Results const& e) { double its = e.GetAs< double >("iterations"); CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); // check that the value of bar is within 0.1% of the expected value - CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2.*its, 0.001); + CHECK_FLOAT_COUNTER_VALUE(e, "bar", EQ, 2.*its, 0.001); } CHECK_BENCHMARK_RESULTS("BM_Counters_Simple", &CheckSimple); @@ -81,8 +81,8 @@ void CheckBytesAndItemsPSec(Results const& e) { CHECK_COUNTER_VALUE(e, int, "foo", EQ, 1); CHECK_COUNTER_VALUE(e, int, "bar", EQ, num_calls1); // check that the values are within 0.1% of the expected values - CHECK_RESULT_VALUE_EPS(e, "bytes_per_second", EQ_EPS, 364./t, 0.001); - CHECK_RESULT_VALUE_EPS(e, "items_per_second", EQ_EPS, 150./t, 0.001); + CHECK_FLOAT_RESULT_VALUE(e, "bytes_per_second", EQ, 364./t, 0.001); + CHECK_FLOAT_RESULT_VALUE(e, "items_per_second", EQ, 150./t, 0.001); } CHECK_BENCHMARK_RESULTS("BM_Counters_WithBytesAndItemsPSec", &CheckBytesAndItemsPSec); @@ -114,8 +114,8 @@ ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Rate\",%csv_report,%float,%float$"}}); void CheckRate(Results const& e) { double t = e.DurationCPUTime(); // this (and not real time) is the time used // check that the values are within 0.1% of the expected values - CHECK_COUNTER_VALUE_EPS(e, "foo", EQ_EPS, 1./t, 0.001); - CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2./t, 0.001); + CHECK_FLOAT_COUNTER_VALUE(e, "foo", EQ, 1./t, 0.001); + CHECK_FLOAT_COUNTER_VALUE(e, "bar", EQ, 2./t, 0.001); } CHECK_BENCHMARK_RESULTS("BM_Counters_Rate", &CheckRate); @@ -204,8 +204,8 @@ ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_AvgThreadsRate/threads:%int\",%csv_report // VS2013 does not allow this function to be passed as a lambda argument // to CHECK_BENCHMARK_RESULTS() void CheckAvgThreadsRate(Results const& e) { - CHECK_COUNTER_VALUE_EPS(e, "foo", EQ_EPS, 1./e.DurationCPUTime(), 0.001); - CHECK_COUNTER_VALUE_EPS(e, "bar", EQ_EPS, 2./e.DurationCPUTime(), 0.001); + CHECK_FLOAT_COUNTER_VALUE(e, "foo", EQ, 1./e.DurationCPUTime(), 0.001); + CHECK_FLOAT_COUNTER_VALUE(e, "bar", EQ, 2./e.DurationCPUTime(), 0.001); } CHECK_BENCHMARK_RESULTS("BM_Counters_AvgThreadsRate/threads:%int", &CheckAvgThreadsRate); From 21600b966f2c7736815fc95a27d9bb0ae03d04d1 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 1 May 2017 22:22:11 +0100 Subject: [PATCH 53/60] Fix VS warning. --- test/user_counters_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/user_counters_test.cc b/test/user_counters_test.cc index f337c137..66df48b3 100644 --- a/test/user_counters_test.cc +++ b/test/user_counters_test.cc @@ -22,7 +22,7 @@ void BM_Counters_Simple(benchmark::State& state) { while (state.KeepRunning()) { } state.counters["foo"] = 1; - state.counters["bar"] = 2 * state.iterations(); + state.counters["bar"] = 2 * (double)state.iterations(); } BENCHMARK(BM_Counters_Simple); ADD_CASES(TC_ConsoleOut, {{"^BM_Counters_Simple %console_report bar=%hrfloat foo=%hrfloat$"}}); From 3443ac2103c891c75d29563b3a2bd5c8b822a133 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 1 May 2017 22:27:56 +0100 Subject: [PATCH 54/60] Fix brace formatting (Habits die hard!). --- test/output_test_helper.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 5e515dce..aa513bef 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -199,8 +199,7 @@ void ResultsChecker::Add(const std::string& entry_pattern, ResultsCheckFn fn) { } // check the results of all subscribed benchmarks -void ResultsChecker::CheckResults(std::stringstream& output) -{ +void ResultsChecker::CheckResults(std::stringstream& output) { // first reset the stream to the start { auto start = std::ios::streampos(0); From 62b1dd9c4ae8664b1e0ddc05ccc774f2462da93a Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 1 May 2017 22:29:26 +0100 Subject: [PATCH 55/60] CHECK_BENCHMARK_RESULTS() was too inconspicuous. --- test/output_test.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 7eb67eda..66e21444 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -64,6 +64,17 @@ void RunOutputTests(int argc, char* argv[]); // ------------------------- Results checking ------------------------------ // // ========================================================================= // +// Call this macro to register a benchmark for checking its results. This +// should be all that's needed. It subscribes a function to check the (CSV) +// results of a benchmark. This is done only after verifying that the output +// strings are really as expected. +// bm_name_pattern: a name or a regex pattern which will be matched against +// all the benchmark names. Matching benchmarks +// will be the subject of a call to checker_function +// checker_function: should be of type ResultsCheckFn (see below) +#define CHECK_BENCHMARK_RESULTS(bm_name_pattern, checker_function) \ + size_t CONCAT(dummy, __LINE__) = AddChecker(bm_name_pattern, checker_function) + struct Results; typedef std::function< void(Results const&) > ResultsCheckFn; @@ -176,9 +187,6 @@ T Results::GetAs(const char* entry_name) const { #define CHECK_FLOAT_COUNTER_VALUE(entry, var_name, relationship, value, eps_factor) \ _CHECK_FLOAT_RESULT_VALUE(entry, GetCounterAs, double, var_name, relationship, value, eps_factor) -#define CHECK_BENCHMARK_RESULTS(bm_name, checker_function) \ - size_t CONCAT(dummy, __LINE__) = AddChecker(bm_name, checker_function) - // ========================================================================= // // --------------------------- Misc Utilities ------------------------------ // // ========================================================================= // From 64b5f3ff2da8278c4c066bdd18a53b5057a24837 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 1 May 2017 22:32:40 +0100 Subject: [PATCH 56/60] Make Results::GetTime() receive an enum. --- test/output_test.h | 16 ++++++++++------ test/output_test_helper.cc | 6 ++++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 66e21444..7c95e482 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -96,13 +96,20 @@ struct Results { int NumThreads() const; - // get the real_time duration of the benchmark in seconds + typedef enum { kCpuTime, kRealTime } BenchmarkTime; + + // get cpu_time or real_time in seconds + double GetTime(BenchmarkTime which) const; + + // get the real_time duration of the benchmark in seconds. + // it is better to use fuzzy float checks for this, as the float + // ASCII formatting is lossy. double DurationRealTime() const { - return GetAs< double >("iterations") * GetTime("real_time"); + return GetAs< double >("iterations") * GetTime(kRealTime); } // get the cpu_time duration of the benchmark in seconds double DurationCPUTime() const { - return GetAs< double >("iterations") * GetTime("cpu_time"); + return GetAs< double >("iterations") * GetTime(kCpuTime); } // get the string for a result by name, or nullptr if the name @@ -126,9 +133,6 @@ struct Results { T tval = static_cast< T >(dval); return tval; } - - // get cpu_time or real_time in seconds - double GetTime(const char* which) const; }; template diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index aa513bef..d0c9f7db 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -303,8 +303,10 @@ int Results::NumThreads() const { return num; } -double Results::GetTime(const char* which) const { - double val = GetAs< double >(which); +double Results::GetTime(BenchmarkTime which) const { + CHECK(which == kCpuTime || which == kRealTime); + const char *which_str = which == kCpuTime ? "cpu_time" : "real_time"; + double val = GetAs< double >(which_str); auto unit = Get("time_unit"); CHECK(unit); if(*unit == "ns") { From b57b2cfd771216af2029aa2c8b6dfec5e62d70af Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 1 May 2017 22:33:19 +0100 Subject: [PATCH 57/60] Improve some comments. --- test/output_test.h | 13 +++++-------- test/output_test_helper.cc | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index 7c95e482..897a1386 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -78,18 +78,15 @@ void RunOutputTests(int argc, char* argv[]); struct Results; typedef std::function< void(Results const&) > ResultsCheckFn; -// Add a function to check the (CSV) results of a benchmark. These -// functions will be called only after the output was successfully -// checked. -// bm_name_pattern: a name or a regex which will be matched agains -// all the benchmark names. Matching benchmarks -// will be the subject of a call to fn size_t AddChecker(const char* bm_name_pattern, ResultsCheckFn fn); -// Class to hold the (CSV!) results of a benchmark. +// Class holding the results of a benchmark. // It is passed in calls to checker functions. struct Results { - std::string name; // the benchmark name + + // the benchmark name + std::string name; + // the benchmark fields std::map< std::string, std::string > values; Results(const std::string& n) : name(n) {} diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index d0c9f7db..fe750c7e 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -247,11 +247,11 @@ void ResultsChecker::SetHeader_(const std::string& csv_header) { field_names = SplitCsv_(csv_header); } -// set the values for subscribed benchmarks, and silently ignore all others +// set the values for a benchmark void ResultsChecker::SetValues_(const std::string& entry_csv_line) { + if(entry_csv_line.empty()) return; // some lines are empty CHECK(!field_names.empty()); auto vals = SplitCsv_(entry_csv_line); - if(vals.empty()) return; CHECK_EQ(vals.size(), field_names.size()); results.emplace_back(vals[0]); // vals[0] is the benchmark name auto &entry = results.back(); From 77b9362b06d7fdef3937a28d56eb6ea24b3d9696 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 1 May 2017 22:33:44 +0100 Subject: [PATCH 58/60] Add output_test.h to output_test_helper for VisualStudio editing. --- test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8d644f90..40eb8808 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -27,7 +27,7 @@ if (DEFINED BENCHMARK_CXX_LINKER_FLAGS) list(APPEND CMAKE_EXE_LINKER_FLAGS ${BENCHMARK_CXX_LINKER_FLAGS}) endif() -add_library(output_test_helper STATIC output_test_helper.cc) +add_library(output_test_helper STATIC output_test_helper.cc output_test.h) macro(compile_benchmark_test name) add_executable(${name} "${name}.cc") From eb2bf345244eff2ceaa129e32ad112254e7603e2 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Tue, 2 May 2017 11:37:46 +0100 Subject: [PATCH 59/60] Fix indentation. [ci-skip] --- src/console_reporter.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/console_reporter.cc b/src/console_reporter.cc index 7513acf5..e8121800 100644 --- a/src/console_reporter.cc +++ b/src/console_reporter.cc @@ -132,10 +132,9 @@ void ConsoleReporter::PrintRunData(const Run& result) { } for (auto& c : result.counters) { - std::string s = HumanReadableNumber(c.second.value); - const char* unit = ((c.second.flags & Counter::Flags::kIsRate) ? - "/s" : ""); - printer(Out, COLOR_DEFAULT, " %s=%s%s", c.first.c_str(), s.c_str(), unit); + std::string s = HumanReadableNumber(c.second.value); + const char* unit = ((c.second.flags & Counter::kIsRate) ? "/s" : ""); + printer(Out, COLOR_DEFAULT, " %s=%s%s", c.first.c_str(), s.c_str(), unit); } if (!rate.empty()) { From feb69ae7102e6bdcb5736cd7c1669969217d45c0 Mon Sep 17 00:00:00 2001 From: Felix Duvallet Date: Tue, 2 May 2017 17:19:35 +0200 Subject: [PATCH 60/60] Ensure all the necessary keys are present before parsing JSON data (#380) This prevents errors when additional non-timing data are present in the JSON that is loaded, for example when complexity data has been computed (see #379). --- tools/gbench/report.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/gbench/report.py b/tools/gbench/report.py index 8f1b0fa8..015d33d9 100644 --- a/tools/gbench/report.py +++ b/tools/gbench/report.py @@ -80,7 +80,9 @@ def generate_difference_report(json1, json2, use_color=True): first_line = "{:<{}s} Time CPU Old New".format( 'Benchmark', first_col_width) output_strs = [first_line, '-' * len(first_line)] - for bn in json1['benchmarks']: + + gen = (bn for bn in json1['benchmarks'] if 'real_time' in bn and 'cpu_time' in bn) + for bn in gen: other_bench = find_test(bn['name']) if not other_bench: continue