Closed rfernandes closed 5 years ago
Seems to be crashing cppcheck ? I'll have a look...
Couple of notes for reviewers:
I've added a couple of alternative implementations for hex_digits and added dec_digits as well.
Suggest use of "FIlename.bt.cpp" similar to the Unit test files. The benchamrk naming convention "group/test" make it easy to regex subsets with the --filter option.
Example usages:
// No parameter functions:
TOOLBOX_BENCHMARK("benchmark_name", func);
// Single parameter functions, single value
TOOLBOX_BENCHMARK("group/benchmark_name", func_with_param).value(1000);
// Range
TOOLBOX_BENCHMARK("group/benchmark_name", func_with_char_param).range(10, 1000);
// Bespoke functions using lamdas
TOOLBOX_BENCHMARK("group/benchmark_name", [](){ return overloaded_function('x'); );
Example output:
bin/toolbox-bench --filter hex | column -t
Benchmark Average(ns) Runs Time(us)
util/hex_digits1 3 2215 3440528
util/hex_digits2 3 3220 3869597
Main points:
Future improvements:
Apparently this segfaults cppcheck with an infinite loop, while trying to resolve variable symbols:
using ArgType = typename Traits::template ArgType<0>;
It was fixed by renaming ArgType to another type. I'll follow up with a upstream bug report.
Minor request: please reference issue numbers in the last line of the commit.
Justification: we'd like to standardise the commit message format as per the contributing guide. (Extract follows.)
GitHub issue numbers should be referenced on the last line of the commit message. Keywords can be used to automatically close issues. For example:
The Subject Line
A more detailed description that explains what the change is,
why it is needed, and how it has been implemented.
Closes #123, #456
Project naming conventions:
The asm benchmark helper will probably screw up ARM32 no ?
- HDRHistogram output support, including full run statistics instead of just avg.
Yes, this is what I currently have downstream:
$ matchbox-micro-bench
NAME COUNT MIN %50 %95 %99 MAX
------------------------------------------------------------------------------------------------
clock_gettime_mono 39317982 22 26 26 31 49
clock_gettime_real 44615025 22 22 24 28 37
cycl_time_set 20879400 47 47 52 59 82
cycl_time_set_and_get 20777050 47 47 53 59 70
sys_clock_gettime_mono 2667810 369 370 400 465 664
sys_clock_gettime_real 2671890 368 369 414 465 664
Would be cool if we get this added to avoid a downstream functional regression.
Note also that there are other kinds of benchmark besides the microbenchmarks. Happy to stick with "Benchmark" as a name, but might also consider "Microbench" or similar as an alternative to disambiguate.
The asm benchmark helper will probably screw up ARM32 no ?
No, it should be fine, because there are no instructions as such. These are the equivalents I currently use downstream:
inline void clobber_memory() noexcept
{
asm volatile("" : : : "memory");
}
template <typename ValueT>
inline void do_not_optimise(const ValueT& val)
{
asm volatile("" : : "r,m"(val) : "memory");
}
template <typename ValueT>
inline void do_not_optimise(ValueT& val)
{
#if defined(__clang__)
asm volatile("" : "+r,m"(val) : : "memory");
#else
asm volatile("" : "+m,r"(val) : : "memory");
#endif
}
See also: https://github.com/google/benchmark/blob/master/include/benchmark/benchmark.h
Downstream migration PRs have been prepared.
Add basic benchmarking mechanism, with an emphasis on microbenchmarks.
Basic usage: