diff --git a/.gitignore b/.gitignore index 83458ab2057b..dc53bf2c0d82 100644 --- a/.gitignore +++ b/.gitignore @@ -94,3 +94,6 @@ rat.txt # for ODBC DLL *.rc + +# Local out-of-tree benchmark build dir +cpp/build-bench/ diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index 21341167fe99..8887a1ed9294 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -633,21 +633,12 @@ endif() if(NOT MSVC) set(C_RELEASE_FLAGS "") - if(CMAKE_C_FLAGS_RELEASE MATCHES "-O3") - string(APPEND C_RELEASE_FLAGS " -O2") - endif() + # Local override: keep -O3 from CMake's default Release flags for the + # pfor benchmark — the bench is sensitive to inlining/unrolling that + # -O3 enables. Upstream Arrow downgrades to -O2 here; we skip that. set(CXX_RELEASE_FLAGS "") - if(CMAKE_CXX_FLAGS_RELEASE MATCHES "-O3") - string(APPEND CXX_RELEASE_FLAGS " -O2") - endif() set(C_RELWITHDEBINFO_FLAGS "") - if(CMAKE_C_FLAGS_RELWITHDEBINFO MATCHES "-O3") - string(APPEND C_RELWITHDEBINFO_FLAGS " -O2") - endif() set(CXX_RELWITHDEBINFO_FLAGS "") - if(CMAKE_CXX_FLAGS_RELWITHDEBINFO MATCHES "-O3") - string(APPEND CXX_RELWITHDEBINFO_FLAGS " -O2") - endif() if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") string(APPEND C_RELEASE_FLAGS " -ftree-vectorize") string(APPEND CXX_RELEASE_FLAGS " -ftree-vectorize") diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 8750598f6c3b..0a239e893258 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -566,6 +566,8 @@ set(ARROW_UTIL_SRCS util/math_internal.cc util/memory.cc util/mutex.cc + util/pfor/pfor.cc + util/pfor/pfor_wrapper.cc util/ree_util.cc util/secure_string.cc util/string.cc diff --git a/cpp/src/arrow/meson.build b/cpp/src/arrow/meson.build index 831bc1218083..ebba2cd22fbb 100644 --- a/cpp/src/arrow/meson.build +++ b/cpp/src/arrow/meson.build @@ -204,6 +204,8 @@ arrow_util_srcs = [ 'util/math_internal.cc', 'util/memory.cc', 'util/mutex.cc', + 'util/pfor/pfor.cc', + 'util/pfor/pfor_wrapper.cc', 'util/ree_util.cc', 'util/secure_string.cc', 'util/string.cc', diff --git a/cpp/src/arrow/util/CMakeLists.txt b/cpp/src/arrow/util/CMakeLists.txt index 628e9a4d1c7e..9be9dbfa7188 100644 --- a/cpp/src/arrow/util/CMakeLists.txt +++ b/cpp/src/arrow/util/CMakeLists.txt @@ -118,6 +118,10 @@ add_arrow_test(threading-utility-test test_common.cc thread_pool_test.cc) +add_arrow_test(pfor-test SOURCES pfor/pfor_test.cc) + +add_arrow_benchmark(pfor/pfor_benchmark) + add_arrow_benchmark(bit_block_counter_benchmark) add_arrow_benchmark(bit_util_benchmark) add_arrow_benchmark(bitmap_reader_benchmark) diff --git a/cpp/src/arrow/util/bpacking.cc b/cpp/src/arrow/util/bpacking.cc index 1bf81df4f28f..4a2ec0f9c059 100644 --- a/cpp/src/arrow/util/bpacking.cc +++ b/cpp/src/arrow/util/bpacking.cc @@ -43,6 +43,23 @@ struct UnpackDynamicFunction { } }; +template +struct UnpackBiasDynamicFunction { + using FunctionType = decltype(&bpacking::unpack_bias_scalar); + + static constexpr auto targets() { + return std::array{ + ARROW_DISPATCH_TARGET_NONE(&bpacking::unpack_bias_scalar) // + ARROW_DISPATCH_TARGET_NEON(&bpacking::unpack_bias_neon) // + ARROW_DISPATCH_TARGET_SVE128(&bpacking::unpack_bias_sve128) // + ARROW_DISPATCH_TARGET_SVE256(&bpacking::unpack_bias_sve256) // + ARROW_DISPATCH_TARGET_SSE4_2(&bpacking::unpack_bias_sse4_2) // + ARROW_DISPATCH_TARGET_AVX2(&bpacking::unpack_bias_avx2) // + ARROW_DISPATCH_TARGET_AVX512(&bpacking::unpack_bias_avx512) // + }; + } +}; + } // namespace template @@ -57,4 +74,19 @@ template void unpack(const uint8_t*, uint16_t*, const UnpackOptions&); template void unpack(const uint8_t*, uint32_t*, const UnpackOptions&); template void unpack(const uint8_t*, uint64_t*, const UnpackOptions&); +template +void unpack_bias(const uint8_t* in, Uint* out, const UnpackOptions& opts, Uint bias) { + static const DynamicDispatch> dispatch; + return dispatch(in, out, opts, bias); +} + +template void unpack_bias(const uint8_t*, uint8_t*, const UnpackOptions&, + uint8_t); +template void unpack_bias(const uint8_t*, uint16_t*, const UnpackOptions&, + uint16_t); +template void unpack_bias(const uint8_t*, uint32_t*, const UnpackOptions&, + uint32_t); +template void unpack_bias(const uint8_t*, uint64_t*, const UnpackOptions&, + uint64_t); + } // namespace arrow::internal diff --git a/cpp/src/arrow/util/bpacking_dispatch_internal.h b/cpp/src/arrow/util/bpacking_dispatch_internal.h index 6ea6adee1800..a2f344009233 100644 --- a/cpp/src/arrow/util/bpacking_dispatch_internal.h +++ b/cpp/src/arrow/util/bpacking_dispatch_internal.h @@ -31,23 +31,54 @@ namespace arrow::internal::bpacking { /// Unpack a zero bit packed array. -template -ARROW_FORCE_INLINE void unpack_null(const uint8_t* in, Uint* out, int batch_size) { - std::memset(out, 0, batch_size * sizeof(Uint)); +template +ARROW_FORCE_INLINE void unpack_null(const uint8_t* in, Uint* out, int batch_size, + Uint bias = Uint{}) { + if constexpr (kHasBias) { + // Every unpacked value is zero, so every output value is the bias. + std::fill(out, out + batch_size, bias); + } else { + std::memset(out, 0, batch_size * sizeof(Uint)); + } } /// Unpack a packed array where packed and unpacked values have exactly the same number of /// bits. -template -ARROW_FORCE_INLINE void unpack_full(const uint8_t* in, Uint* out, int batch_size) { +template +ARROW_FORCE_INLINE void unpack_full(const uint8_t* in, Uint* out, int batch_size, + Uint bias = Uint{}) { if constexpr (ARROW_LITTLE_ENDIAN == 1) { - std::memcpy(out, in, batch_size * sizeof(Uint)); + if constexpr (kHasBias) { + // Two things are needed for this loop to reach memcpy speed, and it is + // 10.8x slower than the memcpy below without them -- far worse than the + // second add pass the bias exists to remove. + // 1. A constant-size memcpy for the load, not SafeLoadAs: SafeLoadAs + // builds an AlignedStorage per element and the vectorizer refuses it, + // while a fixed-size memcpy is just an unaligned load. + // 2. A restrict qualifier: `in` is a uint8_t*, so it may alias + // anything, including `out`. Without restating that they are + // distinct the compiler has to assume overlap and emits a scalar + // loop. + const uint8_t* ARROW_RESTRICT src = in; + Uint* ARROW_RESTRICT dst = out; + for (int k = 0; k < batch_size; k += 1) { + Uint val; + std::memcpy(&val, src + (k * sizeof(Uint)), sizeof(Uint)); + dst[k] = static_cast(val + bias); + } + } else { + std::memcpy(out, in, batch_size * sizeof(Uint)); + } } else { using bit_util::FromLittleEndian; using util::SafeLoadAs; for (int k = 0; k < batch_size; k += 1) { - out[k] = FromLittleEndian(SafeLoadAs(in + (k * sizeof(Uint)))); + Uint val = FromLittleEndian(SafeLoadAs(in + (k * sizeof(Uint)))); + if constexpr (kHasBias) { + val = static_cast(val + bias); + } + out[k] = val; } } } @@ -96,9 +127,9 @@ using SpreadBufferUint = std::conditional_t< /// This function works for all input batch sizes but is not the fastest. /// In prolog mode, instead of unpacking all required element, the function will /// stop if it finds a byte aligned value start. -template +template ARROW_FORCE_INLINE int unpack_exact(const uint8_t* in, const uint8_t* in_end, Uint* out, - int batch_size, int bit_offset) { + int batch_size, int bit_offset, Uint bias = Uint{}) { static_assert(kPackedBitWidth > 0); // For the epilog we adapt the max spread since better alignment give shorter spreads @@ -168,6 +199,9 @@ ARROW_FORCE_INLINE int unpack_exact(const uint8_t* in, const uint8_t* in_end, Ui } } + if constexpr (kHasBias) { + val = static_cast(val + bias); + } *out = val; out++; start_bit += kPackedBitWidth; @@ -190,12 +224,12 @@ ARROW_FORCE_INLINE int unpack_exact(const uint8_t* in, const uint8_t* in_end, Ui /// This is used to safely overread. /// Negative value to deduce from batch_size. template typename Unpacker, - typename UnpackedUInt> + bool kHasBias = false, typename UnpackedUInt> void unpack_width(const uint8_t* in, UnpackedUInt* out, int batch_size, int bit_offset, - int max_read_bytes) { + int max_read_bytes, UnpackedUInt bias = UnpackedUInt{}) { if constexpr (kPackedBitWidth == 0) { // Easy case to handle, simply setting memory to zero. - return unpack_null(in, out, batch_size); + return unpack_null(in, out, batch_size, bias); } else { // Number of bytes to read according to batch_size. const int bytes_batch = static_cast( @@ -206,8 +240,8 @@ void unpack_width(const uint8_t* in, UnpackedUInt* out, int batch_size, int bit_ const uint8_t* in_end = in + (max_read_bytes >= 0 ? max_read_bytes : bytes_batch); // In case of misalignment, we need to run the prolog until aligned. - int extracted = - unpack_exact(in, in_end, out, batch_size, bit_offset); + int extracted = unpack_exact( + in, in_end, out, batch_size, bit_offset, bias); // We either extracted everything or found a alignment const int start_bit = extracted * kPackedBitWidth + bit_offset; ARROW_DCHECK((extracted == batch_size) || ((start_bit) % 8 == 0)); @@ -218,7 +252,7 @@ void unpack_width(const uint8_t* in, UnpackedUInt* out, int batch_size, int bit_ if constexpr (kPackedBitWidth == 8 * sizeof(UnpackedUInt)) { // Only memcpy / static_cast - return unpack_full(in, out, batch_size); + return unpack_full(in, out, batch_size, bias); } else { using UnpackerForWidth = Unpacker; // Number of values extracted by one iteration of the kernel @@ -229,9 +263,30 @@ void unpack_width(const uint8_t* in, UnpackedUInt* out, int batch_size, int bit_ if constexpr (kValuesUnpacked > 0) { const uint8_t* in_last = in_end - kBytesRead; + // Whether this unpacker family folds the bias into its own stores. The + // xsimd kernels do; the generated scalar and AVX-512 families do not, so + // they get a second pass over the values the kernel just wrote. That + // pass is over kValuesUnpacked elements still in L1, not over the whole + // output, but it is a second pass all the same and the measured cost of + // one is 1.47-2.40x the unpack -- so the fallback is for correctness on + // those targets, not a substitute for folding it in. + constexpr bool kUnpackerTakesBias = + requires(const uint8_t* i, UnpackedUInt* o, UnpackedUInt b) { + UnpackerForWidth::unpack(i, o, b); + }; // NOLINT(readability/braces) + // Running the optimized kernel for batch extraction while ((batch_size >= kValuesUnpacked) && (in <= in_last)) { - in = UnpackerForWidth::unpack(in, out); + if constexpr (kHasBias && kUnpackerTakesBias) { + in = UnpackerForWidth::unpack(in, out, bias); + } else { + in = UnpackerForWidth::unpack(in, out); + if constexpr (kHasBias) { + for (int k = 0; k < kValuesUnpacked; ++k) { + out[k] = static_cast(out[k] + bias); + } + } + } out += kValuesUnpacked; batch_size -= kValuesUnpacked; } @@ -245,406 +300,412 @@ void unpack_width(const uint8_t* in, UnpackedUInt* out, int batch_size, int bit_ // Running the epilog for the remaining values that don't fit in a kernel ARROW_DCHECK_GE(batch_size, 0); ARROW_COMPILER_ASSUME(batch_size >= 0); - unpack_exact(in, in_end, out, batch_size, - /* bit_offset= */ 0); + unpack_exact(in, in_end, out, batch_size, + /* bit_offset= */ 0, bias); } } } -template