Describe the enhancement requested
Right now the DynamicDispatch makes no assumption on the DynamicFunction::implementations().
However in practice we can assume the implementations to be a compile-time sequence: each available must have been compiled so there is no possibility to add more at runtime (doing so would require a dlopen style plugin loading, which I don't believe is what we want to go towards).
With C++17 and CTAD, it is straightforward to use a static array without needing with compile-time array sizes.
For e.g. it is already done with:
| return std::array{ |
| #if defined(ARROW_HAVE_SSE4_2) |
| Implementation{DispatchLevel::NONE, &bpacking::unpack_sse4_2<Uint>}, |
| #else |
| Implementation{DispatchLevel::NONE, &bpacking::unpack_scalar<Uint>}, |
| #endif |
| #if defined(ARROW_HAVE_RUNTIME_AVX2) |
| Implementation{DispatchLevel::AVX2, &bpacking::unpack_avx2<Uint>}, |
| #endif |
| #if defined(ARROW_HAVE_RUNTIME_AVX512) |
| Implementation{DispatchLevel::AVX512, &bpacking::unpack_avx512<Uint>}, |
| #endif |
| }; |
There are actually few places to change such as:
| static std::vector<std::pair<DispatchLevel, FunctionType>> implementations() { |
| return {{DispatchLevel::NONE, standard::FindMinMaxImpl} |
| #if defined(ARROW_HAVE_RUNTIME_AVX2) |
| , |
| {DispatchLevel::AVX2, FindMinMaxAvx2} |
| #endif |
| }; |
This will be a small immediate yet negligible improvement (the std::vector is actually just parsed once).
The big win, is to use that information to skip dynamic dispatch altogether when there is only one implementation available, as done (verbosly) here:
| #if defined(ARROW_HAVE_NEON) |
| returnbpacking::unpack_neon(in, out, opts); |
| #else |
| static DynamicDispatch<UnpackDynamicFunction<Uint> > dispatch; |
| return dispatch.func(in, out, opts); |
| #endif |
But not done below:
| MinMax FindMinMax(constint16_t* levels, int64_t num_levels) { |
| static DynamicDispatch<MinMaxDynamicFunction> dispatch; |
| return dispatch.func(levels, num_levels); |
| } |
Component(s)
C++
Describe the enhancement requested
Right now the
DynamicDispatchmakes no assumption on theDynamicFunction::implementations().However in practice we can assume the implementations to be a compile-time sequence: each available must have been compiled so there is no possibility to add more at runtime (doing so would require a
dlopenstyle plugin loading, which I don't believe is what we want to go towards).With C++17 and CTAD, it is straightforward to use a static array without needing with compile-time array sizes.
For e.g. it is already done with:
arrow/cpp/src/arrow/util/bpacking.cc
Lines 35 to 47 in e8b7b4e
There are actually few places to change such as:
arrow/cpp/src/parquet/level_comparison.cc
Lines 57 to 63 in e8b7b4e
This will be a small immediate yet negligible improvement (the std::vector is actually just parsed once).
The big win, is to use that information to skip dynamic dispatch altogether when there is only one implementation available, as done (verbosly) here:
arrow/cpp/src/arrow/util/bpacking.cc
Lines 55 to 60 in e8b7b4e
But not done below:
arrow/cpp/src/parquet/level_comparison.cc
Lines 74 to 77 in e8b7b4e
Component(s)
C++