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/DefineOptions.cmake b/cpp/cmake_modules/DefineOptions.cmake index c372f9f19898..49d1e3b1708d 100644 --- a/cpp/cmake_modules/DefineOptions.cmake +++ b/cpp/cmake_modules/DefineOptions.cmake @@ -194,6 +194,9 @@ takes precedence over ccache if a storage backend is configured" ON) define_option(ARROW_GGDB_DEBUG "Pass -ggdb flag to debug builds" ON) + define_option(ARROW_RELEASE_O3 + "Keep CMake's default -O3 in Release builds instead of -O2" OFF) + define_option(ARROW_WITH_MUSL "Whether the system libc is musl or not" OFF) define_option(ARROW_ENABLE_THREADING "Enable threading in Arrow core" ON) diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index 21341167fe99..34baad5f55af 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -632,20 +632,25 @@ endif() # Same as Release, except with debug symbols enabled. if(NOT MSVC) + # CMake's default Release flags are "-O3 -DNDEBUG"; the appends below + # downgrade them to -O2, since the last -O flag wins in GCC. Set + # ARROW_RELEASE_O3 to skip the downgrade: the bit-packing kernels are + # built around the inlining and unrolling -O3 enables, and the + # throughput figures quoted for them were measured with it in effect. set(C_RELEASE_FLAGS "") - if(CMAKE_C_FLAGS_RELEASE MATCHES "-O3") + if(CMAKE_C_FLAGS_RELEASE MATCHES "-O3" AND NOT ARROW_RELEASE_O3) string(APPEND C_RELEASE_FLAGS " -O2") endif() set(CXX_RELEASE_FLAGS "") - if(CMAKE_CXX_FLAGS_RELEASE MATCHES "-O3") + if(CMAKE_CXX_FLAGS_RELEASE MATCHES "-O3" AND NOT ARROW_RELEASE_O3) string(APPEND CXX_RELEASE_FLAGS " -O2") endif() set(C_RELWITHDEBINFO_FLAGS "") - if(CMAKE_C_FLAGS_RELWITHDEBINFO MATCHES "-O3") + if(CMAKE_C_FLAGS_RELWITHDEBINFO MATCHES "-O3" AND NOT ARROW_RELEASE_O3) string(APPEND C_RELWITHDEBINFO_FLAGS " -O2") endif() set(CXX_RELWITHDEBINFO_FLAGS "") - if(CMAKE_CXX_FLAGS_RELWITHDEBINFO MATCHES "-O3") + if(CMAKE_CXX_FLAGS_RELWITHDEBINFO MATCHES "-O3" AND NOT ARROW_RELEASE_O3) string(APPEND CXX_RELWITHDEBINFO_FLAGS " -O2") endif() if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 8750598f6c3b..6e4e90973456 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -551,6 +551,7 @@ set(ARROW_UTIL_SRCS util/decimal.cc util/delimiting.cc util/dict_util.cc + util/fastlanes/interleaved_pfor_baseline.cc util/fixed_width_internal.cc util/float16.cc util/formatting.cc @@ -566,6 +567,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 @@ -591,6 +594,28 @@ append_runtime_avx512_src(ARROW_UTIL_SRCS util/bpacking_simd_avx512.cc) append_runtime_sve128_src(ARROW_UTIL_SRCS util/bpacking_simd_128_alt.cc) append_runtime_sve256_src(ARROW_UTIL_SRCS util/bpacking_simd_256.cc) +# One source compiled once per instruction set, the way bpacking_simd_256.cc is +# registered for both AVX2 and SVE256 above: the file forks on the platform +# macros, and only one of them is ever defined on a given target. +append_runtime_avx2_src(ARROW_UTIL_SRCS util/fastlanes/interleaved_pfor_simd.cc) +append_runtime_sve128_src(ARROW_UTIL_SRCS util/fastlanes/interleaved_pfor_simd.cc) + +# The interleaved kernels are portable C++ with no intrinsics, so what they +# compile to is decided by the optimizer, not by the source. At width 16 gcc 11.5 +# emits 289 instructions and no vector operations at -O2, 322 with 81 vector +# operations at Release's -O2 -ftree-vectorize, and 849 with 513 at -O3; the +# published throughput figures for this layout were all taken at the last of +# those. APPEND rather than a plain set, because the two calls above have already +# put the instruction-set flags in this property. Release only -- Debug and the +# sanitizer builds want their own level, and MSVC spells this differently. +if(NOT MSVC) + set_property(SOURCE util/fastlanes/interleaved_pfor_baseline.cc + util/fastlanes/interleaved_pfor_simd.cc + APPEND + PROPERTY COMPILE_OPTIONS + "$<$,$>:-O3>") +endif() + if(ARROW_WITH_BROTLI) list(APPEND ARROW_UTIL_SRCS util/compression_brotli.cc) endif() 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..0e1a8020678c 100644 --- a/cpp/src/arrow/util/bpacking.cc +++ b/cpp/src/arrow/util/bpacking.cc @@ -38,7 +38,29 @@ struct UnpackDynamicFunction { ARROW_DISPATCH_TARGET_SVE256(&bpacking::unpack_sve256) // ARROW_DISPATCH_TARGET_SSE4_2(&bpacking::unpack_sse4_2) // ARROW_DISPATCH_TARGET_AVX2(&bpacking::unpack_avx2) // - ARROW_DISPATCH_TARGET_AVX512(&bpacking::unpack_avx512) // + // Cap bit-unpack dispatch at 256 bits. The generated AVX-512 kernels + // assemble vectors from scalar loads and use out-of-line calls, making + // them slower than the AVX2 path. Re-enable this target when its kernels + // use vector loads directly. + // ARROW_DISPATCH_TARGET_AVX512(&bpacking::unpack_avx512) // + }; + } +}; + +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) // + // Capped at 256 bits for the reason given in UnpackDynamicFunction above. + // ARROW_DISPATCH_TARGET_AVX512(&bpacking::unpack_bias_avx512) // }; } }; @@ -57,4 +79,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..e89ccc9e8083 100644 --- a/cpp/src/arrow/util/bpacking_dispatch_internal.h +++ b/cpp/src/arrow/util/bpacking_dispatch_internal.h @@ -31,23 +31,52 @@ 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 details let this loop approach memcpy speed: + // 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 +125,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 +197,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 +222,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 +238,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 +250,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 +261,29 @@ 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 + // Some generated kernels do not accept a bias. Apply it in a fallback + // pass for correctness on those targets. + 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 +297,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