pitrou commented on code in PR #49756:
URL: https://github.com/apache/arrow/pull/49756#discussion_r3124112195
##########
cpp/src/arrow/util/bpacking.cc:
##########
@@ -52,12 +65,14 @@ struct UnpackDynamicFunction {
template <typename Uint>
void unpack(const uint8_t* in, Uint* out, const UnpackOptions& opts) {
-#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
+ auto constexpr kImplementations =
UnpackDynamicFunction<Uint>::implementations();
+ if constexpr (kImplementations.size() == 1) {
Review Comment:
Is this condition actually useful? I guess it's a shortcut, but it's not
obvious that it applies to common cases (x86 or ARM with default SIMD options).
At worse, this could be added generically to `DynamicDispatch` instead. But
I doubt it's worth it.
##########
cpp/src/arrow/util/bpacking_benchmark.cc:
##########
@@ -107,10 +107,10 @@ void BM_Unpack(benchmark::State& state, bool aligned,
UnpackFunc<Int> unpack, bo
// will not emit runs larger than 512 (though other implementation might), so
we biased
// the benchmarks towards a rather small scale.
static const auto kNumValuesRange = benchmark::CreateRange(32, 512, 2);
-constexpr std::initializer_list<int64_t> kBitWidths8 = {1, 2, 8};
-constexpr std::initializer_list<int64_t> kBitWidths16 = {1, 2, 8, 13};
-constexpr std::initializer_list<int64_t> kBitWidths32 = {1, 2, 8, 20};
-constexpr std::initializer_list<int64_t> kBitWidths64 = {1, 2, 8, 20, 47};
+static const auto kBitWidths8 = benchmark::CreateDenseRange(0, 8, 1);
+static const auto kBitWidths16 = benchmark::CreateDenseRange(0, 16, 1);
+static const auto kBitWidths32 = benchmark::CreateDenseRange(0, 32, 1);
+static const auto kBitWidths64 = benchmark::CreateDenseRange(0, 64, 1);
Review Comment:
This will create a large number of nano-benchmarks, do really want this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
