From a6af6eeb6a53c599365bc405539c1ec044fefb32 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Wed, 6 Nov 2024 14:15:22 +0100 Subject: [PATCH 01/28] Add a Python matrix to ensure the bindings build on all supported versions (#1871) Also contains a run of `pre-commit autoupdate`, and a bump of cibuildwheel to its latest tag for CPython 3.13 support. But, since we build for 3.10+ with SABI from 3.12 onwards, we don't even need a dedicated Python 3.13 build job or toolchain - the wheels from 3.12 can be reused. Simplifies some version-dependent logic around assembling the bazel build command in setup.py, and fixes a possible unbound local error in the toolchain patch context manager. --- .github/workflows/test_bindings.yml | 12 ++++++------ .github/workflows/wheels.yml | 2 +- .pre-commit-config.yaml | 6 +++--- setup.py | 22 ++++++++++++++-------- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/.github/workflows/test_bindings.yml b/.github/workflows/test_bindings.yml index 436a8f90e5..b6ac9be8cb 100644 --- a/.github/workflows/test_bindings.yml +++ b/.github/workflows/test_bindings.yml @@ -8,23 +8,23 @@ on: jobs: python_bindings: - name: Test GBM Python bindings on ${{ matrix.os }} + name: Test GBM Python ${{ matrix.python-version }} bindings on ${{ matrix.os }} runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: os: [ ubuntu-latest, macos-latest, windows-latest ] + python-version: [ "3.10", "3.11", "3.12", "3.13" ] steps: - uses: actions/checkout@v4 with: fetch-depth: 0 - - name: Set up Python 3.11 + - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v5 with: - python-version: 3.11 + python-version: ${{ matrix.python-version }} - name: Install GBM Python bindings on ${{ matrix.os }} run: python -m pip install . - - name: Run bindings example on ${{ matrix.os }} - run: - python bindings/python/google_benchmark/example.py + - name: Run example on ${{ matrix.os }} under Python ${{ matrix.python-version }} + run: python bindings/python/google_benchmark/example.py diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index 7544b24758..b463ff83dd 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -53,7 +53,7 @@ jobs: platforms: all - name: Build wheels on ${{ matrix.os }} using cibuildwheel - uses: pypa/cibuildwheel@v2.20 + uses: pypa/cibuildwheel@v2.21.3 env: CIBW_BUILD: "cp310-* cp311-* cp312-*" CIBW_BUILD_FRONTEND: "build[uv]" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ef13c1dabd..2a51592edf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,17 +1,17 @@ repos: - repo: https://github.com/keith/pre-commit-buildifier - rev: 7.1.2 + rev: 7.3.1 hooks: - id: buildifier - id: buildifier-lint - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.11.1 + rev: v1.13.0 hooks: - id: mypy types_or: [ python, pyi ] args: [ "--ignore-missing-imports", "--scripts-are-modules" ] - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.6.1 + rev: v0.7.2 hooks: - id: ruff args: [ --fix, --exit-non-zero-on-fix ] diff --git a/setup.py b/setup.py index 1e4c0db761..238d9d8987 100644 --- a/setup.py +++ b/setup.py @@ -3,6 +3,7 @@ import platform import re import shutil +import sys from pathlib import Path from typing import Any, Generator @@ -15,8 +16,7 @@ # hardcoded SABI-related options. Requires that each Python interpreter # (hermetic or not) participating is of the same major-minor version. -version_tuple = tuple(int(i) for i in platform.python_version_tuple()) -py_limited_api = version_tuple >= (3, 12) +py_limited_api = sys.version_info >= (3, 12) options = {"bdist_wheel": {"py_limited_api": "cp312"}} if py_limited_api else {} @@ -43,10 +43,10 @@ def fmt_toolchain_args(matchobj): return "python.toolchain(" + callargs + ")" CIBW_LINUX = is_cibuildwheel() and IS_LINUX + module_bazel = Path("MODULE.bazel") + content: str = module_bazel.read_text() try: if CIBW_LINUX: - module_bazel = Path("MODULE.bazel") - content: str = module_bazel.read_text() module_bazel.write_text( re.sub( r"python.toolchain\(([\w\"\s,.=]*)\)", @@ -92,10 +92,16 @@ def copy_extensions_to_source(self): def bazel_build(self, ext: BazelExtension) -> None: """Runs the bazel build to create the package.""" temp_path = Path(self.build_temp) - # omit the patch version to avoid build errors if the toolchain is not - # yet registered in the current @rules_python version. - # patch version differences should be fine. - python_version = ".".join(platform.python_version_tuple()[:2]) + if py_limited_api: + # We only need to know the minimum ABI version, + # since it is stable across minor versions by definition. + # The value here is calculated as the minimum of a) the minimum + # Python version required, and b) the stable ABI version target. + # NB: This needs to be kept in sync with [project.requires-python] + # in pyproject.toml. + python_version = "3.12" + else: + python_version = "{0}.{1}".format(*sys.version_info[:2]) bazel_argv = [ "bazel", From 50ffd3e546c51686e468042721704ad59d4e0eac Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Thu, 7 Nov 2024 16:04:51 +0100 Subject: [PATCH 02/28] Declare a Python 3.13 toolchain, revert setup.py toolchain arget selection (#1876) The new solution was too smart (read: dense), because it did not account for the fact that we look for the Windows libs of the interpreter building the wheel, not the hermetic one supplying the header files. The fix is to just align the versions again, so that the libs and headers come from the same minor version. --- MODULE.bazel | 1 + setup.py | 14 ++++---------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 40e306fafa..092ee18068 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -27,6 +27,7 @@ python.toolchain( is_default = True, python_version = "3.12", ) +python.toolchain(python_version = "3.13") pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip", dev_dependency = True) pip.parse( diff --git a/setup.py b/setup.py index 238d9d8987..69cc49da7a 100644 --- a/setup.py +++ b/setup.py @@ -92,16 +92,10 @@ def copy_extensions_to_source(self): def bazel_build(self, ext: BazelExtension) -> None: """Runs the bazel build to create the package.""" temp_path = Path(self.build_temp) - if py_limited_api: - # We only need to know the minimum ABI version, - # since it is stable across minor versions by definition. - # The value here is calculated as the minimum of a) the minimum - # Python version required, and b) the stable ABI version target. - # NB: This needs to be kept in sync with [project.requires-python] - # in pyproject.toml. - python_version = "3.12" - else: - python_version = "{0}.{1}".format(*sys.version_info[:2]) + + # We round to the minor version, which makes rules_python + # look up the latest available patch version internally. + python_version = "{0}.{1}".format(*sys.version_info[:2]) bazel_argv = [ "bazel", From 62a321d6dc377e0ba9c712b6a8d64360616de564 Mon Sep 17 00:00:00 2001 From: dominic <510002+dmah42@users.noreply.github.com> Date: Wed, 13 Nov 2024 13:06:48 +0000 Subject: [PATCH 03/28] update standard to C++17 per C++ build support (#1875) * update standard to C++17 per C++ build support * disable deadcode checks from clang-tidy * fix redundant definition of constexpr --- .github/workflows/clang-tidy.yml | 2 +- BUILD.bazel | 2 +- CMakeLists.txt | 2 +- src/perf_counters.cc | 2 -- test/BUILD | 2 +- 5 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 558375e3ae..37a61cdb3a 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -35,4 +35,4 @@ jobs: - name: run shell: bash working-directory: ${{ runner.workspace }}/_build - run: run-clang-tidy + run: run-clang-tidy -checks=*,-clang-analyzer-deadcode* diff --git a/BUILD.bazel b/BUILD.bazel index 094ed62437..3451b4e758 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -3,7 +3,7 @@ licenses(["notice"]) COPTS = [ "-pedantic", "-pedantic-errors", - "-std=c++14", + "-std=c++17", "-Wall", "-Wconversion", "-Wextra", diff --git a/CMakeLists.txt b/CMakeLists.txt index 3aac35fe69..c90529d8b3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -138,7 +138,7 @@ if (BENCHMARK_BUILD_32_BITS) add_required_cxx_compiler_flag(-m32) endif() -set(BENCHMARK_CXX_STANDARD 14) +set(BENCHMARK_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD ${BENCHMARK_CXX_STANDARD}) set(CMAKE_CXX_STANDARD_REQUIRED YES) diff --git a/src/perf_counters.cc b/src/perf_counters.cc index fc9586b716..a2fa7fe35f 100644 --- a/src/perf_counters.cc +++ b/src/perf_counters.cc @@ -26,8 +26,6 @@ namespace benchmark { namespace internal { -constexpr size_t PerfCounterValues::kMaxCounters; - #if defined HAVE_LIBPFM size_t PerfCounterValues::Read(const std::vector& leaders) { diff --git a/test/BUILD b/test/BUILD index f2179f61c1..c1ca86b5b2 100644 --- a/test/BUILD +++ b/test/BUILD @@ -10,7 +10,7 @@ platform( TEST_COPTS = [ "-pedantic", "-pedantic-errors", - "-std=c++14", + "-std=c++17", "-Wall", "-Wconversion", "-Wextra", From d26047a0ac8485721f3bf1dfe21374cddb58ea3b Mon Sep 17 00:00:00 2001 From: Guo Ci Date: Wed, 27 Nov 2024 04:41:06 -0500 Subject: [PATCH 04/28] Improve examples on `ComputeStatistics` (#1881) --- docs/user_guide.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/user_guide.md b/docs/user_guide.md index 046d7dea87..315276277b 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -1106,6 +1106,7 @@ void BM_spin_empty(benchmark::State& state) { } BENCHMARK(BM_spin_empty) + ->Repetitions(3) // or add option --benchmark_repetitions=3 ->ComputeStatistics("max", [](const std::vector& v) -> double { return *(std::max_element(std::begin(v), std::end(v))); }) @@ -1125,8 +1126,9 @@ void BM_spin_empty(benchmark::State& state) { } BENCHMARK(BM_spin_empty) + ->Repetitions(3) // or add option --benchmark_repetitions=3 ->ComputeStatistics("ratio", [](const std::vector& v) -> double { - return std::begin(v) / std::end(v); + return v.front() / v.back(); }, benchmark::StatisticUnit::kPercentage) ->Arg(512); ``` From c58e6d0710581e3a08d65c349664128a8d9a2461 Mon Sep 17 00:00:00 2001 From: Dominic Hamon Date: Thu, 28 Nov 2024 16:51:38 +0000 Subject: [PATCH 05/28] v1.9.1 bump --- CMakeLists.txt | 2 +- MODULE.bazel | 2 +- bindings/python/google_benchmark/__init__.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c90529d8b3..f045fcd848 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ # Require CMake 3.10. If available, use the policies up to CMake 3.22. cmake_minimum_required (VERSION 3.13...3.22) -project (benchmark VERSION 1.9.0 LANGUAGES CXX) +project (benchmark VERSION 1.9.1 LANGUAGES CXX) 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) diff --git a/MODULE.bazel b/MODULE.bazel index 092ee18068..62870f74f7 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -1,6 +1,6 @@ module( name = "google_benchmark", - version = "1.9.0", + version = "1.9.1", ) bazel_dep(name = "bazel_skylib", version = "1.7.1") diff --git a/bindings/python/google_benchmark/__init__.py b/bindings/python/google_benchmark/__init__.py index e7870c854c..7006352669 100644 --- a/bindings/python/google_benchmark/__init__.py +++ b/bindings/python/google_benchmark/__init__.py @@ -50,7 +50,7 @@ def my_benchmark(state): oNSquared as oNSquared, ) -__version__ = "1.9.0" +__version__ = "1.9.1" class __OptionMaker: From 4b0533b726dd8613f7d5fd0d1d044ce81f05651d Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Fri, 29 Nov 2024 12:06:08 +0100 Subject: [PATCH 06/28] Add artifact name to download before wheel PyPI upload (#1882) Otherwise, the folder structure gets messed up, and twine errors out. --- .github/workflows/wheels.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index b463ff83dd..e2a96bd067 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -77,7 +77,7 @@ jobs: merge_wheels: name: Merge all built wheels into one artifact runs-on: ubuntu-latest - needs: build_wheels + needs: [build_sdist, build_wheels] steps: - name: Merge wheels uses: actions/upload-artifact/merge@v4 @@ -96,5 +96,6 @@ jobs: steps: - uses: actions/download-artifact@v4 with: + name: dist path: dist - uses: pypa/gh-action-pypi-publish@release/v1 From 3d88affa59e15018831cc36229c43ab9e741a667 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Fri, 29 Nov 2024 12:55:32 +0100 Subject: [PATCH 07/28] Remove if statement from wheel upload job (#1883) This to see if it works with the new artifact download config. --- .github/workflows/wheels.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index e2a96bd067..74676be830 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -90,7 +90,6 @@ jobs: name: Publish google-benchmark wheels to PyPI needs: [merge_wheels] runs-on: ubuntu-latest - if: github.event_name == 'release' && github.event.action == 'published' permissions: id-token: write steps: From b2b0aab464b1d5be3cf1728d36bb03f8b84f246f Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Tue, 3 Dec 2024 18:42:57 +0100 Subject: [PATCH 08/28] Fix malformed clang invocation in build_ext.run (#1884) The fix is, unsurprisingly, to not invoke clang at all, because we use Bazel to build everything anyway. This also means that we can drop the setuptools pin. --- pyproject.toml | 2 +- setup.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 14f173f956..338b0b907d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ["setuptools<73"] +requires = ["setuptools"] build-backend = "setuptools.build_meta" [project] diff --git a/setup.py b/setup.py index 69cc49da7a..5393350824 100644 --- a/setup.py +++ b/setup.py @@ -77,7 +77,6 @@ class BuildBazelExtension(build_ext.build_ext): def run(self): for ext in self.extensions: self.bazel_build(ext) - super().run() # explicitly call `bazel shutdown` for graceful exit self.spawn(["bazel", "shutdown"]) From b32ae9c9afd800a010bac61a1e4a44aff24094e1 Mon Sep 17 00:00:00 2001 From: Dominic Hamon Date: Tue, 10 Dec 2024 12:04:53 +0000 Subject: [PATCH 09/28] remove noenable_bzlmod as workspace support is going away --- .github/workflows/bazel.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index b50a8f6464..f86b1a06d7 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -6,7 +6,7 @@ on: jobs: build_and_test_default: - name: bazel.${{ matrix.os }}.${{ matrix.bzlmod && 'bzlmod' || 'no_bzlmod' }} + name: bazel.${{ matrix.os }} runs-on: ${{ matrix.os }} strategy: fail-fast: false @@ -28,8 +28,8 @@ jobs: - name: build run: | - bazel build ${{ matrix.bzlmod && '--enable_bzlmod' || '--noenable_bzlmod' }} //:benchmark //:benchmark_main //test/... + bazel build //:benchmark //:benchmark_main //test/... - name: test run: | - bazel test ${{ matrix.bzlmod && '--enable_bzlmod' || '--noenable_bzlmod' }} --test_output=all //test/... + bazel test --test_output=all //test/... From c8c66e0b4a4c37558ce40ab4ca45f9d3f86a97d3 Mon Sep 17 00:00:00 2001 From: Dominic Hamon Date: Tue, 10 Dec 2024 12:07:53 +0000 Subject: [PATCH 10/28] remove unnecessary bazel action parameter --- .github/workflows/bazel.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index f86b1a06d7..ea231a3c4d 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -12,7 +12,6 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] - bzlmod: [false, true] steps: - uses: actions/checkout@v4 From ae52c9e66e776d401636aa9e26b3a1b50746f3ab Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Tue, 10 Dec 2024 13:27:12 +0100 Subject: [PATCH 11/28] Remove wheel merge job, merge artifacts on download (#1886) This is supported by `actions/download-artifact@v4`, and endorsed by cibuildwheel in their documentation (see https://cibuildwheel.pypa.io/en/stable/deliver-to-pypi/#github-actions). Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- .github/workflows/wheels.yml | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index 74676be830..0569dcc90f 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -53,12 +53,11 @@ jobs: platforms: all - name: Build wheels on ${{ matrix.os }} using cibuildwheel - uses: pypa/cibuildwheel@v2.21.3 + uses: pypa/cibuildwheel@v2.22.0 env: CIBW_BUILD: "cp310-* cp311-* cp312-*" CIBW_BUILD_FRONTEND: "build[uv]" CIBW_SKIP: "*-musllinux_*" - CIBW_TEST_SKIP: "cp38-macosx_*:arm64" CIBW_ARCHS_LINUX: auto64 aarch64 CIBW_ARCHS_WINDOWS: auto64 CIBW_BEFORE_ALL_LINUX: bash .github/install_bazel.sh @@ -74,27 +73,16 @@ jobs: name: dist-${{ matrix.os }} path: wheelhouse/*.whl - merge_wheels: - name: Merge all built wheels into one artifact - runs-on: ubuntu-latest - needs: [build_sdist, build_wheels] - steps: - - name: Merge wheels - uses: actions/upload-artifact/merge@v4 - with: - name: dist - pattern: dist-* - delete-merged: true - pypi_upload: name: Publish google-benchmark wheels to PyPI - needs: [merge_wheels] + needs: [build_sdist, build_wheels] runs-on: ubuntu-latest permissions: id-token: write steps: - uses: actions/download-artifact@v4 with: - name: dist path: dist + pattern: dist-* + merge-multiple: true - uses: pypa/gh-action-pypi-publish@release/v1 From f4f93b5553ced834b2120048f65690cddb4b7a2f Mon Sep 17 00:00:00 2001 From: Devon Loehr Date: Tue, 10 Dec 2024 10:29:03 -0500 Subject: [PATCH 12/28] Change SDK version check (#1887) Now that github seems to have updated its builders, perhaps we can check the SDK version the more standard way. --- src/sysinfo.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sysinfo.cc b/src/sysinfo.cc index 7148598264..49bff75e58 100644 --- a/src/sysinfo.cc +++ b/src/sysinfo.cc @@ -354,8 +354,7 @@ std::vector GetCacheSizesWindows() { C.type = "Unknown"; switch (cache.Type) { // Windows SDK version >= 10.0.26100.0 -// 0x0A000010 is the value of NTDDI_WIN11_GE -#if NTDDI_VERSION >= 0x0A000010 +#ifdef NTDDI_WIN11_GE case CacheUnknown: break; #endif From 5af40e824defae36fb70521c793af0594599ac7e Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 8 Jan 2025 03:26:44 -0800 Subject: [PATCH 13/28] [bazel] Remove selects on CPU (#1892) In a future version of bazel this produces a warning. In this case using only the platform being windows is enough. Fixes: ``` WARNING: /.../benchmark/BUILD.bazel:29:15: in config_setting rule //:windows: select() on cpu is deprecated. Use platform constraints instead: https://bazel.build/docs/configurable-attributes#platforms. ``` --- BUILD.bazel | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 3451b4e758..95557a35b2 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -17,30 +17,12 @@ COPTS = [ "-Werror=old-style-cast", ] -config_setting( - name = "qnx", - constraint_values = ["@platforms//os:qnx"], - values = { - "cpu": "x64_qnx", - }, - visibility = [":__subpackages__"], -) - config_setting( name = "windows", constraint_values = ["@platforms//os:windows"], - values = { - "cpu": "x64_windows", - }, visibility = [":__subpackages__"], ) -config_setting( - name = "macos", - constraint_values = ["@platforms//os:macos"], - visibility = ["//visibility:public"], -) - config_setting( name = "perfcounters", define_values = { From f65741b2bd92461dc2c816056eb9c996ae48ad62 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Wed, 8 Jan 2025 13:03:53 +0100 Subject: [PATCH 14/28] cycleclock: Support for PA-RISC (hppa) architecture (#1894) Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- src/cycleclock.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/cycleclock.h b/src/cycleclock.h index bd62f5d7e7..7852f3df52 100644 --- a/src/cycleclock.h +++ b/src/cycleclock.h @@ -229,6 +229,16 @@ inline BENCHMARK_ALWAYS_INLINE int64_t Now() { struct timeval tv; gettimeofday(&tv, nullptr); return static_cast(tv.tv_sec) * 1000000 + tv.tv_usec; +#elif defined(__hppa__) + // HP PA-RISC provides a user-readable clock counter (cr16), but + // it's not syncronized across CPUs and only 32-bit wide when programs + // are built as 32-bit binaries. + // Use clock_gettime(CLOCK_MONOTONIC, ...) instead of gettimeofday + // because is provides nanosecond resolution. + // Initialize to always return 0 if clock_gettime fails. + struct timespec ts = {0, 0}; + clock_gettime(CLOCK_MONOTONIC, &ts); + return static_cast(ts.tv_sec) * 1000000000 + ts.tv_nsec; #else // The soft failover to a generic implementation is automatic only for ARM. // For other platforms the developer is expected to make an attempt to create From 7ddc400d6232259e9acc6e09cd77c9a6f758e030 Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 8 Jan 2025 12:41:17 +0000 Subject: [PATCH 15/28] fix: remove clang-cl compilation warning (#1895) - MP flag only applies to cl, not cl frontends to other compilers (e.g. clang-cl, icx-cl etc). Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f045fcd848..fd6906040d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -147,7 +147,12 @@ set(CMAKE_CXX_EXTENSIONS OFF) if (MSVC) # Turn compiler warnings up to 11 string(REGEX REPLACE "[-/]W[1-4]" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4 /MP") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4") + + # MP flag only applies to cl, not cl frontends to other compilers (e.g. clang-cl, icx-cl etc) + if(CMAKE_CXX_COMPILER_ID MATCHES MSVC) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /MP") + endif() add_definitions(-D_CRT_SECURE_NO_WARNINGS) if(BENCHMARK_ENABLE_WERROR) From f981f58da37d1e7214f8b498d5055e48361380b9 Mon Sep 17 00:00:00 2001 From: 0dminnimda <0dminnimda@gmail.com> Date: Wed, 8 Jan 2025 15:49:09 +0300 Subject: [PATCH 16/28] README.md: fix build instructions (#1880) Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8e5428f995..c77f9b6cbe 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ $ cmake -E make_directory "build" # Generate build system files with cmake, and download any dependencies. $ cmake -E chdir "build" cmake -DBENCHMARK_DOWNLOAD_DEPENDENCIES=on -DCMAKE_BUILD_TYPE=Release ../ # or, starting with CMake 3.13, use a simpler form: -# cmake -DCMAKE_BUILD_TYPE=Release -S . -B "build" +# cmake -DBENCHMARK_DOWNLOAD_DEPENDENCIES=on -DCMAKE_BUILD_TYPE=Release -S . -B "build" # Build the library. $ cmake --build "build" --config Release ``` From 077db43001b42af3ad23e993b2bdcb4fadb7bcf8 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Wed, 8 Jan 2025 17:54:08 +0100 Subject: [PATCH 17/28] cycleclock: Use cock_gettime() as fallback for any Linux architecture (#1899) The Linux kernel provides the clock_gettime() functions since a long time already, so it's possible to use it as a generic fallback option for any architecture if no other (better) possibility has been provided instead. I noticed the benchmark package failed to build on debian on the SH-4 architecture, so with this change SH-4 is now the first user of this fallback option. --- src/cycleclock.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cycleclock.h b/src/cycleclock.h index 7852f3df52..03e02f8055 100644 --- a/src/cycleclock.h +++ b/src/cycleclock.h @@ -229,10 +229,12 @@ inline BENCHMARK_ALWAYS_INLINE int64_t Now() { struct timeval tv; gettimeofday(&tv, nullptr); return static_cast(tv.tv_sec) * 1000000 + tv.tv_usec; -#elif defined(__hppa__) +#elif defined(__hppa__) || defined(__linux__) + // Fallback for all other architectures with a recent Linux kernel, e.g.: // HP PA-RISC provides a user-readable clock counter (cr16), but // it's not syncronized across CPUs and only 32-bit wide when programs // are built as 32-bit binaries. + // Same for SH-4 and possibly others. // Use clock_gettime(CLOCK_MONOTONIC, ...) instead of gettimeofday // because is provides nanosecond resolution. // Initialize to always return 0 if clock_gettime fails. From 39be87d3004ff9ff4cdf736651af80c3d15e2497 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Thu, 9 Jan 2025 11:47:29 +0100 Subject: [PATCH 18/28] Fix runtime crash when parsing /proc/cpuinfo fails (#1900) The testcase fails on sparc64, because the parsing of /proc/cpuinfo fails and thus currently returns "0" CPUs which finally leads to division-by-zero faults in the tests. Fix the issue by returning at least "1" CPU which allows the tests to run. A error message will be printed in any case. Long-term the code should be fixed to parse the cpuinfo output on sparch which looks like this: ... type : sun4v ncpus probed : 48 ncpus active : 48 --- src/sysinfo.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sysinfo.cc b/src/sysinfo.cc index 49bff75e58..ce14b8d8ed 100644 --- a/src/sysinfo.cc +++ b/src/sysinfo.cc @@ -561,10 +561,12 @@ int GetNumCPUsImpl() { } int GetNumCPUs() { - const int num_cpus = GetNumCPUsImpl(); + int num_cpus = GetNumCPUsImpl(); if (num_cpus < 1) { std::cerr << "Unable to extract number of CPUs. If your platform uses " "/proc/cpuinfo, custom support may need to be added.\n"; + /* There is at least one CPU which we run on. */ + num_cpus = 1; } return num_cpus; } From c24774dc4f4402c3ad150363321cc972ed2669e7 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Thu, 9 Jan 2025 17:07:43 +0100 Subject: [PATCH 19/28] Get number of CPUs with sysconf() on Linux (#1901) * Get number of CPUs with sysconf() on Linux Avoid parsing the /proc/cpuinfo just to get number of CPUs. Instead use the portable function provided by glibc. * Update sysinfo.cc --- src/sysinfo.cc | 54 +++----------------------------------------------- 1 file changed, 3 insertions(+), 51 deletions(-) diff --git a/src/sysinfo.cc b/src/sysinfo.cc index ce14b8d8ed..eddd430e68 100644 --- a/src/sysinfo.cc +++ b/src/sysinfo.cc @@ -492,14 +492,14 @@ int GetNumCPUsImpl() { GetSystemInfo(&sysinfo); // number of logical processors in the current group return static_cast(sysinfo.dwNumberOfProcessors); -#elif defined(BENCHMARK_OS_SOLARIS) +#elif defined(__linux__) || defined(BENCHMARK_OS_SOLARIS) // Returns -1 in case of a failure. - long num_cpu = sysconf(_SC_NPROCESSORS_ONLN); + int num_cpu = static_cast(sysconf(_SC_NPROCESSORS_ONLN)); if (num_cpu < 0) { PrintErrorAndDie("sysconf(_SC_NPROCESSORS_ONLN) failed with error: ", strerror(errno)); } - return (int)num_cpu; + return num_cpu; #elif defined(BENCHMARK_OS_QNX) return static_cast(_syspage_ptr->num_cpu); #elif defined(BENCHMARK_OS_QURT) @@ -508,54 +508,6 @@ int GetNumCPUsImpl() { hardware_threads.max_hthreads = 1; } return hardware_threads.max_hthreads; -#else - int num_cpus = 0; - int max_id = -1; - std::ifstream f("/proc/cpuinfo"); - if (!f.is_open()) { - std::cerr << "Failed to open /proc/cpuinfo\n"; - return -1; - } -#if defined(__alpha__) - const std::string Key = "cpus detected"; -#else - const std::string Key = "processor"; -#endif - std::string ln; - while (std::getline(f, ln)) { - if (ln.empty()) continue; - std::size_t split_idx = ln.find(':'); - std::string value; -#if defined(__s390__) - // s390 has another format in /proc/cpuinfo - // it needs to be parsed differently - if (split_idx != std::string::npos) - value = ln.substr(Key.size() + 1, split_idx - Key.size() - 1); -#else - if (split_idx != std::string::npos) value = ln.substr(split_idx + 1); -#endif - if (ln.size() >= Key.size() && ln.compare(0, Key.size(), Key) == 0) { - num_cpus++; - if (!value.empty()) { - const int cur_id = benchmark::stoi(value); - max_id = std::max(cur_id, max_id); - } - } - } - if (f.bad()) { - PrintErrorAndDie("Failure reading /proc/cpuinfo"); - } - if (!f.eof()) { - PrintErrorAndDie("Failed to read to end of /proc/cpuinfo"); - } - f.close(); - - if ((max_id + 1) != num_cpus) { - fprintf(stderr, - "CPU ID assignments in /proc/cpuinfo seem messed up." - " This is usually caused by a bad BIOS.\n"); - } - return num_cpus; #endif BENCHMARK_UNREACHABLE(); } From 4834ae9e57589cde4c8fbbdbdcb680d54c0a38e1 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Mon, 13 Jan 2025 14:33:04 +0100 Subject: [PATCH 20/28] Update nanobind-bazel to v2.4.0 (#1904) Contains nanobind v2.4.0, which brings some more functionality, free-threading fixes, and performance improvements. --- MODULE.bazel | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 62870f74f7..62a3aa8ba4 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -8,7 +8,7 @@ bazel_dep(name = "platforms", version = "0.0.10") bazel_dep(name = "rules_foreign_cc", version = "0.10.1") bazel_dep(name = "rules_cc", version = "0.0.9") -bazel_dep(name = "rules_python", version = "0.37.0", dev_dependency = True) +bazel_dep(name = "rules_python", version = "1.0.0", dev_dependency = True) bazel_dep(name = "googletest", version = "1.14.0", dev_dependency = True, repo_name = "com_google_googletest") bazel_dep(name = "libpfm", version = "4.11.0") @@ -39,4 +39,4 @@ use_repo(pip, "tools_pip_deps") # -- bazel_dep definitions -- # -bazel_dep(name = "nanobind_bazel", version = "2.2.0", dev_dependency = True) +bazel_dep(name = "nanobind_bazel", version = "2.4.0", dev_dependency = True) From d6536acfe80d05d2b9e63e4dc786dd1dc9f0b960 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Mon, 13 Jan 2025 14:38:59 +0100 Subject: [PATCH 21/28] ci: Update pre-commit hooks (#1905) As a fix, also turn the comment in libpfm's build file into a proper Starlark docstring. Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- .pre-commit-config.yaml | 6 +++--- tools/libpfm.BUILD.bazel | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2a51592edf..78a4580763 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,17 +1,17 @@ repos: - repo: https://github.com/keith/pre-commit-buildifier - rev: 7.3.1 + rev: 8.0.0 hooks: - id: buildifier - id: buildifier-lint - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.13.0 + rev: v1.14.1 hooks: - id: mypy types_or: [ python, pyi ] args: [ "--ignore-missing-imports", "--scripts-are-modules" ] - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.7.2 + rev: v0.9.1 hooks: - id: ruff args: [ --fix, --exit-non-zero-on-fix ] diff --git a/tools/libpfm.BUILD.bazel b/tools/libpfm.BUILD.bazel index 62695342aa..4ef112352f 100644 --- a/tools/libpfm.BUILD.bazel +++ b/tools/libpfm.BUILD.bazel @@ -1,5 +1,4 @@ -# Build rule for libpfm, which is required to collect performance counters for -# BENCHMARK_ENABLE_LIBPFM builds. +"""Build rule for libpfm, which is required to collect performance counters for BENCHMARK_ENABLE_LIBPFM builds.""" load("@rules_foreign_cc//foreign_cc:defs.bzl", "make") From ecb5df647341456596928d72c4c56b3f438a005e Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Wed, 22 Jan 2025 12:50:00 +0100 Subject: [PATCH 22/28] Lint Python: Add more ruff rules (#1909) * Lint Python: Add more ruff rules * range(len()) --> enumerate() * zip(strict=True) --- .pre-commit-config.yaml | 2 +- bindings/python/google_benchmark/example.py | 3 +- pyproject.toml | 6 +- setup.py | 14 ++- tools/compare.py | 4 +- tools/gbench/report.py | 106 +++++++++++--------- tools/gbench/util.py | 46 ++++----- tools/strip_asm.py | 14 +-- 8 files changed, 98 insertions(+), 97 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 78a4580763..c16928a946 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,7 +11,7 @@ repos: types_or: [ python, pyi ] args: [ "--ignore-missing-imports", "--scripts-are-modules" ] - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.9.1 + rev: v0.9.2 hooks: - id: ruff args: [ --fix, --exit-non-zero-on-fix ] diff --git a/bindings/python/google_benchmark/example.py b/bindings/python/google_benchmark/example.py index b92245ea67..5909c0fc0e 100644 --- a/bindings/python/google_benchmark/example.py +++ b/bindings/python/google_benchmark/example.py @@ -57,7 +57,7 @@ def skipped(state): state.skip_with_error("some error") return # NOTE: You must explicitly return, or benchmark will continue. - ... # Benchmark code would be here. + # Benchmark code would be here. @benchmark.register @@ -78,7 +78,6 @@ def custom_counters(state): num_foo = 0.0 while state: # Benchmark some code here - pass # Collect some custom metric named foo num_foo += 0.13 diff --git a/pyproject.toml b/pyproject.toml index 338b0b907d..761473c204 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,9 +68,11 @@ target-version = "py311" [tool.ruff.lint] # Enable pycodestyle (`E`, `W`), Pyflakes (`F`), and isort (`I`) codes by default. -select = ["E", "F", "I", "W"] +select = ["ASYNC", "B", "C4", "C90", "E", "F", "I", "PERF", "PIE", "PT018", "RUF", "SIM", "UP", "W"] ignore = [ - "E501", # line too long + "E501", # line too long + "PLW2901", # redefined-loop-name + "UP031", # printf-string-formatting ] [tool.ruff.lint.isort] diff --git a/setup.py b/setup.py index 5393350824..3c0269b199 100644 --- a/setup.py +++ b/setup.py @@ -4,8 +4,9 @@ import re import shutil import sys +from collections.abc import Generator from pathlib import Path -from typing import Any, Generator +from typing import Any import setuptools from setuptools.command import build_ext @@ -86,15 +87,14 @@ def copy_extensions_to_source(self): This is done in the ``bazel_build`` method, so it's not necessary to do again in the `build_ext` base class. """ - pass - def bazel_build(self, ext: BazelExtension) -> None: + def bazel_build(self, ext: BazelExtension) -> None: # noqa: C901 """Runs the bazel build to create the package.""" temp_path = Path(self.build_temp) # We round to the minor version, which makes rules_python # look up the latest available patch version internally. - python_version = "{0}.{1}".format(*sys.version_info[:2]) + python_version = "{}.{}".format(*sys.version_info[:2]) bazel_argv = [ "bazel", @@ -142,9 +142,7 @@ def bazel_build(self, ext: BazelExtension) -> None: # we do not want the bare .so file included # when building for ABI3, so we require a # full and exact match on the file extension. - if "".join(fp.suffixes) == suffix: - should_copy = True - elif fp.suffix == ".pyi": + if "".join(fp.suffixes) == suffix or fp.suffix == ".pyi": should_copy = True elif Path(root) == srcdir and f == "py.typed": # copy py.typed, but only at the package root. @@ -155,7 +153,7 @@ def bazel_build(self, ext: BazelExtension) -> None: setuptools.setup( - cmdclass=dict(build_ext=BuildBazelExtension), + cmdclass={"build_ext": BuildBazelExtension}, package_data={"google_benchmark": ["py.typed", "*.pyi"]}, ext_modules=[ BazelExtension( diff --git a/tools/compare.py b/tools/compare.py index 7572520cc0..36cbe07569 100755 --- a/tools/compare.py +++ b/tools/compare.py @@ -94,9 +94,7 @@ def create_parser(): dest="utest", default=True, action="store_false", - help="The tool can do a two-tailed Mann-Whitney U test with the null hypothesis that it is equally likely that a randomly selected value from one sample will be less than or greater than a randomly selected value from a second sample.\nWARNING: requires **LARGE** (no less than {}) number of repetitions to be meaningful!\nThe test is being done by default, if at least {} repetitions were done.\nThis option can disable the U Test.".format( - report.UTEST_OPTIMAL_REPETITIONS, report.UTEST_MIN_REPETITIONS - ), + help=f"The tool can do a two-tailed Mann-Whitney U test with the null hypothesis that it is equally likely that a randomly selected value from one sample will be less than or greater than a randomly selected value from a second sample.\nWARNING: requires **LARGE** (no less than {report.UTEST_OPTIMAL_REPETITIONS}) number of repetitions to be meaningful!\nThe test is being done by default, if at least {report.UTEST_MIN_REPETITIONS} repetitions were done.\nThis option can disable the U Test.", ) alpha_default = 0.05 utest.add_argument( diff --git a/tools/gbench/report.py b/tools/gbench/report.py index 7158fd1654..6b58918bfc 100644 --- a/tools/gbench/report.py +++ b/tools/gbench/report.py @@ -14,7 +14,7 @@ from scipy.stats import gmean, mannwhitneyu -class BenchmarkColor(object): +class BenchmarkColor: def __init__(self, name, code): self.name = name self.code = code @@ -249,9 +249,7 @@ def get_utest_color(pval): # We still got some results to show but issue a warning about it. if not utest["have_optimal_repetitions"]: dsc_color = BC_WARNING - dsc += ". WARNING: Results unreliable! {}+ repetitions recommended.".format( - UTEST_OPTIMAL_REPETITIONS - ) + dsc += f". WARNING: Results unreliable! {UTEST_OPTIMAL_REPETITIONS}+ repetitions recommended." special_str = "{}{:<{}s}{endc}{}{:16.4f}{endc}{}{:16.4f}{endc}{} {}" @@ -260,7 +258,7 @@ def get_utest_color(pval): use_color, special_str, BC_HEADER, - "{}{}".format(bc_name, UTEST_COL_NAME), + f"{bc_name}{UTEST_COL_NAME}", first_col_width, get_utest_color(utest["time_pvalue"]), utest["time_pvalue"], @@ -285,7 +283,7 @@ def get_difference_report(json1, json2, utest=False): partitions = partition_benchmarks(json1, json2) for partition in partitions: benchmark_name = partition[0][0]["name"] - label = partition[0][0]["label"] if "label" in partition[0][0] else "" + label = partition[0][0].get("label", "") time_unit = partition[0][0]["time_unit"] measurements = [] utest_results = {} @@ -329,11 +327,7 @@ def get_difference_report(json1, json2, utest=False): # time units which are not compatible with other time units in the # benchmark suite. if measurements: - run_type = ( - partition[0][0]["run_type"] - if "run_type" in partition[0][0] - else "" - ) + run_type = partition[0][0].get("run_type", "") aggregate_name = ( partition[0][0]["aggregate_name"] if run_type == "aggregate" @@ -464,7 +458,7 @@ def load_results(self): os.path.dirname(os.path.realpath(__file__)), "Inputs" ) testOutput = os.path.join(testInputs, "test3_run0.json") - with open(testOutput, "r") as f: + with open(testOutput) as f: json = json.load(f) return json @@ -480,8 +474,8 @@ def test_basic(self): print("\n") print("\n".join(output_lines)) self.assertEqual(len(output_lines), len(expect_lines)) - for i in range(0, len(output_lines)): - self.assertEqual(expect_lines[i], output_lines[i]) + for i, output_line in enumerate(output_lines): + self.assertEqual(expect_lines[i], output_line) class TestReportDifference(unittest.TestCase): @@ -495,9 +489,9 @@ def load_results(): ) testOutput1 = os.path.join(testInputs, "test1_run1.json") testOutput2 = os.path.join(testInputs, "test1_run2.json") - with open(testOutput1, "r") as f: + with open(testOutput1) as f: json1 = json.load(f) - with open(testOutput2, "r") as f: + with open(testOutput2) as f: json2 = json.load(f) return json1, json2 @@ -584,8 +578,8 @@ def test_json_diff_report_pretty_printing(self): print("\n") print("\n".join(output_lines_with_header)) self.assertEqual(len(output_lines), len(expect_lines)) - for i in range(0, len(output_lines)): - parts = [x for x in output_lines[i].split(" ") if x] + for i, output_line in enumerate(output_lines): + parts = [x for x in output_line.split(" ") if x] self.assertEqual(len(parts), 7) self.assertEqual(expect_lines[i], parts) @@ -819,7 +813,9 @@ def test_json_diff_report_output(self): }, ] self.assertEqual(len(self.json_diff_report), len(expected_output)) - for out, expected in zip(self.json_diff_report, expected_output): + for out, expected in zip( + self.json_diff_report, expected_output, strict=True + ): self.assertEqual(out["name"], expected["name"]) self.assertEqual(out["label"], expected["label"]) self.assertEqual(out["time_unit"], expected["time_unit"]) @@ -837,7 +833,7 @@ def load_result(): os.path.dirname(os.path.realpath(__file__)), "Inputs" ) testOutput = os.path.join(testInputs, "test2_run.json") - with open(testOutput, "r") as f: + with open(testOutput) as f: json = json.load(f) return json @@ -861,8 +857,8 @@ def test_json_diff_report_pretty_printing(self): print("\n") print("\n".join(output_lines_with_header)) self.assertEqual(len(output_lines), len(expect_lines)) - for i in range(0, len(output_lines)): - parts = [x for x in output_lines[i].split(" ") if x] + for i, output_line in enumerate(output_lines): + parts = [x for x in output_line.split(" ") if x] self.assertEqual(len(parts), 7) self.assertEqual(expect_lines[i], parts) @@ -947,7 +943,9 @@ def test_json_diff_report(self): }, ] self.assertEqual(len(self.json_diff_report), len(expected_output)) - for out, expected in zip(self.json_diff_report, expected_output): + for out, expected in zip( + self.json_diff_report, expected_output, strict=True + ): self.assertEqual(out["name"], expected["name"]) self.assertEqual(out["time_unit"], expected["time_unit"]) assert_utest(self, out, expected) @@ -965,9 +963,9 @@ def load_results(): ) testOutput1 = os.path.join(testInputs, "test3_run0.json") testOutput2 = os.path.join(testInputs, "test3_run1.json") - with open(testOutput1, "r") as f: + with open(testOutput1) as f: json1 = json.load(f) - with open(testOutput2, "r") as f: + with open(testOutput2) as f: json2 = json.load(f) return json1, json2 @@ -1025,8 +1023,8 @@ def test_json_diff_report_pretty_printing(self): print("\n") print("\n".join(output_lines_with_header)) self.assertEqual(len(output_lines), len(expect_lines)) - for i in range(0, len(output_lines)): - parts = [x for x in output_lines[i].split(" ") if x] + for i, output_line in enumerate(output_lines): + parts = [x for x in output_line.split(" ") if x] self.assertEqual(expect_lines[i], parts) def test_json_diff_report_pretty_printing_aggregates_only(self): @@ -1081,8 +1079,8 @@ def test_json_diff_report_pretty_printing_aggregates_only(self): print("\n") print("\n".join(output_lines_with_header)) self.assertEqual(len(output_lines), len(expect_lines)) - for i in range(0, len(output_lines)): - parts = [x for x in output_lines[i].split(" ") if x] + for i, output_line in enumerate(output_lines): + parts = [x for x in output_line.split(" ") if x] self.assertEqual(expect_lines[i], parts) def test_json_diff_report(self): @@ -1190,7 +1188,9 @@ def test_json_diff_report(self): }, ] self.assertEqual(len(self.json_diff_report), len(expected_output)) - for out, expected in zip(self.json_diff_report, expected_output): + for out, expected in zip( + self.json_diff_report, expected_output, strict=True + ): self.assertEqual(out["name"], expected["name"]) self.assertEqual(out["time_unit"], expected["time_unit"]) assert_utest(self, out, expected) @@ -1210,9 +1210,9 @@ def load_results(): ) testOutput1 = os.path.join(testInputs, "test3_run0.json") testOutput2 = os.path.join(testInputs, "test3_run1.json") - with open(testOutput1, "r") as f: + with open(testOutput1) as f: json1 = json.load(f) - with open(testOutput2, "r") as f: + with open(testOutput2) as f: json2 = json.load(f) return json1, json2 @@ -1270,8 +1270,8 @@ def test_json_diff_report_pretty_printing(self): print("\n") print("\n".join(output_lines_with_header)) self.assertEqual(len(output_lines), len(expect_lines)) - for i in range(0, len(output_lines)): - parts = [x for x in output_lines[i].split(" ") if x] + for i, output_line in enumerate(output_lines): + parts = [x for x in output_line.split(" ") if x] self.assertEqual(expect_lines[i], parts) def test_json_diff_report(self): @@ -1380,7 +1380,9 @@ def test_json_diff_report(self): }, ] self.assertEqual(len(self.json_diff_report), len(expected_output)) - for out, expected in zip(self.json_diff_report, expected_output): + for out, expected in zip( + self.json_diff_report, expected_output, strict=True + ): self.assertEqual(out["name"], expected["name"]) self.assertEqual(out["time_unit"], expected["time_unit"]) assert_utest(self, out, expected) @@ -1398,9 +1400,9 @@ def load_results(): ) testOutput1 = os.path.join(testInputs, "test4_run0.json") testOutput2 = os.path.join(testInputs, "test4_run1.json") - with open(testOutput1, "r") as f: + with open(testOutput1) as f: json1 = json.load(f) - with open(testOutput2, "r") as f: + with open(testOutput2) as f: json2 = json.load(f) return json1, json2 @@ -1416,8 +1418,8 @@ def test_json_diff_report_pretty_printing(self): print("\n") print("\n".join(output_lines_with_header)) self.assertEqual(len(output_lines), len(expect_lines)) - for i in range(0, len(output_lines)): - parts = [x for x in output_lines[i].split(" ") if x] + for i, output_line in enumerate(output_lines): + parts = [x for x in output_line.split(" ") if x] self.assertEqual(expect_lines[i], parts) def test_json_diff_report(self): @@ -1439,7 +1441,9 @@ def test_json_diff_report(self): } ] self.assertEqual(len(self.json_diff_report), len(expected_output)) - for out, expected in zip(self.json_diff_report, expected_output): + for out, expected in zip( + self.json_diff_report, expected_output, strict=True + ): self.assertEqual(out["name"], expected["name"]) self.assertEqual(out["time_unit"], expected["time_unit"]) assert_utest(self, out, expected) @@ -1456,7 +1460,7 @@ def load_result(): os.path.dirname(os.path.realpath(__file__)), "Inputs" ) testOutput = os.path.join(testInputs, "test4_run.json") - with open(testOutput, "r") as f: + with open(testOutput) as f: json = json.load(f) return json @@ -1480,13 +1484,15 @@ def test_json_diff_report_pretty_printing(self): "88 family 1 instance 1 aggregate", ] - for n in range(len(self.json["benchmarks"]) ** 2): + for _n in range(len(self.json["benchmarks"]) ** 2): random.shuffle(self.json["benchmarks"]) sorted_benchmarks = util.sort_benchmark_results(self.json)[ "benchmarks" ] self.assertEqual(len(expected_names), len(sorted_benchmarks)) - for out, expected in zip(sorted_benchmarks, expected_names): + for out, expected in zip( + sorted_benchmarks, expected_names, strict=True + ): self.assertEqual(out["name"], expected) @@ -1503,12 +1509,12 @@ def load_results(): ) testOutput1 = os.path.join(testInputs, "test5_run0.json") testOutput2 = os.path.join(testInputs, "test5_run1.json") - with open(testOutput1, "r") as f: + with open(testOutput1) as f: json1 = json.load(f) json1["benchmarks"] = [ json1["benchmarks"][0] for i in range(1000) ] - with open(testOutput2, "r") as f: + with open(testOutput2) as f: json2 = json.load(f) json2["benchmarks"] = [ json2["benchmarks"][0] for i in range(1000) @@ -1535,8 +1541,8 @@ def test_json_diff_report_pretty_printing(self): ) output_lines = output_lines_with_header[2:] found = False - for i in range(0, len(output_lines)): - parts = [x for x in output_lines[i].split(" ") if x] + for output_line in output_lines: + parts = [x for x in output_line.split(" ") if x] found = expect_line == parts if found: break @@ -1578,7 +1584,9 @@ def test_json_diff_report(self): }, ] self.assertEqual(len(self.json_diff_report), len(expected_output)) - for out, expected in zip(self.json_diff_report, expected_output): + for out, expected in zip( + self.json_diff_report, expected_output, strict=True + ): self.assertEqual(out["name"], expected["name"]) self.assertEqual(out["time_unit"], expected["time_unit"]) assert_utest(self, out, expected) @@ -1602,7 +1610,7 @@ def assert_utest(unittest_instance, lhs, rhs): def assert_measurements(unittest_instance, lhs, rhs): - for m1, m2 in zip(lhs["measurements"], rhs["measurements"]): + for m1, m2 in zip(lhs["measurements"], rhs["measurements"], strict=False): unittest_instance.assertEqual(m1["real_time"], m2["real_time"]) unittest_instance.assertEqual(m1["cpu_time"], m2["cpu_time"]) # m1['time'] and m1['cpu'] hold values which are being calculated, diff --git a/tools/gbench/util.py b/tools/gbench/util.py index 1119a1a2ca..596b51a07c 100644 --- a/tools/gbench/util.py +++ b/tools/gbench/util.py @@ -46,7 +46,7 @@ def is_json_file(filename): 'False' otherwise. """ try: - with open(filename, "r") as f: + with open(filename) as f: json.load(f) return True except BaseException: @@ -97,7 +97,8 @@ def find_benchmark_flag(prefix, benchmark_flags): if it is found return the arg it specifies. If specified more than once the last value is returned. If the flag is not found None is returned. """ - assert prefix.startswith("--") and prefix.endswith("=") + assert prefix.startswith("--") + assert prefix.endswith("=") result = None for f in benchmark_flags: if f.startswith(prefix): @@ -110,7 +111,8 @@ def remove_benchmark_flags(prefix, benchmark_flags): Return a new list containing the specified benchmark_flags except those with the specified prefix. """ - assert prefix.startswith("--") and prefix.endswith("=") + assert prefix.startswith("--") + assert prefix.endswith("=") return [f for f in benchmark_flags if not f.startswith(prefix)] @@ -133,17 +135,16 @@ def benchmark_wanted(benchmark): name = benchmark.get("run_name", None) or benchmark["name"] return re.search(benchmark_filter, name) is not None - with open(fname, "r") as f: + with open(fname) as f: results = json.load(f) - if "context" in results: - if "json_schema_version" in results["context"]: - json_schema_version = results["context"]["json_schema_version"] - if json_schema_version != 1: - print( - "In %s, got unnsupported JSON schema version: %i, expected 1" - % (fname, json_schema_version) - ) - sys.exit(1) + if "json_schema_version" in results.get("context", {}): + json_schema_version = results["context"]["json_schema_version"] + if json_schema_version != 1: + print( + "In %s, got unnsupported JSON schema version: %i, expected 1" + % (fname, json_schema_version) + ) + sys.exit(1) if "benchmarks" in results: results["benchmarks"] = list( filter(benchmark_wanted, results["benchmarks"]) @@ -157,9 +158,7 @@ def sort_benchmark_results(result): # From inner key to the outer key! benchmarks = sorted( benchmarks, - key=lambda benchmark: benchmark["repetition_index"] - if "repetition_index" in benchmark - else -1, + key=lambda benchmark: benchmark.get("repetition_index", -1), ) benchmarks = sorted( benchmarks, @@ -169,15 +168,11 @@ def sort_benchmark_results(result): ) benchmarks = sorted( benchmarks, - key=lambda benchmark: benchmark["per_family_instance_index"] - if "per_family_instance_index" in benchmark - else -1, + key=lambda benchmark: benchmark.get("per_family_instance_index", -1), ) benchmarks = sorted( benchmarks, - key=lambda benchmark: benchmark["family_index"] - if "family_index" in benchmark - else -1, + key=lambda benchmark: benchmark.get("family_index", -1), ) result["benchmarks"] = benchmarks @@ -197,11 +192,12 @@ def run_benchmark(exe_name, benchmark_flags): is_temp_output = True thandle, output_name = tempfile.mkstemp() os.close(thandle) - benchmark_flags = list(benchmark_flags) + [ - "--benchmark_out=%s" % output_name + benchmark_flags = [ + *list(benchmark_flags), + "--benchmark_out=%s" % output_name, ] - cmd = [exe_name] + benchmark_flags + cmd = [exe_name, *benchmark_flags] print("RUNNING: %s" % " ".join(cmd)) exitCode = subprocess.call(cmd) if exitCode != 0: diff --git a/tools/strip_asm.py b/tools/strip_asm.py index bc3a774a79..14d80ed48d 100755 --- a/tools/strip_asm.py +++ b/tools/strip_asm.py @@ -73,16 +73,16 @@ def process_identifiers(line): parts = re.split(r"([a-zA-Z0-9_]+)", line) new_line = "" for tk in parts: - if is_identifier(tk): - if tk.startswith("__Z"): - tk = tk[1:] - elif ( + if is_identifier(tk) and ( + tk.startswith("__Z") + or ( tk.startswith("_") and len(tk) > 1 and tk[1].isalpha() and tk[1] != "Z" - ): - tk = tk[1:] + ) + ): + tk = tk[1:] new_line += tk return new_line @@ -148,7 +148,7 @@ def main(): print("ERROR: input file '%s' does not exist" % input) sys.exit(1) - with open(input, "r") as f: + with open(input) as f: contents = f.read() new_contents = process_asm(contents) with open(output, "w") as f: From 6f21075d9cc3e6664152851f4a13ae240847b3bd Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Wed, 22 Jan 2025 15:24:22 +0100 Subject: [PATCH 23/28] GitHub Actions: build-and-test on an ARM processor (#1911) [Standard GitHub-hosted runners for public repositories](https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories) --> `ubuntu-22.04-arm`, `ubuntu-24.04-arm` --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index d05300db06..c32e799db9 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -17,7 +17,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-22.04, ubuntu-20.04, macos-latest] + os: [ubuntu-22.04, ubuntu-20.04, ubuntu-22.04-arm, macos-latest] build_type: ['Release', 'Debug'] compiler: ['g++', 'clang++'] lib: ['shared', 'static'] From 3d027d7e3817ec265872434378306f1e92fda9bb Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Wed, 22 Jan 2025 17:43:07 +0100 Subject: [PATCH 24/28] ruff rule E501: Fix long lines in Python code (#1910) * ruff rule E501: Fix long lines in Python code * Add missing space --------- Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- .ycm_extra_conf.py | 8 ++--- bindings/python/google_benchmark/__init__.py | 15 +++++---- bindings/python/google_benchmark/example.py | 3 +- pyproject.toml | 1 - tools/compare.py | 35 ++++++++++++++++---- tools/gbench/report.py | 16 ++++++--- tools/gbench/util.py | 8 +++-- 7 files changed, 60 insertions(+), 26 deletions(-) diff --git a/.ycm_extra_conf.py b/.ycm_extra_conf.py index caf257f054..ffef1b4daf 100644 --- a/.ycm_extra_conf.py +++ b/.ycm_extra_conf.py @@ -83,10 +83,10 @@ def IsHeaderFile(filename): def GetCompilationInfoForFile(filename): - # The compilation_commands.json file generated by CMake does not have entries - # for header files. So we do our best by asking the db for flags for a - # corresponding source file, if any. If one exists, the flags for that file - # should be good enough. + # The compilation_commands.json file generated by CMake does not have + # entries for header files. So we do our best by asking the db for flags for + # a corresponding source file, if any. If one exists, the flags for that + # file should be good enough. if IsHeaderFile(filename): basename = os.path.splitext(filename)[0] for extension in SOURCE_EXTENSIONS: diff --git a/bindings/python/google_benchmark/__init__.py b/bindings/python/google_benchmark/__init__.py index 7006352669..3685928f21 100644 --- a/bindings/python/google_benchmark/__init__.py +++ b/bindings/python/google_benchmark/__init__.py @@ -60,7 +60,8 @@ class __OptionMaker: """ class Options: - """Pure data class to store options calls, along with the benchmarked function.""" + """Pure data class to store options calls, along with the benchmarked + function.""" def __init__(self, func): self.func = func @@ -83,8 +84,8 @@ def __builder_method(*args, **kwargs): def __decorator(func_or_options): options = self.make(func_or_options) options.builder_calls.append((builder_name, args, kwargs)) - # The decorator returns Options so it is not technically a decorator - # and needs a final call to @register + # The decorator returns Options so it is not technically a + # decorator and needs a final call to @register return options return __decorator @@ -93,8 +94,8 @@ def __decorator(func_or_options): # Alias for nicer API. -# We have to instantiate an object, even if stateless, to be able to use __getattr__ -# on option.range +# We have to instantiate an object, even if stateless, to be able to use +# __getattr__ on option.range option = __OptionMaker() @@ -104,8 +105,8 @@ def register(undefined=None, *, name=None): # Decorator is called without parenthesis so we return a decorator return lambda f: register(f, name=name) - # We have either the function to benchmark (simple case) or an instance of Options - # (@option._ case). + # We have either the function to benchmark (simple case) or an instance of + # Options (@option._ case). options = __OptionMaker.make(undefined) if name is None: diff --git a/bindings/python/google_benchmark/example.py b/bindings/python/google_benchmark/example.py index 5909c0fc0e..5635c41842 100644 --- a/bindings/python/google_benchmark/example.py +++ b/bindings/python/google_benchmark/example.py @@ -13,7 +13,8 @@ # limitations under the License. """Example of Python using C++ benchmark framework. -To run this example, you must first install the `google_benchmark` Python package. +To run this example, you must first install the `google_benchmark` Python +package. To install using `setup.py`, download and extract the `google_benchmark` source. In the extracted directory, execute: diff --git a/pyproject.toml b/pyproject.toml index 761473c204..4595b6dd11 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -70,7 +70,6 @@ target-version = "py311" # Enable pycodestyle (`E`, `W`), Pyflakes (`F`), and isort (`I`) codes by default. select = ["ASYNC", "B", "C4", "C90", "E", "F", "I", "PERF", "PIE", "PT018", "RUF", "SIM", "UP", "W"] ignore = [ - "E501", # line too long "PLW2901", # redefined-loop-name "UP031", # printf-string-formatting ] diff --git a/tools/compare.py b/tools/compare.py index 36cbe07569..1dd9de239f 100755 --- a/tools/compare.py +++ b/tools/compare.py @@ -85,7 +85,10 @@ def create_parser(): "-d", "--dump_to_json", dest="dump_to_json", - help="Additionally, dump benchmark comparison output to this file in JSON format.", + help=( + "Additionally, dump benchmark comparison output to this file in" + " JSON format." + ), ) utest = parser.add_argument_group() @@ -94,7 +97,16 @@ def create_parser(): dest="utest", default=True, action="store_false", - help=f"The tool can do a two-tailed Mann-Whitney U test with the null hypothesis that it is equally likely that a randomly selected value from one sample will be less than or greater than a randomly selected value from a second sample.\nWARNING: requires **LARGE** (no less than {report.UTEST_OPTIMAL_REPETITIONS}) number of repetitions to be meaningful!\nThe test is being done by default, if at least {report.UTEST_MIN_REPETITIONS} repetitions were done.\nThis option can disable the U Test.", + help=( + "The tool can do a two-tailed Mann-Whitney U test with the null" + " hypothesis that it is equally likely that a randomly selected" + " value from one sample will be less than or greater than a" + " randomly selected value from a second sample.\nWARNING: requires" + f" **LARGE** (no less than {report.UTEST_OPTIMAL_REPETITIONS})" + " number of repetitions to be meaningful!\nThe test is being done" + f" by default, if at least {report.UTEST_MIN_REPETITIONS}" + " repetitions were done.\nThis option can disable the U Test." + ), ) alpha_default = 0.05 utest.add_argument( @@ -103,7 +115,9 @@ def create_parser(): default=alpha_default, type=float, help=( - "significance level alpha. if the calculated p-value is below this value, then the result is said to be statistically significant and the null hypothesis is rejected.\n(default: %0.4f)" + "significance level alpha. if the calculated p-value is below this" + " value, then the result is said to be statistically significant" + " and the null hypothesis is rejected.\n(default: %0.4f)" ) % alpha_default, ) @@ -114,7 +128,10 @@ def create_parser(): parser_a = subparsers.add_parser( "benchmarks", - help="The most simple use-case, compare all the output of these two benchmarks", + help=( + "The most simple use-case, compare all the output of these two" + " benchmarks" + ), ) baseline = parser_a.add_argument_group("baseline", "The benchmark baseline") baseline.add_argument( @@ -178,7 +195,10 @@ def create_parser(): parser_c = subparsers.add_parser( "benchmarksfiltered", - help="Compare filter one of first benchmark with filter two of the second benchmark", + help=( + "Compare filter one of first benchmark with filter two of the" + " second benchmark" + ), ) baseline = parser_c.add_argument_group("baseline", "The benchmark baseline") baseline.add_argument( @@ -203,7 +223,10 @@ def create_parser(): metavar="test_contender", type=argparse.FileType("r"), nargs=1, - help="The second benchmark executable or JSON output file, that will be compared against the baseline", + help=( + "The second benchmark executable or JSON output file, that will be" + " compared against the baseline" + ), ) contender.add_argument( "filter_contender", diff --git a/tools/gbench/report.py b/tools/gbench/report.py index 6b58918bfc..e143e45a71 100644 --- a/tools/gbench/report.py +++ b/tools/gbench/report.py @@ -249,7 +249,10 @@ def get_utest_color(pval): # We still got some results to show but issue a warning about it. if not utest["have_optimal_repetitions"]: dsc_color = BC_WARNING - dsc += f". WARNING: Results unreliable! {UTEST_OPTIMAL_REPETITIONS}+ repetitions recommended." + dsc += ( + f". WARNING: Results unreliable! {UTEST_OPTIMAL_REPETITIONS}+" + " repetitions recommended." + ) special_str = "{}{:<{}s}{endc}{}{:16.4f}{endc}{}{:16.4f}{endc}{} {}" @@ -397,12 +400,17 @@ def get_color(res): first_col_width = find_longest_name(json_diff_report) first_col_width = max(first_col_width, len("Benchmark")) first_col_width += len(UTEST_COL_NAME) - first_line = "{:<{}s}Time CPU Time Old Time New CPU Old CPU New".format( - "Benchmark", 12 + first_col_width + fmt_str = ( + "{:<{}s}Time CPU Time Old Time New CPU Old" + " CPU New" ) + first_line = fmt_str.format("Benchmark", 12 + first_col_width) output_strs = [first_line, "-" * len(first_line)] - fmt_str = "{}{:<{}s}{endc}{}{:+16.4f}{endc}{}{:+16.4f}{endc}{:14.0f}{:14.0f}{endc}{:14.0f}{:14.0f}" + fmt_str = ( + "{}{:<{}s}{endc}{}{:+16.4f}{endc}{}{:+16.4f}{endc}{:14.0f}{:14.0f}" + "{endc}{:14.0f}{:14.0f}" + ) for benchmark in json_diff_report: # *If* we were asked to only include aggregates, # and if it is non-aggregate, then don't print it. diff --git a/tools/gbench/util.py b/tools/gbench/util.py index 596b51a07c..2e91006be4 100644 --- a/tools/gbench/util.py +++ b/tools/gbench/util.py @@ -1,4 +1,6 @@ -"""util.py - General utilities for running, loading, and processing benchmarks""" +"""util.py - General utilities for running, loading, and processing +benchmarks +""" import json import os @@ -141,8 +143,8 @@ def benchmark_wanted(benchmark): json_schema_version = results["context"]["json_schema_version"] if json_schema_version != 1: print( - "In %s, got unnsupported JSON schema version: %i, expected 1" - % (fname, json_schema_version) + f"In {fname}, got unnsupported JSON schema version:" + f" {json_schema_version}, expected 1" ) sys.exit(1) if "benchmarks" in results: From 049f6e79cc3e8636cec21bbd94ed185b4a5f2653 Mon Sep 17 00:00:00 2001 From: xdje42 Date: Wed, 29 Jan 2025 01:53:56 -0800 Subject: [PATCH 25/28] [BUG] Run external profiler (ProfilerManager) same number of iterations #1913 (#1914) Run the external profiler the same number of iterations as the benchmark was run normally. This makes, for example, a trace collected via ProfilerManager consistent with collected PMU data. --- src/benchmark_runner.cc | 9 ++-- src/benchmark_runner.h | 2 +- test/CMakeLists.txt | 3 ++ test/profiler_manager_iterations_test.cc | 61 ++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 test/profiler_manager_iterations_test.cc diff --git a/src/benchmark_runner.cc b/src/benchmark_runner.cc index 463f69fc52..3e8aea7376 100644 --- a/src/benchmark_runner.cc +++ b/src/benchmark_runner.cc @@ -438,9 +438,7 @@ MemoryManager::Result* BenchmarkRunner::RunMemoryManager( return memory_result; } -void BenchmarkRunner::RunProfilerManager() { - // TODO: Provide a way to specify the number of iterations. - IterationCount profile_iterations = 1; +void BenchmarkRunner::RunProfilerManager(IterationCount profile_iterations) { std::unique_ptr manager; manager.reset(new internal::ThreadManager(1)); b.Setup(); @@ -507,7 +505,10 @@ void BenchmarkRunner::DoOneRepetition() { } if (profiler_manager != nullptr) { - RunProfilerManager(); + // We want to externally profile the benchmark for the same number of + // iterations because, for example, if we're tracing the benchmark then we + // want trace data to reasonably match PMU data. + RunProfilerManager(iters); } // Ok, now actually report. diff --git a/src/benchmark_runner.h b/src/benchmark_runner.h index 6e5ceb31e0..332bbb51ec 100644 --- a/src/benchmark_runner.h +++ b/src/benchmark_runner.h @@ -109,7 +109,7 @@ class BenchmarkRunner { MemoryManager::Result* RunMemoryManager(IterationCount memory_iterations); - void RunProfilerManager(); + void RunProfilerManager(IterationCount profile_iterations); IterationCount PredictNumItersNeeded(const IterationResults& i) const; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 321e24d94b..3e2c651830 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -195,6 +195,9 @@ benchmark_add_test(NAME memory_manager_test COMMAND memory_manager_test --benchm compile_output_test(profiler_manager_test) benchmark_add_test(NAME profiler_manager_test COMMAND profiler_manager_test --benchmark_min_time=0.01s) +compile_benchmark_test(profiler_manager_iterations_test) +benchmark_add_test(NAME profiler_manager_iterations COMMAND profiler_manager_iterations_test) + # MSVC does not allow to set the language standard to C++98/03. if(NOT (MSVC OR CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC")) compile_benchmark_test(cxx03_test) diff --git a/test/profiler_manager_iterations_test.cc b/test/profiler_manager_iterations_test.cc new file mode 100644 index 0000000000..e727929ddb --- /dev/null +++ b/test/profiler_manager_iterations_test.cc @@ -0,0 +1,61 @@ +#include +#include +#include +#include + +#include "benchmark/benchmark.h" + +// Tests that we can specify the number of profiler iterations with +// --benchmark_min_time=x. +namespace { + +int iteration_count = 0; +int end_profiler_iteration_count = 0; + +class TestProfilerManager : public benchmark::ProfilerManager { + void AfterSetupStart() override { iteration_count = 0; } + void BeforeTeardownStop() override { + end_profiler_iteration_count = iteration_count; + } +}; + +class NullReporter : public benchmark::BenchmarkReporter { + public: + bool ReportContext(const Context& /*context*/) override { return true; } + void ReportRuns(const std::vector& /* report */) override {} +}; + +} // end namespace + +static void BM_MyBench(benchmark::State& state) { + for (auto s : state) { + ++iteration_count; + } +} +BENCHMARK(BM_MyBench); + +int main(int argc, char** argv) { + // Make a fake argv and append the new --benchmark_profiler_iterations= + // to it. + int fake_argc = argc + 1; + const char** fake_argv = new const char*[static_cast(fake_argc)]; + for (int i = 0; i < argc; ++i) fake_argv[i] = argv[i]; + fake_argv[argc] = "--benchmark_min_time=4x"; + + std::unique_ptr pm(new TestProfilerManager()); + benchmark::RegisterProfilerManager(pm.get()); + + benchmark::Initialize(&fake_argc, const_cast(fake_argv)); + + NullReporter null_reporter; + const size_t returned_count = + benchmark::RunSpecifiedBenchmarks(&null_reporter, "BM_MyBench"); + assert(returned_count == 1); + + // Check the executed iters. + assert(end_profiler_iteration_count == 4); + + benchmark::RegisterProfilerManager(nullptr); + delete[] fake_argv; + return 0; +} From 4642758438659e01fed407b28062d88b70d31331 Mon Sep 17 00:00:00 2001 From: Dominic Hamon Date: Thu, 30 Jan 2025 09:52:07 +0000 Subject: [PATCH 26/28] fix some clang-tidy issues --- src/benchmark_runner.h | 1 - test/profiler_manager_test.cc | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/benchmark_runner.h b/src/benchmark_runner.h index 332bbb51ec..20a37c1ae3 100644 --- a/src/benchmark_runner.h +++ b/src/benchmark_runner.h @@ -19,7 +19,6 @@ #include #include "benchmark_api_internal.h" -#include "internal_macros.h" #include "perf_counters.h" #include "thread_manager.h" diff --git a/test/profiler_manager_test.cc b/test/profiler_manager_test.cc index 3b08a60d1d..21f2f1dc03 100644 --- a/test/profiler_manager_test.cc +++ b/test/profiler_manager_test.cc @@ -1,5 +1,6 @@ // FIXME: WIP +#include #include #include "benchmark/benchmark.h" From 4a805f9f0f468bd4d499d060a1a1c6bd5d6b6b73 Mon Sep 17 00:00:00 2001 From: Dominic Hamon Date: Thu, 30 Jan 2025 10:00:04 +0000 Subject: [PATCH 27/28] clang-tidy warning --- src/string_util.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/string_util.h b/src/string_util.h index 731aa2c04c..f1e50be4f4 100644 --- a/src/string_util.h +++ b/src/string_util.h @@ -9,7 +9,6 @@ #include "benchmark/benchmark.h" #include "benchmark/export.h" #include "check.h" -#include "internal_macros.h" namespace benchmark { From c35af58b61daa111c93924e0e7b65022871fadac Mon Sep 17 00:00:00 2001 From: Brad Smith Date: Tue, 4 Feb 2025 05:32:41 -0500 Subject: [PATCH 28/28] Update error message now that /proc/cpuinfo is no longer in use (#1917) c24774dc4f4402c3ad150363321cc972ed2669e7 removed using /proc/cpuinfo so no longer mention it in the error message. --- src/sysinfo.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sysinfo.cc b/src/sysinfo.cc index eddd430e68..2787e87c8e 100644 --- a/src/sysinfo.cc +++ b/src/sysinfo.cc @@ -515,8 +515,7 @@ int GetNumCPUsImpl() { int GetNumCPUs() { int num_cpus = GetNumCPUsImpl(); if (num_cpus < 1) { - std::cerr << "Unable to extract number of CPUs. If your platform uses " - "/proc/cpuinfo, custom support may need to be added.\n"; + std::cerr << "Unable to extract number of CPUs.\n"; /* There is at least one CPU which we run on. */ num_cpus = 1; }