* Address warnings on NVIDIA nvc++
Types of warnings were being generated:
1. Deprecated warnings - solved by defining the relevant BENCHMARK_*
macros for nvc++ and adding pragma suppress on a couple of .cc files
2. Setup/TearDown const vs non-const partial override - solved by
adding non-const version
3. Static but not referenced - added diagnostic suppress for that file
* Modified manually to comply with CD/CI
* Revert partial override
* Suppress warnings from tests if compiler is NVHPC
---------
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
* [FR] Provide public accessors to benchmark name and arguments #1551
* Update AUTHORS and CONTRIBUTORS
* Update benchmark_register.cc
* Fix lint formatting
Build breaks when -Werror is turned on because of unhandled cases of
inocuous/pedantic warnings. Adopted the same solution as for Intel PGI
compiler - just disable -Werror manually, unless BENCHMARK_FORCE_WERROR
is enabled. Fixes#1556.
* Update AUTHORS/CONTRIBUTORS
* Fix examples with deprecated DoNotOptimize API
The const-reference API to DoNotOptimize was deprecated with #1493. Some
examples in the user guide are using exactly that deprecated interface.
This fixes that by passing non-const lvalues instead. Fixes#1566
Bumps nanobind to v0.2.0, the latest stable version to include all
features needed to create the GBM bindings. Deprecated names in v0.2.0
were migrated to their new counterparts.
Linkopts for macOS were changed to mirror the "endorsed" linker options
used in nanobind's CMake config, which were changed since the last
commit.
* Refactoring of PerfCounters infrastructure
The main feature in this pull request is the removal of the static
sharing of PerfCounters and instead creating them at the top
`RunBenchmarks()` function where all benchmark runners are created. A
single PerfCountersMeasurement object is created and then shared with
all the new BenchmarkRunners objects, one per existing benchmark.
Other features conflated here in this PR are:
- Added BENCHMARK_DONT_OPTIMIZE macro in global scope
- Removal of the `IsValid()` query, being replaced by checking the
number of remaining counters after validity tests
- Refactoring of all GTests to reflect the changes and new semantics
- extra comments throughout the new code to clarify intent
It was extremely hard to separate all those features in different PRs
as requested since they are so interdependent on each other so I'm just
pushing them altogether and asking for forgiveness.
This PR comes replacing PRs 1555 and 1558 which have been closed.
* Fixed whitespace issue with clang-format
My clang-format insists in deleting this single white space on line 601
while Github's clang format breaks when it is added. I had to disable
format-on-save to check-in this revert change.
I'm using clang 14.0.6.
* Filter performance counter names, not invalidate all
Currently, the performance counters are validated while they
are being created and one failure returns NoCounters(), ie it
effecitvely invalidates all the counters.
I would like to propose a new behavior: filter instead. If an
invalid name is added to the counter list, or if that particular
counter is not supported on this platform, that counter is dropped
from the list and an error messages is created, while all the
other counters remain active.
This will give testers a peace of mind that if one mistake is made
or if something is changed or removed from libpfm, their entire
test will not be invalidated. This feature gives more tolerance
with respect to versioning.
Another positive is that testers can now input a superset of all
desired counters for all platforms they support and just let
Benchmark drop all those that are not supported, although it will
create quite a lot of noise down the line, in which case perhaps
we should drop silently or make a consolidated, single error line
but this was not implemented in this change set.
* Removed unused helper type.
* Implement unlimited number of performance counters
Linux performance counters will limit the number of hardware
counters per reading group. For that reason the implementation of
PerfCounters is limited to 3. However if only software counters
are added, there is no reason to limit the counters. For hardware
counters, we create multiple groups and store a vector or leaders
in the PerfCounters object. When reading, there is an extra time
waste by iterating through all the group leaders. However this
should be the same performance as with today. Reading is done by
groups and it had to be heavily adjusted with the logic being
moved to PerfCounterValues. I created a test for x86-64 and took
care of filtering out the events in case it runs in a platform
that does not support those counters - the test will not fail. The
current tests were already failing (ReOpenExistingCounters,
CreateExistingMeasurements and MultiThreaded) on the main branch
and they continue to fail after this implementation - I did not
fix those not to conflate all here.
* Moved the PerfCounterValues::Read() implementation from header to body.
* Added missing implementation of PerfCounters::IsCounterSupported when HAVE_LIBPFM is not defined.
* Changed comments to reflect the implementation
* Removed arg name so it does not generate an error when HAVE_LIBPBM is not defined.
* Made loop counter a const reference for clang-tidy
* Added missig BENCHMARK_EXPORT to PerfCounterValues
* add compiler to build-and-test and create min-cmake CI bot
* fix CXX env var
* downgrade msvc generator for cmake-3.10
* assume windows users have the latest cmake
* Fix CPU frequency estimation on riscv
* Cleanup code for CPU frequency estimation
* Fix use before definition of the macro
* Move the platform definitions back
* Fix compilation error on windows
* Remove unused sleep.h and sleep.cc
* cleanup: support CMake >= 3.10
This aligns the project with the CMake support policies in:
https://opensource.google/documentation/policies/cplusplus-support
I also simplied the management of CMake policies. Most of the overriden
policies (anything <= CMP0067) are enabled by default when you require
CMake >= 3.10. But it is easier to just declare that you will accept
newer policies when they are available using the `...3.22` notation.
* Address review comments
* inlined links
---------
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
The previous code was triggering a warning in Debug builds where NDEBUG
is not defined and BM_CHECK() is included:
benchmark/src/benchmark_runner.cc: In function ‘benchmark::internal::BenchTimeType benchmark::internal::ParseBenchMinTime(const std::string&)’:
benchmark/src/benchmark_runner.cc:212:24: error: suggest parentheses around ‘&&’ within ‘||’ [-Werror=parentheses]
212 | (has_suffix && *p_end == 's' || *p_end == '\0'))
| ~~~~~~~~~~~^~~~~~~~~~~~~~~~
benchmark/src/check.h:82:4: note: in definition of macro ‘BM_CHECK’
82 | (b ? ::benchmark::internal::GetNullLogInstance() \
| ^
Add parenthesis around the && expression.
Also fix a spelling error and move the comma in the preceding comment to
improve clarity.
Tested:
- cmake -E make_directory build
- cmake -E chdir "build" cmake -DBENCHMARK_DOWNLOAD_DEPENDENCIES=on -DCMAKE_BUILD_TYPE=Debug ../
- cmake --build "build" --config Debug
- cmake -E chdir "build" ctest --build-config Debug
* Allow specifying number of iterations via --benchmark_min_time.
Make the flag accept two new suffixes:
+ <integer>x: number of iterations
+ <floag>s: minimum number of seconds.
This matches the internal benchmark API.
* forgot to change flag type to string
* used tagged union instead of std::variant, which is not available pre C++14
* update decl in benchmark_runner.h too
* fixed errors
* refactor
* backward compat
* typo
* use IterationCount type
* fixed test
* const_cast
* ret type
* remove extra _
* debug
* fixed bug from reporting that caused the new configs not to be included in the final report
* addressed review comments
* restore unnecessary changes in test/BUILD
* fix float comparisons warnings from Release builds
* clang format
* fix visibility warning
* remove misc file
* removed backup files
* addressed review comments
* fix shorten in warning
* use suffix for existing min_time specs to silent warnings in tests
* fix leaks
* use default min-time value in flag decl for consistency
* removed double kMinTimeDecl from benchmark.h
* dont need to preserve errno
* add death tests
* Add BENCHMARK_EXPORT to hopefully fix missing def errors
* only enable death tests in debug mode because bm_check is no-op in release mode
* guard death tests with additional support-check macros
* Add additional guard to prevent running in Release mode
---------
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
Previously compare.py ignored the --benchmarks_filter
argument when loading JSON. This defeated any workflow when
a single run of the benchmark was run, followed by multiple
"subset reports" run against it with the 'benchmarks'
command.
Concretely this came up with the simple case:
compare.py benchmarks a.json b.json --benchmarks_filter=BM_Example
This has no practical impact on the 'filters' and
'benchmarkfiltered' comand, which do their thing at a later
stage.
Fixes#1484
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
* fix#1446 by removing the address operator
* add test
* format
---------
Co-authored-by: Thomas <thomas.maierbacher@rohde-schwarz.com>
Co-authored-by: Dominic Hamon <dominichamon@users.noreply.github.com>
The compiler assume that a constant reference, even though escaped via asm
volatile, is unchanged. The const-ref interface is deprecated to discourage
new uses of it, as subtle compiler optimizations (invariant hoisting, etc.) can
occur.
Within microbenchmarks for Abseil's hashtables, BM_FindMiss_Hot
(c0eaa90671/fleetbench/swissmap/hot_swissmap_benchmark.cc (L48))
has a `const uint32_t key` is passed to to the lookup of a hashtable.
With the `key` marked `const`, LLVM hoists part of the lookup
calculation outside of the loop.
With the `const` removed, this hoisting does not occur.
Co-authored-by: Dominic Hamon <dominichamon@users.noreply.github.com>
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
* End support for Python 3.7, update cibuildwheel and publish actions
Removes Python 3.7 from the support matrix, since it does not support
PEP590 vectorcalls.
Bumps the `cibuildwheel` and `pypa-publish` actions to their latest
available versions respectively.
* Add nanobind to the Bazel dependencies, add a BUILD file
The build file builds nanobind as a static `cc_library`. Currently,
the git SHA points to HEAD, since some necessary features have not
been included in a release yet.
* Delete pybind11 BUILD file
* Switch bindings implementation to nanobind
Switches over the binding tool to `nanobind` from `pybind11`. Most
changes in the build setup itself were drop-in replacements of existing
code changed to nanobind names, no new concepts needed to be
implemented.
Sets the minimum required macOS to 10.14 for full C++17 support. Also,
to avoid ambiguities in Bazel, build for macOS 11 on Mac ARM64.
* Use Bazel select for linker options
Guards against unknown linker option errors by selecting required
linker options for nanobind only on macOS, where they are relevant.
Other changes:
* Bump cibuildwheel action to v2.12.0
* Bump Bazel for aarch64 linux wheels to 6.0.0
* Remove C++17 flag from build files since it is present in setup.py `bazel build` command
* Bump nanobind commit to current HEAD (TBD: Bump to next stable release)
* Unbreak Windows builds of nanobind-based bindings
Guards compiler options behind a new `select` macro choosing between
MSVC and not MSVC.
Other changes:
* Inject the proper C++17 standard cxxopt in the `setup.py` build
command.
* Bump nanobind to current HEAD.
* Make `macos` a benchmark-wide condition, with public visibility to
allow its use in the nanobind BUILD file.
* Fall back to `nb::implicitly_convertible` for Counter construction
Since `benchmark::Counter` only has a constructor for `double`,
the nanobind `nb::init_implicit` template cannot be used. Therefore,
to support implicit construction from ints, we fall back to the
`nb::implicitly_convertible` template instead.
distutils is deprecated and will be removed in Python 3.12, so this
commit modernizes the Python bindings `setup.py` file in order to
future-proof the code.
On top of this, type hints were added for all of the convenience
functions to make static type checking adoption easier in the future,
if desired.
A context manager was added to temporarily write the Python include
path to the Bazel WORKSPACE file - but unlike previously, the
WORKSPACE file is reverted to its previous state after the build to not
produce changes on every rebuild.
Lastly, the Python bindings test matrix was extended to all major
platforms to create a more complete picture of the current state of
the bindings, especially with regards to upcoming wheel builds.
* Include the benchmark's family-name in State
For compat with internal library, where State::name() returns the benchmark's family name.
* added missing files from prev commit
* fix field-init order error
* added test
* Bug fix variable 'actual_iterations' set but not used
Compiling the project in clang 15 without -Wno-unused-but-set-variable flag the following error is generated:
benchmark-src/test/options_test.cc:70:10: error: variable 'actual_iterations' set but not used [-Werror,-Wunused-but-set-variable]
size_t actual_iterations = 0;
^
* Adjust according formatting of `clang-format`
Co-authored-by: dominic hamon <510002+dmah42@users.noreply.github.com>
