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: |
prtkgaur
force-pushed
the
pfor-delta-encoding
branch
from
September 28, 2026 05:32
36395ec to
9174cff
Compare
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.
Make LoadHeader fallible: move the header-size check from Decode into LoadHeader, return Result<PforHeader>, and update Decode to use ARROW_ASSIGN_OR_RAISE. Mirrors the corresponding ALP review fix.
Replace std::memcpy / raw byte writes in PforWrapper::StoreHeader, LoadHeader, and the offset-array read/write paths with util::SafeLoadAs and util::SafeStore. Mirrors the corresponding ALP review fix.
Reject invalid packing_mode, value_byte_width mismatch, log_vector_size out of [kMin, kMax] range, and negative num_elements when loading the PFOR page header. Removes the redundant packing_mode and value_byte_width checks from Decode now that they live in LoadHeader. Mirrors the corresponding ALP review fix.
Replace size_t with int64_t for max_size/comp_size to match the PforWrapper API signature, and qualify pfor::PforWrapper as ::arrow::util::pfor::PforWrapper to avoid ADL ambiguity.
Aligns with Arrow buffer conventions (Buffer::data() returns uint8_t*). Removes the reinterpret_cast<char*> at the parquet encoder/decoder call sites and switches std::vector<char> compressed buffers to std::vector<uint8_t> in the unit test and benchmark. Also fixes a pre-existing size_t / int64_t* mismatch in pfor_benchmark.cc that surfaced once the buffer pointer type was tightened. Mirrors the corresponding ALP review fix.
Per Google C++ style, replace the PforVectorInfo struct with a class that has private trailing-underscore members and getter/setter accessors. Replace std::memcpy calls in Store/Load and the exception patch loop in DecodeVector with util::SafeLoadAs / util::SafeStore. Add bit_width range validation inside Load() so callers do not have to repeat the check. Caches num_exceptions() in a local in DecodeVector so the #pragma GCC unroll can still see a constant loop bound.
Per Google C++ style, both types become classes with private trailing-underscore members and const getters, mutable getters, and setters. Updates all access sites in pfor.cc (EncodeVector, LoadView, SerializedVectorSize, SerializeVector) and pfor_test.cc to go through the new accessors. Mirrors the corresponding ALP review fix.
- Move the num_exceptions < 0 check from DecodeVector into PforVectorInfo::Load alongside the bit_width range check, so all loaded-data invariants are enforced at the same layer. - Use PforVectorInfo's parameterized constructor in EncodeVector instead of three separate setter calls on a default-constructed instance.
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.
PforEncodedVectorView was reachable only from its own test and duplicated DecodeVector's validation. SerializeVector now rejects a vector whose info disagrees with the sections it carries.
An incompressible column encodes to its plain size plus the per-vector metadata, and only ColumnWriterImpl can relabel the page.
43 tests become 80. Ten cases that only ran at 32 bits now run at 64, which caught corrupt-page tests whose outlier the cost model declined to patch once an exception cost a 64-bit value.
A parquet page header counts nulls, and a PFOR page stores only the non-null values, so the level count the reader hands SetData is an upper bound on the values in the payload rather than the number of them. The decoder was using it as both: as the amount to decode and as the amount of decoded output to hand back, which for any nullable column decoded past the end of what the page actually holds. Read the count from the page header instead, where the encoder wrote it, and keep the level count only as the bound it is. Decoding a whole page in one call still writes straight into the caller's buffer; a partial read now decodes the page once into a pool-backed scratch buffer and serves the rest of the batch from it. DecodeArrow reserves once and then either advances over the values it just wrote or expands them leftward into their null positions, which drops the separate null-aware copy loop.
Two ways a malformed page got through. A header count short of the caller's capacity filled part of the output buffer and returned OK, leaving the rest of it holding whatever was there before; the count is now required to equal the expected value count, not merely fit inside it. And the offsets were each checked in isolation, only for being inside the buffer, so a page whose offsets overlap or run backwards decoded part-way and emitted values built out of another vector's bytes. Check the array as a chain up front instead: the first offset lands just past the array, each later one is strictly greater than the one before it, and all of them are inside the payload.
An all-null optional page buffers no values and is still written, and the empty payload the encoder emitted for it has no header for the reader to load, so reading such a column threw. Zero values now writes the header and nothing after it, and the wrapper accepts that count on both sides. The encoding test that covered this passed a level count of zero to SetData, which no real reader does -- the count it passes includes the nulls. It now passes the full count, and the writer tests round-trip an optional PFOR column with some nulls and with nothing but nulls.
Every multi-byte wire field -- the header's element count, the offset array, each vector's frame of reference and exception count, and the exception positions and values -- now converts explicitly, so the big-endian static_assert is gone. The bit-packed deltas already needed no conversion: BitWriter writes them little-endian and the unpacker reads them back the same way, so those bytes are copied verbatim.
The CMake build compiles them and runs pfor-test; meson knew about neither. The two translation units carry the only definitions of the PforWrapper and PforCompression instantiations parquet's encoder and decoder call, so a meson build has nothing to link them against.
A caller that reads a page in pieces decodes all of it up front, and the page layout can do better than that. Say so where the scratch buffer is, so the next reader of this code does not have to work it out.
Everything PFOR had so far drove the encoder and decoder classes directly, so nothing covered the path a real reader takes: writer properties, page headers, row group boundaries, null expansion. These 19 tests write real files and read them back, over the distributions the encoding is built for (a tight cluster far from zero, a constant vector, a cluster with outliers), the ones it has to survive (values at the bounds of each type, all-null and leading-null columns, row groups that end mid-vector, batches smaller than a page), and each supported compression codec. Every round trip also asserts the file's column chunks report PFOR. Without that a round trip passes just as happily when the writer picks some other encoding, and the test would be checking the default instead. arrow_reader_writer_test.cc is already long, so these go in arrow/arrow_encoding_test.cc, which later encodings can share.
The names now say what the install rules already do: arrow_install_all_headers globs a single directory and skips anything matching "internal", so nothing under arrow/util/pfor is installed and none of it carries a backward-compatibility obligation.
DecodeVector left UnpackOptions::max_read_bytes at its -1 default, which tells the bit-unpacker it may not read a byte past the vector's own packed payload. The vector kernels load a fixed-size window per step, wider than a step consumes at most bit widths, so under that bound the last step is refused and the tail of the vector falls to the scalar epilog -- 32 of 1024 values at bit width 3. The span DecodeVector is handed runs from this vector to the end of the page, so the true bound is already in hand; pass it. Whole-page int32 decode gains a median 1.13x over a 29-column corpus and 1.9x on the columns that pack to 3 bits, and nothing at widths 1, 2, 4, 8, 16 and 31, whose kernels strand no values to begin with. The gain is flat from a 78 KiB data page to a 4 MB destination, which is what a per-vector cost looks like.
The tests and benchmarks drew their section headings as boxes: a row of equals signs above the title and, in the benchmark and integration files, a second one below. Arrow writes a single rule of seventy dashes and then the title, in 121 places under cpp/src; boxed headings appear in seven files, four of them vendored. This converts twenty-nine headings to the house rule and drops the twelve closing rules. Two comments also shouted: three spellings of "ARE the signed values" in the decoder, and the bias comment in the bit-packing kernel announcing "The whole point:" before making it. One more narrated the branch's own history, calling the second add-FOR pass what the code "used to do". The cost that pass measured is why the add is folded into the unpacker, so the figures stay and the history goes. Comments only. Stripping comments leaves all six files byte-identical to their previous revision, clang-format reports no new warnings, and no line crosses ninety columns.
A caller that read a page in batches decoded the whole page into decoded_values_ on the first batch and was served by memcpy from it afterwards, which cost a page-sized allocation to answer a batch-sized request. The page layout is already addressed by vector: the offset array locates any vector in constant time, so a batched read can decode only the vectors it needs. PforWrapper gains VectorReader, a validated view over an encoded page that holds no decoded values. Open() checks the header and offset array once; VectorLength() and DecodeVector() then serve any vector index. The decoder keeps a one-vector cache instead of a whole-page buffer, so a batched read allocates one vector and a sequential read reuses that same buffer across the page. This replaces the deferred-decode TODOs on both the encoder and decoder paths. Page consumption is now tracked explicitly: levels_remaining_ counts down across Decode and DecodeSpaced, including nulls, and CheckPageConsumed() verifies the page ends where the level count says it should. DecodeSpaced is overridden for that accounting. SetData and Decode now reject negative arguments rather than relying on the callers to pass sane ones, and the page and header layouts are written out as tables in the wrapper header.
PFOR has always used the minimum as its frame of reference, which makes every exception an overshoot: one value far below the cluster drags the packed window down and nothing can patch it back. And it has only ever packed values, so a column whose values are spread but whose steps are small had no representation that fit it. Both are encoder-side choices, so the cost model now makes them per vector: whether to difference the values first, and where to put the frame. The frame becomes any lower bound rather than the lowest, which costs nothing on the wire -- the field already holds a full-width value and the decoder only ever adds it -- and lets a value below the window fail the same unsigned mask test as one above it, so patching works on both sides with no second test and no sign to track. The delta flag is a bit of its own in the bit-width byte rather than another code in a mode field, because differencing is orthogonal to how the payload is laid out and the two have to be able to combine. A delta vector carries one extra full-width field, its first value, which is what keeps it decodable without the vector before it -- storing the first value as an exception instead would cost more (a position and a value) and only conditionally. Measured against the previous encoder over 18 distributions at n=102400, int64: the sawtooth packs 16.6x smaller, monotonic ids with gaps 5.4x, sorted keys 3.9x, event timestamps 2.8x, a random walk 1.63x, and a sampled continuous signal 1.48x. Nine distributions are untouched, and none regressed. Decode costs 2.6x where a vector is differenced, from the running sum, and is unchanged elsewhere. Encode costs 2-3x across the board, which is the searches, and is the next thing to fix. The decision logic lives in its own header, free of Arrow, so it can be exercised on its own without going through a page or a buffer.
The frame search left encode at 2-3x its old cost, all of it in two extra
traversals per vector. Both are now conditional or fused:
- The bit-width histogram and the bucket counts are gathered in one walk.
Each needs the same offset, and computing it twice was the larger half
of the search.
- A constant vector returns from min/max alone. Equal values all land in
one histogram bin, where the read-modify-write serializes, so this case
was running at a fraction of the usual rate for an answer that is
already at the floor. Constant now encodes 1.5x (int64) to 3.3x (int32)
faster than it did before the search existed.
- The scan is seeded with the minimum-frame cost, so a window has to beat
the incumbent to register. On the columns a frame cannot help -- 13 of
18 here -- everything after the scan is skipped.
- ComputeDeltas returns the bounds of what it wrote, so the differences
are not walked again to find their range.
The scan picks a bucket, and bucket boundaries stand 2^shift apart, which
on a wide column is thousands. A cluster sitting just above a boundary was
paying those bits for nothing, so the winning window is now walked once to
lower the frame onto the smallest value it actually covers. Per-bucket
minima kept inside the counting pass would avoid the walk but cost every
vector a compare and a store per element, including the vectors that
discard them -- about 30% of encode throughput to improve one column.
Behind the seeded scan, only a vector the search has already won pays.
Against the previous commit: sensor_dropouts is 1.20x smaller and
tcp_sawtooth 1.04-1.07x, no column grows, and encode moves within +-10%
-- faster where the scan now bails early, slower on the five columns whose
refinement walk buys those bits.
pfor_benchmark.cc gains the ten distributions this work was measured on.
The ones already there are either unordered or perfectly regular, so none
of them separates a plan that differences from one that does not, and none
puts the frame anywhere but the minimum. pfor_test.cc gains eleven tests
covering the delta mode, the two-sided frame, and the width/flag packing.
prtkgaur
force-pushed
the
pfor-delta-encoding
branch
from
October 2, 2026 15:17
3b7bef6 to
af6bbd6
Compare
The comparison benchmark's generators are all either unordered or perfectly regular, so none of them separates an encoding that differences neighbouring values from one that packs them, and none of them puts a frame of reference anywhere but the minimum. That left PFOR's delta mode and DELTA_BINARY_PACKED untested against each other on the columns where they actually make different choices. Register the ten shapes from pfor_benchmark.cc here too, at both widths: timestamps regular and bursty, a sawtooth, bounded-rate series, monotonic ids with and without gaps, a low sentinel below the cluster, and two clusters no single window covers. These are templates rather than a pair of per-width functions, so both widths are drawn from the same distribution by construction. They sit in their own namespace because two of them build on base distributions whose names this file already uses for columns drawn from different seeds; the figures are therefore comparable with pfor_benchmark.cc. Benchmark count goes from 476 to 756 (20 datasets x 14 codecs).
The delta mode was costed by writing every difference out and running a frame search over them, which is most of what encoding a vector costs, and it was charged on every vector including the ones that went on to decline it. Estimate first from a strided sample of the differences and drop the mode there when the estimate cannot reach the incumbent, so a vector that will not use it pays a fraction of a pass instead of two full ones. The estimate samples widths rather than a span. A gate on the span of the differences was tried first and had to go: a sawtooth is a tight cluster of small positive differences with a handful of large negative ones, so its span is as wide as its raw span while its cost is a fraction of it. Zigzagging is what lets a histogram stand in for a search that has not run, since differences in [-k, k] zigzag into the same [0, 2k] a frame at -k would produce. On 20 distributions at both widths, encode throughput on the vectors that decline the mode is 1.55-1.93x what it was, and the vectors that accept it pay 1.4-5.2%. Two earlier shapes were measured and dropped: accumulating the histogram over every difference inside the differencing walk costs 6-27% on accepting vectors, and moving it to a pass of its own changes that by under 3%, so the cost is the histogram work and not a lost vectorization. A test pins the estimate against the ungated chooser -- same mode, width, frame and cost -- over every distribution the benchmark covers, so an estimate that turns pessimistic shows up as a lost delta rather than as a silent ratio regression.
PFOR is a Preview feature in the Parquet format, so a writer must not emit it unless the user has asked for it. Selecting Encoding::PFOR without calling enable_pfor_encoding() now makes WriterProperties::Builder::build() throw, naming whichever column asked for it; the Builder's constructor is the only way to reach a WriterProperties, so the check cannot be bypassed. Decoding is unaffected -- a file that exists is always readable. The delta mode gets its own property, on by default. It is part of PFOR rather than a separate encoding: every reader has to handle it, because each vector says in its own header which mode it used, so a page written with the mode disabled reads back through the same decoder. The property exists because differencing costs encode time on vectors it then declines, so a writer that knows its data is not sequential can skip the search. The option travels as a PforEncodeOptions struct rather than a bare bool, so a later mode does not change these signatures again. MakeEncoder and MakeTypedEncoder take the WriterProperties, which is how the per-column delta setting reaches the encoder; a caller with no properties to hand gets the defaults. Tests: 14 property tests covering the opt-in, the per-column overrides and the interaction between the global and per-column maps; and an end-to-end test that writes an arithmetic run twice and requires the column chunk to be smaller with the mode left on. A negative control -- the encoder factory ignoring the property -- makes that end-to-end test fail with both sizes equal, so it does reach the encoder rather than stopping at build().
Two cases the corruption tests did not reach, both specific to the flag in bit 7 of the bit-width byte. Setting the flag on a vector that was written without one makes the decoder consume sizeof(T) extra bytes of metadata. On a page sized exactly, as the encoder writes it, that pushes the vector past the end of the buffer and the decode has to fail rather than read on. Truncating a page inside a delta vector's start value leaves enough bytes for the info block and not enough for the vector. The bound that catches this is the one place a length check reads is_delta(), and it sits before the start value is loaded -- so without it the load runs sizeof(T) bytes past the end and only the later payload bound reports the problem. Removing it as a negative control leaves the test failing on the error message, which is all a non-sanitizer build can see of an overread; the assertion is on the message for that reason. Store also asserts, in debug builds, that the width it is about to pack fits the seven bits it has. A wider one would lose its high bits to the mask and set the delta flag on the way out, which is the failure the seven-bit mask was introduced to prevent -- Load rejects it on the way in, and this reports it at the encoder instead.
The sum is one dependent add per value and uses no lanes, which is why a delta vector does not pick up the speed a narrower type gives the unpack. A scan across lanes plus a carry broadcast would shorten the chain, but only for a vector with no exceptions: patching sits between the frame add and the sum, so a vector holding even one exception keeps this loop.
The sequential unpacker's read bound is the distance from the packed payload to the end of the page span. It was computed by subtracting the fixed metadata size, which stops being right once a vector can be a delta vector: that header carries a start value as well, and the read pointer is already past it. Subtract info_bytes -- what this vector's header actually wrote -- instead. Taking the fixed size would offer the kernels sizeof(T) bytes past the end of the caller's buffer, and because those kernels only widen their load window when it fits, it would do so on exactly the vectors whose last step would otherwise be refused.
The earlier sweep converted the boxed section headings and the
announce-then-explain comments across the PFOR sources. Three comments
written since it ran carry the same shapes: the frame search states its
own point before making it ("the whole point of a frame above the
minimum"), a test heading is drawn as a box rather than the house rule
of seventy dashes, and the delta-shape generator calls its two element
widths arms.
Comments only. Stripping comments leaves all three files byte-identical
to their previous revision, clang-format reports no new warnings, and no
line crosses ninety columns.
Comment-only; the file is byte-identical once comments are stripped. The comment above the low-outlier test called frame=min "the old frame", which reads as a previous revision of this code rather than what it is: the classic PFOR frame choice this test exists to contrast against. Naming it directly also makes the following clause redundant, since "at the minimum" was carrying the same information.
The planner costs the raw and differenced representations per vector and keeps the cheaper one, so on most columns the delta path cannot be reached from the public API at all. That makes the two representations impossible to compare on the same data, and leaves the delta encode and decode paths exercised only by inputs that happen to select them. force_delta skips the three places the planner declines -- the short and zero-width shortcut, the sampled estimate, and the final cost compare -- and commits to the delta plan. It is off by default and does not change what the planner chooses on its own; the serialized vector is a normal delta vector that any decoder reads. The comparison benchmark gains raw-only and delta-only encode and decode registrations on top of the planner-chosen ones, so a column can be measured in both representations. pfor_test.cc covers the round trip under the flag.
Consecutive miniblocks sharing a bit width are packed back to back with no padding and hold a multiple of 32 values, so a run of them is bit-identical to one longer run at that width and can be unpacked in a single call. A miniblock joins the run only when its stored width equals the current one, which InitMiniBlock has already validated, so an unchecked width is never used. The run stops at the end of the block and at what the caller has room for. The prefix-sum loop also keeps the running value and the frame of reference in locals. Both have the same type as the caller's output buffer, so the compiler cannot prove the store does not alias them and otherwise reloads both for every value. Arithmetic is unchanged: every term stays in the unsigned type, so the wrapping the format specifies is preserved and no decoded value changes.
The value-at-a-time prefix sum becomes a helper that scans whole registers with a log-step inclusive scan and finishes the remainder one value at a time. Adding the frame of reference before the scan turns its running multiple into a term the scan produces rather than a multiply per lane, and the running total is carried between registers in a vector register rather than through a general-purpose one. The xsimd include and the vector loop are guarded on ARROW_HAVE_NEON or ARROW_HAVE_SSE4_2, so a build with neither compiles the scalar loop alone. The vector loop is compiled only where a register holds four or more values, which at the 128-bit baseline vectorizes 32-bit values and leaves 64-bit ones on the value-at-a-time loop. Also adds a decode benchmark on non-decreasing values, the shape this encoding is usually chosen for. Arithmetic is unchanged: every term stays in the unsigned type, so the wrapping the format specifies is preserved and no decoded value changes.
prtkgaur
force-pushed
the
pfor-delta-encoding
branch
from
October 2, 2026 16:04
af6bbd6 to
e7f5049
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
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
Patched Frame of Reference (PFOR) stores most integers at a common bit width
and records values that do not fit as exceptions. This branch explores PFOR
for Parquet integer columns, including an optional delta mode for locally
correlated values.
The PR also compares the patched-delta payload with Parquet's existing
DELTA_BINARY_PACKEDlayout. The results separate format-level differencesfrom decoder implementation costs.
What changes are included in this PR?
This experimental PR adds:
INT32andINT64.DELTA_BINARY_PACKEDpayloads.The layout study and its measurements are documented in
pfor_delta_layout_report.md.Are these changes tested?
Yes. Tests cover
INT32andINT64round trips, raw and delta modes,exceptions, boundary values, corrupt metadata, writer-property selection,
and Parquet integration.
The benchmark corpus includes timestamp-like values, counters, random walks,
outliers, and generated TPC-style column shapes. The checked-in report records
the method, results, and limitations of the delta-layout comparison.
Are there any user-facing changes?
This is an experimental implementation. It adds writer properties for opting
into PFOR and its delta mode, plus reader support for the corresponding pages.
Neither mode is selected by default.
The encoding is not yet part of the Parquet format specification and should
not be treated as a stable interchange format.