Skip to content

perf(reader): decode fastlanes.delta without heap staging - #343

Closed
dfa1 wants to merge 1 commit into
mainfrom
fix/338-delta-heap-arrays
Closed

perf(reader): decode fastlanes.delta without heap staging#343
dfa1 wants to merge 1 commit into
mainfrom
fix/338-delta-heap-arrays

Conversation

@dfa1

@dfa1 dfa1 commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Closes #338.

Problem

DeltaEncodingDecoder routed the whole column through four row-scaled heap long[] arrays before writing a single arena segment: basesAll and deltasAll copied out of their segments, a full-length decoded, and a result slice of it.

Every value was widened to 8 bytes regardless of ptype, so an I8 delta column allocated 8× its natural width on the GC heap, three times over — CLAUDE.md's allocation rule violated four times at row scale. decodedresult was a pure arraycopy duplicate whose only effect was dropping offset leading elements.

Change

Values are reconstructed straight into one arena segment at the column's own width. The untranspose and the window shift happen in the same step, so both staging arrays disappear. What remains is fixed-size per-chunk scratch (bases, deltas, undelta) — cache-resident and reused across chunks.

Delta genuinely has to reconstruct values (each depends on its predecessor), so unlike the dict/runend/sequence work in #334#337 there is no lazy carrier to return. This is purely about not staging the reconstruction on the heap.

Hot loops

Both anti-patterns in the old readLongs are gone:

  • The per-element switch (ptype) is hoisted out of every loop.
  • The per-element i % cap is branch-split away. readDirect handles the case where the child covers the range, with no division at all. The broadcast path an undersized (ConstantEncoding) child still needs is strength-reduced to a rolling index — exactly one % before the loop, then a compare-and-reset.

Reading the broadcast cycle into a scratch array instead would reintroduce a cap-sized heap allocation, which is the thing this removes. Modulo in these loops is the repeated cause of the 5–10× regressions recorded in CLAUDE.md (ed658b7051a794442021f), so: none survives on the hot path.

The scatter keeps one unsigned bounds compare per element. It folds the leading chunk's negative output index and the trailing chunk's overrun into a single test, and the stores are a transposeIndex permutation that never vectorizes regardless — so it costs nothing the untranspose was not already paying.

Tests

Coverage was thin: four tests, none touching offset or a chunk boundary — precisely what this rewrite changes. Added round-trips over all eight integer ptypes, across chunk boundaries, over six offsets, and with a window shorter than the decoded length.

The wire form is built in the test by mirroring DeltaEncodingEncoder, since the writer module is not on the reader's test classpath and the encoder never emits a non-zero offset — so that shape is otherwise untestable from here.

I verified the offset test actually bites by removing the window shift and watching it fail, then restoring.

./mvnw verify green, including the Rust interop oracles.

What I did not do

Not benchmarked. There is no delta JMH harness and adding one felt out of scope. The allocation removal is unambiguous; the loop changes follow the documented rule rather than a measurement. Happy to add a benchmark if you want the number before merging.

🤖 Generated with Claude Code

DeltaEncodingDecoder routed the whole column through four row-scaled
heap long[] arrays before writing a single arena segment: basesAll and
deltasAll copied out of their segments, a full-length `decoded`, and a
`result` slice of it. Every value was widened to 8 bytes regardless of
ptype, so an I8 delta column allocated 8x its natural width on the GC
heap, three times over — CLAUDE.md's allocation rule violated four times
at row scale. `decoded` -> `result` was a pure arraycopy duplicate whose
only effect was dropping `offset` leading elements.

Values now go straight into one arena segment at the column's own width.
The untranspose and the window shift happen in the same step, so both
staging arrays disappear; what remains is fixed-size per-chunk scratch
(bases, deltas, undelta), which is cache-resident and reused.

Delta has to reconstruct values — each depends on its predecessor — so
unlike the dict/runend/sequence work there is no lazy carrier to return
here. This is purely about not staging the reconstruction on the heap.

Both hot-loop anti-patterns in the old readLongs are gone:

- The per-element `switch (ptype)` is hoisted out of every loop.
- The per-element `i % cap` is branch-split away: readDirect handles the
  case where the child covers the range, with no division at all. The
  broadcast path an undersized (ConstantEncoding) child still needs is
  strength-reduced to a rolling index — exactly one `%` before the loop,
  then a compare-and-reset. Reading the cycle into scratch instead would
  reintroduce a cap-sized heap allocation, which is what this removes.
  Modulo in these loops is the repeated cause of 5-10x regressions here
  (ed658b7 -> 051a794 -> 442021f), so it is worth stating that none
  survives on the hot path.

The scatter keeps one unsigned bounds compare per element: it folds the
leading chunk's negative output index and the trailing chunk's overrun
into a single test, and the stores are a transposeIndex permutation that
never vectorizes regardless, so it costs nothing the untranspose was not
already paying.

Coverage was thin — four tests, none touching `offset` or a chunk
boundary, which is precisely what this rewrite changes. Adds round-trips
over all eight integer ptypes, across chunk boundaries, over six offsets,
and with a window shorter than the decoded length. The wire form is built
in the test by mirroring DeltaEncodingEncoder, since the writer module is
not on the reader's test classpath and the encoder never emits a non-zero
offset. Verified the offset test fails when the window shift is removed.

Not benchmarked: there is no delta JMH harness, and adding one is out of
scope here. The allocation removal is unambiguous; the loop changes
follow the documented rule rather than a measurement.

Closes #338

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dfa1

dfa1 commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #348, which takes this branch's better half rather than dropping it.

I need to own the sequencing here: I fixed #338 in #345 and merged it without checking whether an open PR already existed for the issue. This branch was that PR. Rebasing it now isn't mechanical — both commits rewrote DeltaEncodingDecoder end to end, so the rebase conflicts in six hunks and 'resolving' it means choosing an implementation, not merging text.

Comparing the two properly, neither dominated:

This branch was better on:

#345 was better on:

  • Only reconstructing chunks that overlap the row window. This branch walks every chunk and discards out-of-window stores per element, so a one-chunk slice of a thousand-chunk column does a thousand chunks of work.
  • The untrusted-metadata range guard. Dropping the heap arrays removed the NegativeArraySizeException, but numChunks = (int)(deltasLen / CHUNK) stayed unguarded — a deltas_len of Long.MAX_VALUE drives the loop ~9e15 times, which is a hang rather than the OOM it replaced.

#348 carries all four of this branch's advantages plus both of #345's, with your commit credited via Co-Authored-By. Closing here rather than force-pushing a reconciliation onto a branch whose PR number no longer matches what landed — say the word if you'd rather it land under this number instead and I'll move it.

@dfa1 dfa1 closed this in #348 Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DeltaEncodingDecoder routes decode through four row-scaled heap long[] arrays

1 participant