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/ab_compare.sh b/ab_compare.sh new file mode 100755 index 000000000000..71c1bbd19e9a --- /dev/null +++ b/ab_compare.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# +# A/B comparison protocol for pfor_comparison_benchmark. +# +# WHY THIS EXISTS +# This benchmark has two deterministic confounds, both larger than most +# effects being measured. Neither is noise; neither shrinks with more +# repetitions. See INTEL_RESULTS.md section 10. +# +# (1) ARROW_USER_SIMD_LEVEL caps Arrow's runtime dispatch (bpacking.cc:29-45). +# It reaches BM_PforDecode only -- the FastLanes kernel has no dispatch. +# Capping to AVX2 makes Arrow's sequential decoder 3.21x FASTER than +# leaving it uncapped on Granite Rapids, because the AVX-512 family takes +# a second pass for the frame bias where the xsimd kernels fold it in. +# unset == MAX == AVX512, so the DEFAULT selects the slow path. +# +# (2) The interleaved arm reads either ~36 or ~48 GiB/s (1.33x) at identical +# binary, cap, filter and reps. It is stable to 1.00-1.03x across +# consecutive processes but flips between batches minutes apart, and it +# also moves with how many columns the filter admits (more columns -> +# more corpora allocated before the hot buffer). fl_order is immune. +# +# THE PROTOCOL +# Run every variant ALTERNATING inside one batch, several processes each, with +# the cap pinned. Ratios are then taken within a batch, where confound (2) is +# constant. Report the cross-process spread, because the within-process stddev +# is 0.2-1% here and hides both confounds completely. +# +# The arms come from bench_arms.sh, which also states which of them may be +# divided by which. This script used to name the production decoder against an +# exception-free standalone one and call the ratio a layout result. +# +# USAGE +# ./ab_compare.sh : : [...] # binaries under bin/ +# ROUNDS=3 REPS=3 CORE=2 ./ab_compare.sh bench_O2_256:AVX2 fix_O2_256:AVX2 +# ARM_SETS='ARMS_LAYOUT ARMS_DEST' ./ab_compare.sh bench_O2_256:AVX2 +# +set -uo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")" +. ./bench_arms.sh + +BIN_DIR=${BIN_DIR:-$HOME/Projects/pfor_x86_handoff/width_matrix_v2/bin} +OUT=${OUT:-/tmp/ab} +ROUNDS=${ROUNDS:-3} +REPS=${REPS:-3} +CORE=${CORE:-2} + +# Which arm groups to time. Several in one process is fine and preferred, since +# arms timed together share the machine state; dividing across groups is not -- +# see bench_arms.sh. +ARM_SETS=${ARM_SETS:-'ARMS_LAYOUT ARMS_ORDER'} +SETS=() +for NAME in $ARM_SETS; do SETS+=("${!NAME}"); done +FILTER=$(bench_filter "${SETS[@]}") + +[ $# -ge 1 ] || { sed -n '2,40p' "$0"; exit 1; } + +mkdir -p "$OUT" +LOAD=$(awk '{print int($1)}' /proc/loadavg) +if [ "$LOAD" -gt 2 ] && [ "${FORCE:-0}" != 1 ]; then + echo "!! loadavg is $LOAD -- too busy. Set FORCE=1 to override."; exit 1 +fi + +for i in $(seq 1 "$ROUNDS"); do + for SPEC in "$@"; do + IFS=: read -r B CAP <<<"$SPEC" + [ -x "$BIN_DIR/$B" ] || { echo "-- no binary $BIN_DIR/$B, skipping"; continue; } + # Cap pinned per variant. Stated in the output filename so the analysis + # cannot silently mix caps -- that is confound (1). + ARROW_USER_SIMD_LEVEL="${CAP:-MAX}" taskset -c "$CORE" "$BIN_DIR/$B" \ + --benchmark_filter="$FILTER" \ + --benchmark_repetitions="$REPS" \ + --benchmark_report_aggregates_only=true \ + --benchmark_out="$OUT/${B}__${CAP:-MAX}__$i.json" \ + --benchmark_out_format=json >/dev/null 2>&1 \ + && echo " $B cap=${CAP:-MAX} round $i" \ + || echo " !! $B cap=${CAP:-MAX} round $i FAILED" + done +done +echo +echo "analyze with: python3 ~/Projects/pfor_x86_handoff/ab_analyze.py $OUT" diff --git a/bench_arms.sh b/bench_arms.sh new file mode 100644 index 000000000000..13680b457812 --- /dev/null +++ b/bench_arms.sh @@ -0,0 +1,69 @@ +# Shared arm sets and column set for pfor_comparison_benchmark drivers. +# Sourced, not executed: . "$(dirname "$0")/bench_arms.sh" +# +# WHY THIS FILE EXISTS +# Four drivers each carried their own copy of the column list and the arm +# regex, and all four copies named the same wrong pair: BM_PforDecode (the +# production decoder, which patches exceptions) against BM_InterleavedPforDecode +# and BM_InterleavedPforFlOrderDecode (a standalone path with no exception +# handling at all). A ratio between those is not a layout result -- the +# interleaved side is doing less work. The correct pair had already landed in +# the same benchmark file when those drivers were written. +# +# Keeping the sets here means the rule below is stated once and cannot drift +# between drivers. +# +# WHICH ARMS MAY BE COMPARED WITH WHICH +# Only within a group. Every arm in a group runs the same code with the same +# exception handling and the same destination policy, so the named variable is +# the only difference. +# +# ARMS_LAYOUT sequential vs interleaved container, both through the +# production encoder and decoder with delta declined on both +# sides. This is the pair a layout verdict is read from. +# +# ARMS_ORDER file order vs the paper's lane assignment inside the +# interleaved container. These run arrow/util/fastlanes/ +# interleaved_pfor.h, which has no exception handling anywhere in +# it, so the file-order arm in this group is their only valid +# baseline. Never pair one of these against a production arm. +# The production format has no lane-assignment mode, which is why +# this group exists outside it. +# +# ARMS_DEST whole-output materialization vs a reused block-sized +# destination, at each layout. Answers what the output buffer +# costs, not what the layout costs. +# +# ARMS_DEST arms arrive with the output-buffer-reuse change and +# are absent from a tree without it; a filter naming them then +# matches nothing, which google-benchmark reports as no +# benchmarks to run. +# +# ARMS_SHIPPED the shipping default, planner free to difference a vector. Not +# a layout arm: on a sorted or correlated column it deltas, and +# the decode then also pays a serial prefix sum. +# +# COLUMN SET +# Changing it changes the numbers, because the count of columns a filter +# admits decides how many corpora are allocated before the timed buffer. Hold +# it fixed across anything meant to be compared. + +COLS=${COLS:-'TpcdsSoldDateSk|TpcdsStoreSk|TpcdsItemSk|TpcdsQuantity|EventDate|ClientIP|CounterID|SortedKeys|MonotoneRowId|RandomWalk'} + +ARMS_LAYOUT='BM_(PforPlainSeqDecode|PforPlainInterleavedDecode)' +ARMS_ORDER='BM_(InterleavedPforDecode|InterleavedPforFlOrderRawDecode|InterleavedPforFlOrderDecode)' +ARMS_DEST='BM_Pfor(Whole|Reuse)(Seq|Interleaved)Decode' +ARMS_SHIPPED='BM_Pfor(64)?Decode' + +# Build a google-benchmark filter from one or more arm sets. +# FILTER=$(bench_filter "$ARMS_LAYOUT" "$ARMS_ORDER") +# Groups are unioned so one process can time several of them -- that is fine and +# is in fact preferred, since arms timed in one process share the machine state. +# What is not fine is dividing a number from one group by a number from another. +bench_filter() { + local joined="" a + for a in "$@"; do + joined="${joined:+$joined|}${a}" + done + printf '(%s)/(%s)/' "$joined" "$COLS" +} diff --git a/build_transpose_ab.sh b/build_transpose_ab.sh new file mode 100755 index 000000000000..8ad3d05573b8 --- /dev/null +++ b/build_transpose_ab.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +# +# Build the AVX2 Transpose32x32 A/B points. +# +# Treatment: transposed_delta.h gained Transpose32x32Avx2 (sixteen in-register +# 8x8 transposes) behind #if defined(__AVX2__). Before this, x86 fell through to +# Transpose32x32Scalar -- 1024 four-byte loads at a 128-byte stride plus 1024 +# scalar stores per block, because only NEON had a hand-written path. Isolated, +# the permutation goes 28.67 -> 73.56 GiB/s (2.57x). +# +# Only the kFlOrder arm calls the transpose, so BM_InterleavedPforDecode (file +# order) and BM_PforDecode are controls this change cannot reach. If either +# moves by more than the cross-process spread, the comparison is contaminated -- +# see INTEL_RESULTS.md section 10. +# +# Correctness is not left to inspection: pfor_comparison_benchmark.cc +# round-trips every column through encode+decode and ARROW_CHECKs the result +# before timing starts, for both orders. A wrong transpose aborts the binary. +# Transpose32x32 is also its own inverse on a square grid, so encode and decode +# share the path and a broken permutation cannot cancel itself out. +# +# BUILD ONLY. Time with ab_compare.sh. +set -uo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")" +export PATH="$HOME/.local/bin:$PATH" # cmake must be <4 + +BUILD=build-x86-sweep +OUT="$HOME/Projects/pfor_x86_handoff/width_matrix_v2" +mkdir -p "$OUT/bin" + +# Matched to the existing fix_* binaries so the only difference is the transpose. +POINTS=( + "tr_O3_256:AVX2:-O3:256" + "tr_O2_256:AVX2:-O2:256" +) + +for P in "${POINTS[@]}"; do + IFS=: read -r NAME LEVEL OPT PVW <<<"$P" + echo "== $NAME : ARROW_SIMD_LEVEL=$LEVEL $OPT -mprefer-vector-width=$PVW" + # Explicit every time: CMAKE_CXX_FLAGS_RELEASE is a CACHE variable and would + # otherwise inherit the previous point's value. + if ! cmake -S cpp -B "$BUILD" -GNinja \ + -DCMAKE_BUILD_TYPE=Release \ + -DARROW_SIMD_LEVEL="$LEVEL" \ + -DCMAKE_CXX_FLAGS_RELEASE="$OPT -DNDEBUG -mprefer-vector-width=$PVW" \ + -DCMAKE_C_FLAGS_RELEASE="$OPT -DNDEBUG -mprefer-vector-width=$PVW" \ + -DARROW_PARQUET=ON -DARROW_BUILD_BENCHMARKS=ON \ + -DARROW_WITH_ZSTD=ON -DARROW_WITH_LZ4=ON \ + -DARROW_BUILD_TESTS=OFF > "$OUT/configure_$NAME.log" 2>&1; then + echo "!! configure FAILED -- $OUT/configure_$NAME.log"; continue + fi + if ! cmake --build "$BUILD" --target parquet-pfor-comparison-benchmark -j 6 \ + > "$OUT/build_$NAME.log" 2>&1; then + echo "!! build FAILED -- tail:"; tail -30 "$OUT/build_$NAME.log"; continue + fi + cp "$BUILD/release/parquet-pfor-comparison-benchmark" "$OUT/bin/bench_$NAME" + echo " -> $OUT/bin/bench_$NAME" + # Prove the AVX2 transpose is actually in the kFlOrder kernel. + objdump -d --demangle "$OUT/bin/bench_$NAME" 2>/dev/null | awk ' + /^[0-9a-f]+ <.*InterleavedPforDecode<.*>:$/ { + inside=1 + name=($0 ~ /InterleavedPforOrder\)1/) ? "kFlOrder(1)" : "kFileOrder(0)" + n=0; y=0; z=0; perm=0; next + } + /^[0-9a-f]+ ' + +# name | ARROW_SIMD_LEVEL | optimizer | prefer-vector-width +POINTS=( + "O2_128:SSE4_2:-O2:128" + "O2_256:AVX2:-O2:256" + "O3_128:SSE4_2:-O3:128" + "O3_256:AVX2:-O3:256" +) + +for P in "${POINTS[@]}"; do + IFS=: read -r NAME LEVEL OPT PVW <<<"$P" + echo "==================================================================" + echo "== $NAME : ARROW_SIMD_LEVEL=$LEVEL $OPT -mprefer-vector-width=$PVW" + echo "==================================================================" + + # Explicit every time. Nothing inherited, nothing assumed about Release. + if ! cmake -S cpp -B "$BUILD" -GNinja \ + -DCMAKE_BUILD_TYPE=Release \ + -DARROW_SIMD_LEVEL="$LEVEL" \ + -DCMAKE_CXX_FLAGS_RELEASE="$OPT -DNDEBUG -mprefer-vector-width=$PVW" \ + -DCMAKE_C_FLAGS_RELEASE="$OPT -DNDEBUG -mprefer-vector-width=$PVW" \ + -DARROW_PARQUET=ON -DARROW_BUILD_BENCHMARKS=ON \ + -DARROW_WITH_ZSTD=ON -DARROW_WITH_LZ4=ON \ + -DARROW_BUILD_TESTS=OFF > "$OUT/configure_$NAME.log" 2>&1; then + echo "!! configure FAILED -- $OUT/configure_$NAME.log"; continue + fi + # Record the flags cmake actually resolved, so the log proves the level. + grep -E "CMAKE_CXX_FLAGS(_RELEASE)?:" "$OUT/configure_$NAME.log" | sed 's/^/ /' + + if ! cmake --build "$BUILD" --target parquet-pfor-comparison-benchmark -j 6 \ + > "$OUT/build_$NAME.log" 2>&1; then + echo "!! build FAILED -- $OUT/build_$NAME.log"; continue + fi + + SRC="$BUILD/release/parquet-pfor-comparison-benchmark" + BIN="$OUT/bin/bench_$NAME" + cp "$SRC" "$BIN" + + # ---- verify the width the autovectorizer gave THIS kernel ---------------- + # Disassemble only the kernel's address range: from its symbol to the next. + ADDR=$(nm -C "$BIN" | grep -F "$KERNEL" | awk '{print $1}' | head -1) + { + echo "point=$NAME level=$LEVEL opt=$OPT prefer=$PVW" + echo "sha256: $(sha256sum "$BIN" | cut -d' ' -f1)" + echo "kernel: $KERNEL" + echo "kernel addr: 0x$ADDR" + if [ -n "$ADDR" ]; then + # Boundaries come from objdump's OWN ' :' labels, not from + # sorting nm addresses. The nm approach silently produced an empty range + # for O2_256 -- kernel_O2_256.asm came out 0 bytes and the point then + # reported "zmm: 0 ymm: 0", which reads as a failed vectorization rather + # than a failed extraction. nm lists symbols from the symbol table in an + # order that need not bracket the function body (aliases, local symbols + # and ifunc resolvers all land in between), so "next address after this + # one" is not the function's end. objdump cannot disagree with itself. + objdump -dC --no-show-raw-insn "$BIN" 2>/dev/null \ + | awk -v s="$KERNEL" ' + /^[0-9a-f]+ 0 } + inside' > "$OUT/kernel_$NAME.asm" + echo "kernel insns: $(grep -cE '^\s+[0-9a-f]+:' "$OUT/kernel_$NAME.asm")" + echo "IN-KERNEL vector register usage:" + for R in xmm ymm zmm; do + echo " $R: $(grep -c "%$R" "$OUT/kernel_$NAME.asm")" + done + echo "IN-KERNEL notable ops:" + # v?-prefixed: a 128-bit SSE build emits non-VEX psrld/movdqa, so + # VEX-only patterns reported zero shifts and zero stack traffic for every + # 128-bit point -- which made the narrow builds look spill-free when + # O3_128 in fact had the most stack traffic in the matrix. + for OP in 'v?psrld' 'v?pslld' 'vpsrlvd' 'vpsllvd' 'v?pand' 'v?por' \ + 'v?movdqu' 'v?movdqa' 'vpgatherdd' 'vpermd' 'v?pshufb' \ + 'vpbroadcastd' 'vzeroupper'; do + C=$(grep -cE "^\s+[0-9a-f]+:\s+$OP" "$OUT/kernel_$NAME.asm") + [ "$C" -gt 0 ] && echo " $OP: $C" + done + # Vector moves with a stack-relative operand. Reported as traffic, not + # spills: a genuine stack scratch buffer matches this too, which is + # exactly what the pre-fix kFileOrder path was (a 4 KiB grid, ~1984 + # write-only stores with zero matching reloads). + echo " stack vector traffic: $(grep -cE '^\s+[0-9a-f]+:\s+v?mov(dqu|dqa|ups|aps).*\((%rsp|%rbp)' "$OUT/kernel_$NAME.asm")" + else + echo "!! kernel symbol not found -- cannot verify width" + fi + } > "$OUT/verify_$NAME.txt" 2>&1 + cat "$OUT/verify_$NAME.txt" + + # A width that did not materialize is reported, never silently averaged in. + case "$PVW" in + 256) grep -qE '^ ymm: [1-9]' "$OUT/verify_$NAME.txt" \ + || echo "!! WARNING $NAME: no ymm IN KERNEL -- this is NOT a 256-bit point" ;; + esac + echo +done + +echo "=== builds complete. binaries in $OUT/bin/ ===" +sha256sum "$OUT"/bin/* 2>/dev/null +echo +echo "Distinct binaries (if two points share a sha256, they are the SAME build" +echo "and any difference between their timings is noise):" +sha256sum "$OUT"/bin/* 2>/dev/null | awk '{print $1}' | sort -u | wc -l +echo +echo "Now time them, when the box is quiet:" +echo " BIN_DIR=$OUT/bin ./ab_compare.sh \\" +echo " bench_O2_128:SSE4_2 bench_O2_256:AVX2 \\" +echo " bench_O3_128:SSE4_2 bench_O3_256:AVX2" 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..cea16fe55089 100644 --- a/cpp/src/arrow/util/bpacking.cc +++ b/cpp/src/arrow/util/bpacking.cc @@ -38,7 +38,39 @@ 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 the bit-unpack dispatch at 256 bits. The 512-bit target is the one + // still driven by the legacy generated kernels in + // bpacking_simd512_generated_internal.h, which build their SIMD input + // register from an initializer list of scalar loads (one vmovd + one + // vpinsrd per element) and are not force-inlined, so each step is an + // out-of-line call. Measured on Granite Rapids over 102400 values, that + // makes it 6.26x slower than the AVX2 target (geomean widths 1..31: 6.54 + // vs 40.97 GiB/s) and 0.67x of the *scalar* kernel. Since + // ARROW_RUNTIME_SIMD_LEVEL defaults to MAX, every AVX-512 machine was + // preferring it. Re-enable once the 512-bit kernels issue real vector + // loads -- naively pointing this TU at the Kernel<> machinery used by + // bpacking_simd_{128,256}.cc is NOT the fix: it measures 1.17 GiB/s, + // 5.6x worse again, because most widths land on is_oversized() -> + // NoOpKernel and fall through to the naive path. + // 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 +89,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