* Introduce Coefficient of variation aggregate
I believe, it is much more useful / use to understand,
because it is already normalized by the mean,
so it is not affected by the duration of the benchmark,
unlike the standard deviation.
Example of real-world output:
```
raw.pixls.us-unique/GoPro/HERO6 Black$ ~/rawspeed/build-old/src/utilities/rsbench/rsbench GOPR9172.GPR --benchmark_repetitions=27 --benchmark_display_aggregates_only=true --benchmark_counters_tabular=true
2021-09-03T18:05:56+03:00
Running /home/lebedevri/rawspeed/build-old/src/utilities/rsbench/rsbench
Run on (32 X 3596.16 MHz CPU s)
CPU Caches:
L1 Data 32 KiB (x16)
L1 Instruction 32 KiB (x16)
L2 Unified 512 KiB (x16)
L3 Unified 32768 KiB (x2)
Load Average: 7.00, 2.99, 1.85
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations CPUTime,s CPUTime/WallTime Pixels Pixels/CPUTime Pixels/WallTime Raws/CPUTime Raws/WallTime WallTime,s
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
GOPR9172.GPR/threads:32/process_time/real_time_mean 11.1 ms 353 ms 27 0.353122 31.9473 12M 33.9879M 1085.84M 2.83232 90.4864 0.0110535
GOPR9172.GPR/threads:32/process_time/real_time_median 11.0 ms 352 ms 27 0.351696 31.9599 12M 34.1203M 1090.11M 2.84336 90.8425 0.0110081
GOPR9172.GPR/threads:32/process_time/real_time_stddev 0.159 ms 4.60 ms 27 4.59539m 0.0462064 0 426.371k 14.9631M 0.0355309 1.24692 158.944u
GOPR9172.GPR/threads:32/process_time/real_time_cv 1.44 % 1.30 % 27 0.0130136 1.44633m 0 0.0125448 0.0137802 0.0125448 0.0137802 0.0143795
```
Fixes https://github.com/google/benchmark/issues/1146
* Be consistent, it's CV, not 'rel std dev'
* Statistics: add support for percentage unit in addition to time
I think, `stddev` statistic is useful, but confusing.
What does it mean if `stddev` of `1ms` is reported?
Is that good or bad? If the `median` is `1s`,
then that means that the measurements are pretty noise-less.
And what about `stddev` of `100ms` is reported?
If the `median` is `1s` - awful, if the `median` is `10s` - good.
And hurray, there is just the statistic that we need:
https://en.wikipedia.org/wiki/Coefficient_of_variation
But, naturally, that produces a value in percents,
but the statistics are currently hardcoded to produce time.
So this refactors thinkgs a bit, and allows a percentage unit for statistics.
I'm not sure whether or not `benchmark` would be okay
with adding this `RSD` statistic by default,
but regales, that is a separate patch.
Refs. https://github.com/google/benchmark/issues/1146
* Address review notes
The test used to work with scipy 1.6, but does not work with 1.7.1
I believe this is because of https://github.com/scipy/scipy/pull/4933
Since there doesn't appear that there is anything wrong with how
we call the `mannwhitneyu()`, i guess we should just expect the new values.
Notably, the tests now fail with earlier scipy versions.
This slightly prettifies the CMake's `feature_summary()` output,
should the library be built as part of some project that then prints
the CMake summary.
Refactoring in 201b981a moved most of the documentation from `README.md` to `docs/user_guide.md`. Some links from `README.md` to other `docs/*.md` files ended up unchanged in `docs/user_guide.md`. Those links were now broken as they did not link from outside the `docs` directory anymore, but from inside it. Removing the leading `docs/` for these links fixes this.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate the direct access to these fields.
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate the direct access to these fields.
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate direct access to `.thread_index` and make threads a private field
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate direct access to `.thread_index` and make threads a private field
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate direct access to `.thread_index` and make threads a private field
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate direct access to `.thread_index` and make threads a private field
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate direct access to `.thread_index` and make threads a private field
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate direct access to `.thread_index` and make threads a private field
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate direct access to `.thread_index` and make threads a private field
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`
Also deprecate direct access to `.thread_index` and make threads a private field
Motivations:
Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)
I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
Both `.` and `<empty>` already means "run all benchmarks" here, as commented on this flag's declaration (and below around line 448-449).
So this is a NFC.
On the other hand, this help internally because internally, if the flag is empty (or if it's not a specified by a binary), we don't call the RunSpecifiedBenchmarks.
There is still a difference in what <empty> means internally (runs no benchmarks) and externally (runs all benchmarks).
But we can work around this.
* Remove `min` with dead path.
When `isSignificant` is false, the smallest value `multiplier` can
approach is 14. Thus, min(10, multiplier) will always return 10.
Addresses part one of #1205.
* Remove always false condition.
1. `multiplier <= 1.0` implies `i.seconds >= min_time * 1.4`
2. By (1), `isSignficant` is true because `i.seconds > min_time * 1.4` implies `i.seconds > min_time` implies that `i.seconds / minTime > 1 > 0.1`. Thus, the ternary maintains the same multiplier value.
3. `ShouldReportResults` is always called before `PredictNumItersNeeded`, if `i.seconds >= min_time` then the loop is broken and `PredictNumItersNeeded` is never called.
4. 1 and 3 together imply that `multiplier <= 1.0` is never true.
Addresses part 2 of #1205.
* add g++ to sanitizer buildbots
* add compiler to sanitizer build name
* spell g++ correctly. look, it's early, ok?
* only set libcxx if we're using clang
This can be used together with ArgsProduct() to allow multiple ranges
with different multipliers and mixing dense and sparse ranges.
Example:
BENCHMARK(MyTest)->ArgsProduct({
CreateRange(0, 1024, /*multi=*/32),
CreateRange(0, 100, /*multi=*/4),
CreateDenseRange(0, 4, /*step=*/1)
});
Co-authored-by: Jen-yee Hong <pcmantw@google.com>
* Enable various sanitizer builds in github actions
* try with off the shelf versions
* nope
* specific version?
* rats
* oops
* remove msan for now
* reorder so env is set before building libc++
Inspired by the original implementation by Hai Huang @haih-g
from https://github.com/google/benchmark/pull/1105.
The original implementation had design deficiencies that
weren't really addressable without redesign, so it was reverted.
In essence, the original implementation consisted of two separateable parts:
* reducing the amount time each repetition is run for, and symmetrically increasing repetition count
* running the repetitions in random order
While it worked fine for the usual case, it broke down when user would specify repetitions
(it would completely ignore that request), or specified per-repetition min time (while it would
still adjust the repetition count, it would not adjust the per-repetition time,
leading to much greater run times)
Here, like i was originally suggesting in the original review, i'm separating the features,
and only dealing with a single one - running repetitions in random order.
Now that the runs/repetitions are no longer in-order, the tooling may wish to sort the output,
and indeed `compare.py` has been updated to do that: #1168.
Currently the lifetime of a single BenchmarkRunner is constrained
to a RunBenchmark(), but that will have to change for interleaved
benchmark execution, because we'll need to keep it around to not
forget how much repetitions of an instance we've done.
Currently, the tooling just keeps the whatever benchmark order
that was present, and this is fine nowadays, but once the benchmarks
will be optionally run interleaved, that will be rather suboptimal.
So, now that i have introduced family index and per-family instance index,
we can define an order for the benchmarks, and sort them accordingly.
There is a caveat with aggregates, we assume that they are in-order,
and hopefully we won't mess that order up..
While the current variant works, it assumes that all the instances of
a single family will be run together, with nothing inbetween them.
Naturally, that won't work once the runs may be interleaved.
Much like it makes sense to enumerate all the families,
it makes sense to enumerate stuff within families.
Alternatively, we could have a global instance index,
but i'm not sure why that would be better.
This will be useful when the benchmarks are run not in order,
for the tools to sort the results properly.
It may be useful for those wishing to further post-process JSON results,
but it is mainly geared towards better support for run interleaving,
where results from the same family may not be close-by in the JSON.
While we won't be able to do much about that for outputs,
the tools can and perhaps should reorder the results to that
at least in their output they are in proper order, not run order.
Note that this only counts the families that were filtered-in,
so if e.g. there were three families, and we filtered-out
the second one, the two families (which were first and third)
will have family indexes 0 and 1.
