We used assert() a lot in tests and that can cause build breakages in some of the opt builds (since assert() are removed)
it's not practical to sprinkle "(void)" everywhere so I think setting this warning option is the best option for now.
* Make json and csv output consistent.
Currently, the --benchmark_format=csv option does not output the correct value for the cv statistics. Also, the json output should not contain a time unit for the cv statistics.
* fix formatting
* undo json change
---------
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
There are three major compilers on Windows targeting the MSVC ABI (i.e.
linking with microsofts STL etc.):
- `MSVC`
- `clang-cl` aka clang with the MSVC compatible CLI
- `clang++` aka clang with gcc compatible CLI
The cmake variable `MSVC` is only set for the first two as it defined in
terms of the CLI interface provided:
> Set to true when the compiler is some version of Microsoft Visual
> C++ or another compiler simulating the Visual C++ cl command-line syntax.
(from cmake docs)
For many of the tests in the library its the ABI that matters not the
cmdline, so check `CMAKE_CXX_SIMULATE_ID` too, if it is `MSVC` the
current compiler is targeting the MSVC ABI. This handles `clang++`
* test: Use gtest_main only when needed
There are two types of tests. `*_gtest.cc` files use `gtest` and
`gtest_main`. `*_test.cc` files define their own main.
Only depend on `gtest`/`gtest_main` when needed. This is similar
to what `CMakeLists.txt` does.
* comment-only: gunit => gtest
* Fix typo
* State: Initialize counters with kAvgIteration in constructor
Previously, `counters` was updated in `PauseTiming()` with
`counters[name] += Counter(measurement, kAvgIteration)`.
The first `counters[name]` call inserts a counter with no flags.
There is no `operator+=` for `Counter`, so the insertion is done
by converting the `Counter` to a `double`, then constructing a
`Counter` to insert from the `double`, which drops the flags.
Pre-insert the `Counter` with the correct flags, then only
update `Counter::value`.
Introduced in 1c64a36 ([perf-counters] Fix pause/resume (#1643)).
* perf_counters_test.cc: Don't divide by iterations
Perf counters are now divided by iterations, so dividing again
in the test is wrong.
* State: Fix shadowed param error
* benchmark.cc: Fix clang-tidy error
---------
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
* perf_counters_gtest: Make test pass on Android
Tested on Pixel 3 and Pixel 6. Reduce test to the intersection of
what passes on all platforms.
Pixel 6 doesn't support BRANCHES, and only supports two perf
counters.
---------
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
* [perf-counters] Fix pause/resume
Using `state.PauseTiming() / state.ResumeTiming()` was broken.
Thanks [@virajbshah] for the the repro testcase.
* ran clang-format over the whole perf_counters_test.cc
* Remove check that perf counters are 0 on `Pause`, since `Pause`/`Resume`
sequences would cause a non-0 counter value
* both upper and lower bound for the with/without resume counters
---------
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
* Add pfm CI actions for bazel
* Fix problems in unit test.
* Undo enabling the CI tests for pfm - github CI machines seemingly do not support performance counters.
* Remove commented code - can be revisited in github history when needed, and there's a comment explaining the rationale behind the new test code.
---------
Co-authored-by: Andy Christiansen <achristiansen@google.com>
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
* 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>
* 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
* 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>
* 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>
* 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>
Some linux distributions no longer provide `python` binary and require
usage of `python3` instead. This changes the scripts here and uses
cmake `find_package(Python3` when running python.
Co-authored-by: Dominic Hamon <dominichamon@users.noreply.github.com>
This patch fixes compilation on Solaris, addressing the problems reported
in Issue #1499:
* Provide `HOST_NAME_MAX` definition.
* Match `sysconf(3C)` return type.
* Avoid `-Wcast-qual` warnings with `libkstat(3KSTAT)` functions.
* Avoid clash with `<floatingpoint.h>` `single` typedef.
* Ensure we don't need benchmark installed to pass c++ feature checks
Requires removal of some dependencies on benchmark.h from internal
low-level headers, which is a good thing.
Also added better logging to the feature check cmake module.
* Explicitly cast int literals to int8_t in tests so silence implicit-conversion warnings
Error came from:
```
: error: implicit conversion loses integer precision: 'const int' to 'const signed char' [-Werror,-Wimplicit-int-conversion]
```
* clang format
* undo deleted line
* Fixed build issues on window
- Added missing dlimport/export attributes in function definitions. (They are needed in both decls and defs)
- Removed dlimport/dlexprt attribute in private field. (global_context is not exported anywhere).
* fixed incorrect include path
* undo changes w.r.t HelperPrintf
* removed forward decl of private variable - instead, introduce a getter and use it.
* Removed forward decl from benchmark_gtest too
Co-authored-by: Dominic Hamon <dominichamon@users.noreply.github.com>
* Stop generating the export header and just check it in
* format the new header
* support windows
* format the header again
* avoid depending on internal macro
* ensure we define the right thing for windows static builds
* support older cmake
* and for tests
Non-const DoNotOptimize() can't compile when used with some types.
Example of code which can't compile:
char buffer3[3] = "";
benchmark::DoNotOptimize(buffer3);
Error message:
error: impossible constraint in 'asm'
asm volatile("" : "+r"(value) : : "memory");
Introduced in 8545dfb (Fix DoNotOptimize() GCC copy overhead (#1340) (#1410))
The cause is compiler can't work with the +r constraint for types that can't
be placed perfectly in registers. For example, char array[3] can't be perfectly
fit in register on x86_64 so it requires placed in memory but constraint
doesn't allow that.
Solution
- Use +m,r constraint for the small objects so the compiler can decide to use
register or/and memory
- For the big objects +m constraint is used which allows avoiding extra copy
bug(see #1340)
- The same approach is used for the const version of DoNotOptimize()
although the const version works fine with the "r" constraint only.
Using mixed r,m constraint looks more general solution.
See
- Issue #1340 ([BUG] DoNotOptimize() adds overhead with extra copy of argument(gcc))
- Pull request #1410 (Fix DoNotOptimize() GCC copy overhead (#1340) #1410)
- Commit 8545dfb (Fix DoNotOptimize() GCC copy overhead (#1340) (#1410))
* Fix DoNotOptimize() GCC copy overhead (#1340)
The issue is that GCC DoNotOptimize() does a full copy of an argument
if it's not a pointer and it slows down a benchmark. If an argument is big
enough there is a memcpy() call for copying the argument. An argument
object can be a big object so DoNotOptimize() could add sufficient
overhead and affects benchmark results.
The cause is in GCC behavior with asm volatile constraints. Looks like GCC
trying to use r(register) constraint for all cases despite object size.
See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105519
The solution is the split DoNotOptimize() in two cases - value fits
in register and value doesn't fit in register. And use case specific
asm constraint. std::is_trivially_copyable trait is needed because
"+r" constraint doesn't work with non trivial copyable objects.
- Fix requires support C++11 feature std::is_trivially_copyable from GCC
compiler. The feature has been supported since GCC 5
- Fallback for GCC version < 5 still exists but it uses "m" constraint
which means a little bit more overhead in some cases
- Add assembly tests for issued cases
Fixes#1340
* Add supported compiler versions info for assembly tests
- Assembly tests are inherently non-portable. So explicitly add GCC
and Clang versions required for reliable tests passed
- Write a warning message if the current compiler version isn't supported
hamptonm1