Page MenuHomePhabricator

Add benchmarks for std::function.
ClosedPublic

Authored by sbenza on Oct 10 2018, 8:54 AM.

Details

Summary

Benchmarks for construct, copy, move, swap, destroy and invoke, with 8
different input states.
For the cases that matter, it tests with and without allowing constant
value propagation from construction into the operation tested.

This also adds helper functions to generate the cartesian product of
different configurations and generate benchmarks for all of them.

Diff Detail

Repository
rL LLVM

Event Timeline

sbenza created this revision.Oct 10 2018, 8:54 AM
sbenza updated this revision to Diff 169023.Oct 10 2018, 8:58 AM

Fix casing for some function names.

sbenza updated this revision to Diff 169052.Oct 10 2018, 10:56 AM

Minor comment fix

Just an FYI. Currently we build the benchmarks as C++17. So I think it's fair to go nuts with C++17 features where they make these benchmarks better.

benchmarks/CartesianBenchmarks.hpp
1 ↗(On Diff #169023)

Please add the standard license header here.

82 ↗(On Diff #169023)

Let's replace the raw attributes with the TEST_ALWAYS_INLINE defined in `test/support/test_macros.h' (PS. 'test/support' is passed via -I, so you only need to write #include "test_macros.h")

benchmarks/function.bench.cpp
1 ↗(On Diff #169023)

Please add the standard license header here.

210 ↗(On Diff #169023)

I don't love the need to fully qualify the benchmark namespace. Perhaps we can do something better?

sbenza updated this revision to Diff 169071.Oct 10 2018, 12:25 PM

Use TEST_ALWAYS_INLINE instead of attribute((always_inline))

sbenza updated this revision to Diff 169074.Oct 10 2018, 12:36 PM
sbenza marked 3 inline comments as done.

Fixes

benchmarks/function.bench.cpp
210 ↗(On Diff #169023)

They don't need to be.
I just copied the main() from the BENCHMARK_MAIN macro, which had them like that.
If you want I can use the macro to define main and move the registration to a global initializer.

sbenza marked 2 inline comments as done.Oct 11 2018, 9:44 AM
mclow.lists added inline comments.
benchmarks/CartesianBenchmarks.hpp
16 ↗(On Diff #169074)

You don't need "../test/support" here.

EricWF accepted this revision.Oct 11 2018, 10:01 AM

LGTM after addressing inline comments.

benchmarks/CartesianBenchmarks.hpp
16 ↗(On Diff #169074)

I think this can be just test_macros.h. The build should add support to the include directories.

26 ↗(On Diff #169074)

In C++17 we could write this as:

template <class D, class E, size_t ...Idxs>
constexpr auto MakeEnumValueTuple(std::index_sequence<Idxs...>) {
  return std::make_tuple(EnumValue<D, E, Idxs>{}...);
}

template <class Derived, class EnumType, size_t NumLabels>
using EnumValuesAsTuple = decltype(internal::MakeEnumValueTuple<Derived, EnumType>(std::make_index_sequence<NumLabels>{}));
90 ↗(On Diff #169074)

I really need to land something like this in the Benchmark library.

benchmarks/function.bench.cpp
17 ↗(On Diff #169074)

#include "test_macros.h" should work.

This revision is now accepted and ready to land.Oct 11 2018, 10:01 AM
sbenza updated this revision to Diff 169284.Oct 11 2018, 1:00 PM
sbenza marked 4 inline comments as done.

#include changes. C++17 cleanup.

benchmarks/CartesianBenchmarks.hpp
26 ↗(On Diff #169074)

Thanks. I forgot to update this when you said we can use C++17.

This revision was automatically updated to reflect the committed changes.