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) |
|
return bpacking::unpack_neon(in, out, opts); |
|
#else |
|
static DynamicDispatch<UnpackDynamicFunction<Uint> > dispatch; |
|
return dispatch.func(in, out, opts); |
|
#endif |
But not done below:
|
MinMax FindMinMax(const int16_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++