Conversation
|
Thanks for opening a pull request! This pull request has been automatically converted to a draft because its title doesn't match Arrow's required format. If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or After updating the title, you can mark the pull request as ready for review. See also: |
github-actions
Bot
added
Component: Parquet
Component: C++
awaiting review
prtkgaur
changed the title
Implements the PFOR (Patched Frame of Reference) integer compression algorithm as a standalone utility library in arrow/util/pfor/. Includes: - Cost model for optimal bit width selection (histogram-based) - Vector-level encode/decode with FOR + bit-packing + exceptions - Page-level wrapper with header, offset array, and multi-vector layout - Round-trip tests for int32 and int64 at the vector and page level, covering outliers, all-identical and all-zero runs, negative values, the int min/max edges, a single element, and a page whose length is an exact multiple of the vector size
Adds PFOR = 11 to the Encoding enum and wires it into the parquet read/write pipeline: - PforEncoder<DType> in encoder.cc (buffers values, calls PforWrapper::Encode) - PforDecoder<DType> in decoder.cc (decodes all values on first access) - PFOR case in column_reader.cc InitializeDataDecoder - Encoding string mapping in types.cc Supports INT32 and INT64 column types.
Benchmarks encode/decode throughput for int32/int64 across 10 data distributions inspired by Snowflake's NumericComprBenchmark: constant, sequential, small range, high-base-small-range (timestamps), with outliers (exception path), random, TPC-DS date/store/item/quantity keys. Each distribution runs at 1K/10K/100K/1M elements. Reports bytes/s, items/s, and compression ratio.
Defaults to kPforVectorSize, so existing callers are unaffected.
Load() now returns Result<PforVectorInfo> after the Status/Result refactoring. Use ASSERT_OK_AND_ASSIGN to properly unwrap the result in tests.
prtkgaur
force-pushed
the
pgaur_interleavedPlusFastLanesDelta
branch
from
October 2, 2026 15:18
e737aaf to
da4ffbd
Compare
prtkgaur
force-pushed
the
pgaur_interleavedPlusFastLanesDelta
branch
3 times, most recently
from
October 2, 2026 22:46
b15f1e6 to
633ba32
Compare
Make LoadHeader fallible, use SafeLoadAs and SafeStore for header and offset fields, and validate the packing mode, element width, vector size, and element count at the page boundary.
Use int64_t for encoded sizes, uint8_t for byte buffers, and fully qualified PFOR names at Parquet call sites. Remove the pointer casts made unnecessary by the consistent buffer type.
Convert the vector metadata and encoded-vector types to classes with explicit accessors. Validate widths and exception counts while loading metadata, and construct metadata atomically on encode.
Compiles the PFOR sources into the parquet target and adds the parquet-pfor-comparison-benchmark executable.
Replaces hand-rolled power-of-two and ceiling-division arithmetic with bit_util::IsPowerOf2 and bit_util::CeilDiv, and records where incremental encode and decode would hook in.
Commit 00b6318 introduced ARROW_DCHECK(bit_util::IsPowerOf2(vector_size)) in PforWrapper<T>::Encode, but vector_size is int32_t and bit_util has overloads only for int64_t and uint64_t -- the call is ambiguous and the file no longer compiles. Cast to int64_t to disambiguate. CeilDiv calls in the same file already promote to int64_t implicitly via its int64_t-only signature.
Use the validated page-level packing mode to distinguish sequential and lane-interleaved payloads. Apply interleaving only to full 1024-value int32 vectors; encode unsupported widths and tail vectors sequentially.
Every bit width round-trips, framed and unframed, and the framed cases go through the unpack that folds the frame into its own store. The widths are built from a delta mask rather than a range so the cost model cannot land on a narrower width and leave the loop testing one kernel thirty-one times; the test reads back the width the encoder settled on and checks it. One test exists only to keep the others honest. Because the two layouts write the same number of bytes, a mode request that quietly fell back to sequential on both the encode and the decode side would pass every round trip above. So one test asserts the payloads really are different bytes at widths 1 through 31 -- and identical at 32, where the two layouts coincide and the kernel is a memcpy.
The layout is not in the Parquet specification, so a file written with it can only be read by an implementation that knows it. It is therefore off unless a writer asks: WriterProperties::Builder::enable_pfor_interleaved_bit_packing. A reader that does not know the layout rejects the page rather than misreading it, because the mode is recorded in a header byte the reader already validates. The flag is a request. It reaches only 4-byte columns, and within those only the full vectors of a page, so a file whose columns are not all eligible can still be written with one setting: everything the layout cannot cover is written the way PFOR has always written it. MakeEncoder grows a defaulted parameter rather than taking the properties, which would put the whole of the writer's configuration behind a factory that needs one bit of it.
Because both layouts write the same number of bytes, a flag that never reached the encoder would still round-trip a table. So the test writes the same table with the flag and without it and compares the two files: same length, different bytes is what shows the flag arrived. The int64 case asserts the opposite -- the two files are identical, byte for byte -- because the layout cannot apply there and the request is dropped rather than refused.
The benchmark only ever encoded with the default packing mode, so nothing in the tree measured the layout this branch adds. Thread the mode through the two benchmark bodies and register a paired int32 variant for each of the ten distributions. The encode variants report CompRatio%, which comes out identical to the sequential variant's for every distribution: the two layouts write the same number of bits, so the layout is a decode-speed choice and not a size trade- off. There is no int64 variant because only 32-bit values reach the interleaved kernels.
Every existing size is an exact multiple of the 1024-value vector, so the tail path that falls back to sequential packing was never timed. The Parquet writer caps a data page at max_rows_per_page = 20,000 rows, which for int32 always binds before the 1 MB size target, so 20,000 values is the destination a reader actually decodes into. Add it. It also has a partial last vector (19 x 1024 + 544), which is what makes it worth measuring separately from 10240 and 102400.
Name the sequential unpacker in the shared read-bound comment and document where the interleaved layout comparison is meaningful. Comments only.
lane_delta.h gives lane l the container rows l, 32+l, ..., so its in-lane predecessor is 32 positions back in file order and its differences are wider than DELTA_BINARY_PACKED's. The paper assigns lane l the contiguous run [32l, 32l+32) instead, which makes the in-lane predecessor the immediately preceding value and the stored differences exactly the format's, at the price of 32 entry points per block and a transpose back to file order. transposed_delta.h implements that, with the bases stored either raw or delta-encoded across lanes, and the benchmark adds four decode variants so the base stream and the transpose can be priced separately from the container.
Transpose each lane-parallel delta block while computing its prefix sums, avoiding a separate repair pass. Preserve the running value across blocks and verify that the fused path matches the original layout.
The layout had only ever been called as a kernel, from a benchmark that handed it a payload and a value count. That skips the page header, the decoder's dispatch and the level handling, so dividing a kernel rate by a full decoder's rate overstates what a reader would gain. Give it page framing and register it as an encoding, so the same encoder and decoder interfaces every other encoding is reached through also reach this one. The framing adds a little-endian value count ahead of the payload: the payload's own header does not carry one, decode needs it to locate every section, and a page's level count cannot stand in because it counts nulls too. INT32 only, because the packing kernel works on 32-bit lanes. The payload still stores its packed words and per-block minimums in host byte order, which a format specification would have to pin down before a file written this way could be read anywhere else.
Compare the lane-parallel delta path with Arrow's existing DELTA_BINARY_PACKED decoder over the same generated inputs.
The encoding is registered for INT32, so the fuzzer reaches it with pages it has mutated. Four kinds of damage now raise instead of reading past the buffer: a page shorter than the value count it declares, a page too short to hold a count at all, a declared count larger than the page header's level count allows, and a block claiming a bit width no 32-bit lane can hold.
The 32 entry points of a block are bit-packed, and reading them with one unaligned 64-bit load per value reads up to 7 bytes past the stream. That was covered by slack in the kernel's own size bound, but a page is sized to its contents: when the last block's differences are all equal its payload is empty, the entry points end the page, and the load runs off the buffer. Staging the stream -- at most 128 bytes, once per 1024 values -- keeps the loop free of both a bounds test and short loads, and costs 3% of decode. Splitting the loop instead, so that only the last values take a short load, costs 8%: the split needs a division to find where to stop, and the fast half loses its compile-time trip count. A sanitizer harness reports an 8-byte read one byte past a 478-byte page against the old code and nothing against this one, and a sweep of all 33 widths against a reference unpack agrees value for value.
The kernel encodes a whole array and is told its length; a page has to say how long it is, and a decoder has to be able to tell a truncated or mutated page from a valid one before it reads a byte of payload. The framing is a 4-byte value count followed by the kernel's output, and validation walks the blocks: each block's payload width and entry-point width are bounded, and every block's contribution to the size is added up and checked against the page. Decode reads in place when the payload is 4-byte aligned, which it is for a page written by this encoder, and copies only when a caller hands it something else.
Store the value count, lane entry points, and packed delta blocks needed to decode a page independently. Validate truncation, block counts, and alignment, and cover constant and partial final blocks.
InterleavedBitPackingLayout and InterleavedRequestIgnoredForInt64 built WriterProperties directly and never called enable_pfor_encoding(), so both threw the preview-feature guard instead of exercising the layout.
Compare sequential and lane-interleaved PFOR using the same encoded column shapes while holding delta selection fixed.
Add forced raw and delta benchmark variants so layout comparisons do not also measure a change in representation. Use the production encoder paths for both variants.
Exclude the generated AVX-512 unpacker from runtime dispatch because it builds vectors from scalar loads and uses out-of-line calls. Retain AVX2 as the highest dispatched target until the AVX-512 kernels use vector loads directly.
Build each transpose tile directly from unpacked registers and store it in file order, avoiding the intermediate 4 KiB grid. Apply the same structure to the lane-parallel delta path so unpacking, prefix sums, and permutation share one pass.
Run the four layout variants over five source and destination footprints, and add a store-only reference to distinguish compute effects from output-bandwidth limits.
Record the corpus and instruction-set constraints required to interpret the layout benchmarks, and identify which variants have compatible work and output order.
Compile baseline and vectorized kernel tables in separate translation units and select between them with Arrow's runtime dispatch. Parameterize the kernels by architecture so separately compiled instances remain distinct at link time.
Replace boxed headings and informal benchmark terminology, clarify the validity criteria, and centralize the shared benchmark corpus. No codec behavior changes.
Add a four-lane NEON transpose that writes file order directly instead of using the portable scratch-grid fallback. Verify every packed width with and without a frame bias.
Remove convenience constructors that prevented designated initialization and caused force_delta to be dropped at layout call sites. Pass complete option objects through the wrapper and benchmarks.
The ZSTD and LZ4 benchmarks dereferenced Codec::Create directly. For a codec the build did not enable that Result holds NotImplemented, and dereferencing it aborts the process, so one unbuilt compression dependency takes down every other benchmark registered in the same binary -- including all the PFOR ones, which need no compressor at all. With ARROW_WITH_BROTLI=OFF, Codec::Create returns "NotImplemented: Support for codec 'brotli' not built"; the same shape reaches ZSTD and LZ4 wherever those are off. The four call sites now go through a helper that reports the status through SkipWithError and returns, which is how arrow/ipc/read_write_benchmark.cc handles a facility it cannot get. Also drops BM_DbpGeom128x4. It was a second definition of the body already registered as BM_DbpAbFull, was never registered itself, and the comment above it now names BM_DbpAbFull as the 128/4 control instead.
Store the page's packing mode in VectorReader and pass it to each vector decode. Add a regression test covering multiple interleaved vectors and a sequential tail through the vector-at-a-time path.
Replace the informal term "ladder" with "shuffle sequence" in comments. No code changes.
prtkgaur
force-pushed
the
pgaur_interleavedPlusFastLanesDelta
branch
from
October 3, 2026 01:25
633ba32 to
199407c
Compare
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
Parquet uses a continuous bit-packed stream, while FastLanes uses a
lane-interleaved grid. This PR measures whether the grid's cheaper unpacking
can offset the cost of returning values in Parquet file order.
The answer depends on packed width, output element width, decode call shape,
memory footprint, instruction set, and the permutation back to file order.
This branch keeps those variables separate instead of reducing the comparison
to one throughput number.
What changes are included in this PR?
This benchmarking-only proof of concept adds:
Run
./layoutsinfl5_corpuswithout arguments to see the measurement index.Each study names the variable it changes and the assumptions required to
interpret its output.
Are these changes tested?
Yes. The codecs and experimental layouts are checked for bit-exact round trips
before they are timed. Unit tests cover packing widths, output order, delta
paths, tails, exceptions, and boundary values.
The benchmark harness includes timing controls and rejects results that do not
meet its validity checks. Results are recorded for multiple compilers,
instruction sets, and working-set sizes.
Are there any user-facing changes?
No production encoding or default writer behavior is proposed by this PR. It
is a benchmarking and research branch used to evaluate layout choices.
The experimental APIs and on-disk layouts in this branch should not be treated
as stable or interoperable formats.