perf(reader): untranspose fastlanes.delta straight into the output window - #348
Merged
Conversation
…ndow Takes the better half of #343, which fixed #338 independently and in parallel — I merged #345 for the same issue without checking for an open PR first, so this reconciles the two rather than discarding one. From #343: - scatterChunk writes each untransposed value straight to its output index, dropping the chunk-sized `untransposed` buffer #345 staged it in and the separate pass that sliced it. The leading chunk of an offset-sliced array maps to a negative index and the trailing chunk runs past the row count; one `Long.compareUnsigned` covers both, since a negative index reads as a huge unsigned value. The stores are a permutation scatter and never vectorize regardless, so the compare costs nothing the untranspose was not already paying. - The returned segment is read-only. - Element-indexed `getAtIndex` instead of hand-computed byte offsets. - Reader-module tests that build the delta wire form directly, mirroring DeltaEncodingEncoder's layout. These cover offset slicing where it belongs — the writer is not on the reader's test classpath and never emits a non-zero offset, so #345 had to reach into the integration module to cover the same shape. Kept from #345: - Only chunks overlapping the row window are reconstructed. #343 walked every chunk and discarded the out-of-window stores per element, so a one-chunk slice of a thousand-chunk column did a thousand chunks of work. - The metadata range guard. Without it a `deltas_len` of Long.MAX_VALUE drives the chunk loop ~9e15 times — a hang, which is worse than the OutOfMemoryError it replaced. Closes #343. Co-Authored-By: Davide Angelocola <davide.angelocola@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Reconciles #343 with #345, both of which fixed #338 independently. I merged #345 without checking for an open PR on the issue first — that's on me, and this PR exists so #343's work isn't lost rather than because #345 needed changing.
Neither implementation dominated the other, so this takes both halves.
From #343
scatterChunkwrites each untransposed value straight to its output index. That removes the chunk-sizeduntransposedstaging buffer fix(reader): decode fastlanes.delta into the arena, not four heap long[] #345 kept, and the separate pass that sliced it. The bounds handling is the nice part: the leading chunk of an offset-sliced array maps to a negative output index and the trailing chunk runs pastrowCount, and a singleLong.compareUnsigned(at, rowCount) < 0covers both, because a negative index reads as a huge unsigned value. The stores are a permutation scatter and never vectorize regardless, so the compare costs nothing the untranspose wasn't already paying.out.asReadOnly()on the returned segment.getAtIndexinstead of hand-computed byte offsets.DeltaEncodingEncoder's transpose-then-per-lane-delta layout. This is the better call: the writer isn't on the reader's test classpath and never emits a non-zerooffset, so fix(reader): decode fastlanes.delta into the arena, not four heap long[] #345 had to reach into the integration module to cover offset slicing. These cover it where it belongs, across offsets 1, 7, 1023, 1024, 1025 and 2000 — i.e. every relationship a window can have to a chunk boundary.Kept from #345
numChunksand discarded out-of-window stores per element; a one-chunk slice of a thousand-chunk column did a thousand chunks of work. WithfirstChunk/lastChunkit does one. The two mechanisms compose — the scatter's compare still handles the partial first and last chunk.NegativeArraySizeExceptionpath, but leftnumChunks = (int)(deltasLen / CHUNK)unguarded: adeltas_lenofLong.MAX_VALUEdrives the loop ~9×10¹⁵ times. That's a hang, which is worse than theOutOfMemoryErrorit replaced.Tests
Both suites, ten tests total: #343's wire-form round trips (all eight widths, chunk boundaries, offset slicing, short row counts) plus #345's malformed-metadata cases. The integration round-trip through the real file format stays as-is.
./mvnw verifygreen across all 17 modules.Closes #343.
🤖 Generated with Claude Code