Conversation
…ne call The miniblocks of a DELTA_BINARY_PACKED block are packed back to back with no padding between them, so a run of miniblocks that share a bit width is bit-identical to a single longer run at that width. GetInternal called the bit unpacker once per miniblock all the same, which with the default geometry is one call per 32 values - mostly per-call setup. Look ahead over the block's stored bit widths and extend the current call over each following miniblock that has the same width and that the caller has room for in full. A miniblock joins the run only when its width equals the current delta_bit_width_, which InitMiniBlock has already validated, and the run also stops at the end of the block. Add a test over the width patterns that decide where a run starts and stops, and read at a batch size that stops partway through a coalesced run. On the decode benchmarks already in the tree this is 1.17x to 1.33x on top of the previous commit wherever the unpacker's per-call cost is a meaningful share of the work. Decoded values are identical; no encoded byte changes.
The two non-degenerate DELTA_BINARY_PACKED decode arms both hold values in random order, so their deltas are random and the frame sits well below zero. The columns this encoding is chosen for are usually non-decreasing, where every delta is non-negative instead and the frame is at or near zero. Add an arm whose deltas come from the same 1000-wide range as the existing narrow one but accumulate, so the two differ in the order of the values at a nearly unchanged packed width.
The prefix sum that turns deltas back into values does one addition per value, and each one waits on the value before it, so the loop is bound by that chain rather than by how much arithmetic the machine can retire. Replace it with an inclusive scan, which shifts and adds a vector to itself once per power of two and so turns a vector of deltas into a vector of running sums. That shortens the chain from one addition per value to one per vector. The frame is added before the scan, which makes its running multiple fall out of the scan itself, and the previous vector's last value is carried forward in a vector register -- reading it out into a general-purpose register instead costs several times what the scan saves. The scan runs only where a register holds at least four values, the narrowest width measured to win; below that, and on the tail, the value-at-a-time loop does the work. xsimd is already a dependency of this target, so nothing is added to the build. Decode throughput over the previous commit, on the arms whose values are 32-bit: 1.26x on non-decreasing values, 1.24x in random order, and 1.23x at 31 bits per delta. The 64-bit arms hold two values in a register here and are unchanged. Together with the two commits before it this is 2.17x, 2.11x and 1.89x over main, or 2.64 to 5.73 GB/s of decoded output on non-decreasing values.
github-actions
Bot
added
awaiting review
The coalescing guard is against values_remaining_current_mini_block_, so what it needs room for is the rest of the current miniblock, not a whole one; a run legitimately starts from a partly consumed miniblock. "Four is the narrowest width measured to win" reused a word that means bit width everywhere else in this file. The threshold is four lanes. PrefixSumVectorAndTail claimed its deltas pin the stored width at every step of its loop. At the top of the range the spread is all ones, which as a signed delta is -1, so the frame absorbs it and the miniblock stores width 1. Every stored width from 0 to the type's width is still reached, so the coverage is unchanged; only the comment was wrong. Comments only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Once equal-width miniblocks are unpacked in one call, what is left per value is the prefix
sum: a chain of dependent additions with the running total carried from each to the next. A
log-step inclusive scan computes it a register at a time in log2(lanes) shifted additions,
and adding the frame of reference before the scan makes its running multiple fall out of
the scan rather than needing a multiply per lane.
What changes are included in this PR?
The value-at-a-time loop becomes a helper that scans whole registers and finishes the
remainder one value at a time. Every term stays in the unsigned type, so the wrapping the
format specifies is unchanged and decoded values are identical. Two details are worth a
reviewer's attention:
register and re-broadcasting it measured slower than the scalar loop it replaces, so the
helper keeps the carry in a vector register and broadcasts the last lane with a shuffle.
loop is compiled only where a register holds four or more values. At Arrow's default
128-bit baseline that scans 32-bit values and leaves 64-bit ones on today's loop; a wider
baseline scans both. The threshold is
if constexpr, so the 64-bit vector loop is notbuilt by a default configuration and no CI job compiles it, let alone runs it.
This overlaps #51249, whose effect the helper gets for free by taking the frame and the
running total by value, so whichever lands second needs a trivial rebase -- and it is why the
64-bit arms gain here without being vectorized.
Are these changes tested?
A new typed test walks the residual bit widths for both integer widths, at each one over
enough lengths to leave every remainder a register-sized group can leave, so each width is
decoded through the vector loop, through the remainder, and across the hand-off. Its deltas
alternate between the frame and the widest value the width holds, which drives the stored
width across the whole range, keeps a non-zero frame in play, and wraps the running total
repeatedly. Three mutations turn it red: dropping the frame's running multiple and losing the
carry fail the 32-bit instantiation, and ending the remainder loop one value early fails both.
Benchmark
Same setup as #51250.
NarrowSortedis added by the first of the two commits and holdsnon-decreasing values, the shape DELTA_BINARY_PACKED is usually chosen for.
Benchmark
Same setup as #51250.
NarrowSortedis added by the first of the two commits and holdsnon-decreasing values, the shape DELTA_BINARY_PACKED is usually chosen for.
Decode_Int32_NarrowSortedDecode_Int32_NarrowDecode_Int32_WideDecode_Int64_NarrowSortedDecode_Int64_NarrowDecode_Int64_WideDecode_Int32_FixedDecode_Int64_FixedThe 32-bit arms are the vectorized ones. The 64-bit arms hold two values in a register at
this baseline and stay on the value-at-a-time loop; they gain because the helper takes the
frame and the running total by value, which is #51249's effect.
The gain needs #51250 underneath it: on #51249 alone the same kernel measures 0.98x to 1.03x
across the 32-bit arms, because per-register setup only amortizes over the longer runs
coalescing produces.
Are there any user-facing changes?
No. No API change, no format change, and decoded values are identical.