Skip to content

Commit 4d38299

Browse files
dfa1claude
andcommitted
feat(BitpackedCodec): apply patches on decode (TODO #7a step 3)
Per the FastLanes BitPacked spec, BitPackedMetadata carries an optional patches PatchesMetadata field (tag 3) and the array exposes two child slots — slot 0 for patch indices (unsigned, ptype from PatchesMetadata.indices_ptype), slot 1 for patch values (parent dtype). Until now the Java decoder ignored both: any column the Rust writer encoded with bitpacked-plus-patches lost its outlier values. encodings.proto: add `optional PatchesMetadata patches = 3` to BitPackedMetadata. The field was previously dropped silently because protobuf treats unknown fields as opaque. BitpackedCodec.decode: when `meta.hasPatches()`, decode child 0 as the indices array (its own ptype, rowCount = patches.len), decode child 1 as the values array (parent dtype, rowCount = patches.len), and overwrite `output[absIdx - offset]` with the value bytes for each patch. Slot ordering verified against spiraldb/vortex `encodings/fastlanes/src/bitpacking/array/mod.rs` (BitPackedSlots: patch_indices=0, patch_values=1). BitpackedCodecPatchesTest: bit-pack [10,20,30,40,50] via the production encoder (bitWidth=6), attach synthetic PatchesMetadata that rewrites indices {1,3} with values {777,999}, and assert the decoded buffer is [10,777,30,999,50]. TODO.md: move #7a to Done. Trim #7 to pcodec — bitpacked, delta, for, sparse, alp, dict, fsst, sequence, varbin, constant, and runend all landed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 347bc34 commit 4d38299

4 files changed

Lines changed: 150 additions & 23 deletions

File tree

TODO.md

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111
- Writer embeds min/max per flat segment in `ArrayNode.stats` (Protobuf ScalarValue)
1212
- Reader peeks stats from segment FlatBuffer tail, skips chunks excluded by `RowFilter`
1313
- Supports `Gte`, `Lte`, `Eq`, `And` predicates on I8/I16/I32/I64, U8/U16/U32/U64, F32/F64
14+
- [x] **#7a `fastlanes.bitpacked` — spec-compliant rewrite** (all 5 steps incl. patches)
15+
- Protobuf `BitPackedMetadata` (tags 1+2+3); unified FastLanes `unpack` for I8/I16/I32/I64 (signed + unsigned);
16+
patches decoded from children slots 0 (indices) + 1 (values), overwritten by absolute index.
17+
- Encoder writes spec-compliant protobuf metadata; round-trip + patch decode covered by tests.
1418
- [x] **#7b `fastlanes.for` decoder** (reference + bitpacked residuals)
1519
- [x] **#7c `vortex.sparse` decoder** (fill value + patches)
1620
- [x] **#7d `vortex.alp` decoder** (ALP inverse + patches)
@@ -24,29 +28,8 @@
2428
## Open
2529

2630
- [ ] **#7 Additional encodings**
27-
- `fastlanes.bitpacked` — integer bit-packing (steps 1/2/4/5 done; step 3 patches still open, see #7a)
28-
- `fastlanes.delta` — delta encoding for monotonic sequences
29-
- `dict` — dictionary encoding for low-cardinality columns
30-
- `pcodec` — float compression
31-
- `fsst` — string compression (in-flight, see `FsstCodec.java`)
32-
33-
- [ ] **#7a Fix `fastlanes.bitpacked` — spec-compliant rewrite**
34-
- Root cause: current code guesses format by metadata byte size (9 = Java, 2 = JNI). Wrong.
35-
The spec always uses protobuf metadata regardless of writer origin.
36-
- **Spec** (from `encodings/fastlanes/src/bitpacking/vtable/mod.rs`):
37-
- Metadata: protobuf `BitPackedMetadata``bit_width u32` (tag 1), `offset u32` (tag 2, 0≤offset<1024),
38-
`patches PatchesMetadata` (tag 3, optional)
39-
- Buffer size: `ceil((len + offset) / 1024) * 128 * bit_width` bytes
40-
- FastLanes block layout: `LANES = 1024 / T` (T = element bit-width); `FL_ORDER = [0,4,2,6,1,5,3,7]`; logical
41-
index for `(row, lane)` = `FL_ORDER[row/8]*16 + (row%8)*128 + lane` — same formula for all types
42-
- Step 1: delete `decodeJni()` and `decodeJava()`; parse metadata as protobuf (tags 1+2)
43-
- Step 2: implement single unified `unpack(buf, bitWidth, offset, T, rowCount) → long[]` using the FastLanes
44-
algorithm above
45-
- Step 3: handle patches — decode child slots (indices + values), overwrite output at patch indices (**missing**; steps 1,2,4,5 done)
46-
- Step 4: align `encode()` to write protobuf metadata (tags 1+2) instead of the 9-byte custom format
47-
- Step 5: update `BitpackedCodecTest` for round-trip with spec-compliant metadata
48-
- Reference: `spiraldb/vortex` `encodings/fastlanes/src/bitpacking/`, `spiraldb/fastlanes-rs` `src/bitpacking.rs` +
49-
`src/macros.rs`
31+
- `pcodec` — float compression (only remaining gap; bitpacked, delta, for, sparse, alp, dict, fsst, sequence,
32+
varbin, constant, runend all landed)
5033

5134
## Performance
5235

core/src/main/java/io/github/dfa1/vortex/encoding/BitpackedCodec.java

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.github.dfa1.vortex.encoding;
22

33
import com.google.protobuf.InvalidProtocolBufferException;
4+
import dev.vortex.proto.DTypeProtos;
45
import dev.vortex.proto.EncodingProtos;
56
import dev.vortex.proto.ScalarProtos;
67
import io.github.dfa1.vortex.core.Array;
@@ -578,7 +579,61 @@ public Array decode(DecodeContext ctx) {
578579
MemorySegment output = ctx.arena().allocate(rowCount * ptype.byteSize());
579580
fastlanesUnpackToSeg(packed, bitWidth, offset, typeBits, rowCount, output);
580581

582+
if (meta.hasPatches()) {
583+
applyPatches(ctx, meta.getPatches(), output, ptype.byteSize());
584+
}
585+
581586
return new Array(ctx.dtype(), rowCount,
582587
new MemorySegment[]{output.asReadOnly()}, Array.NO_CHILDREN, ArrayStats.empty());
583588
}
589+
590+
// ── Patches ───────────────────────────────────────────────────────────────
591+
592+
private static Array decodeChildAs(DecodeContext parent, int childIdx, DType dtype, long rowCount) {
593+
ArrayNode childNode = parent.node().children()[childIdx];
594+
DecodeContext childCtx = new DecodeContext(
595+
childNode, dtype, rowCount, parent.segmentBuffers(), parent.registry(), parent.arena());
596+
return parent.registry().decode(childCtx);
597+
}
598+
599+
private static long readUnsignedIdx(MemorySegment seg, long i, PType ptype) {
600+
return switch (ptype) {
601+
case U8 -> Byte.toUnsignedLong(seg.get(ValueLayout.JAVA_BYTE, i));
602+
case U16 -> Short.toUnsignedLong(seg.get(LE_SHORT, i * 2));
603+
case U32 -> Integer.toUnsignedLong(seg.get(LE_INT, i * 4));
604+
case U64 -> seg.get(LE_LONG, i * 8);
605+
default -> throw new IllegalStateException(
606+
"fastlanes.bitpacked: non-unsigned patch index ptype " + ptype);
607+
};
608+
}
609+
610+
private static PType ptypeFromProto(DTypeProtos.PType proto) {
611+
return PType.values()[proto.getNumber()];
612+
}
613+
614+
private static void applyPatches(DecodeContext ctx, EncodingProtos.PatchesMetadata pm,
615+
MemorySegment out, int elemBytes) {
616+
long numPatches = pm.getLen();
617+
if (numPatches == 0) {
618+
return;
619+
}
620+
long offset = pm.getOffset();
621+
PType idxPtype = ptypeFromProto(pm.getIndicesPtype());
622+
623+
Array idxArr = decodeChildAs(ctx, 0, new DType.Primitive(idxPtype, false), numPatches);
624+
Array valArr = decodeChildAs(ctx, 1, ctx.dtype(), numPatches);
625+
626+
MemorySegment idxSeg = idxArr.buffer(0);
627+
MemorySegment valSeg = valArr.buffer(0);
628+
629+
long n = ctx.rowCount();
630+
for (long i = 0; i < numPatches; i++) {
631+
long absIdx = readUnsignedIdx(idxSeg, i, idxPtype) - offset;
632+
if (absIdx < 0 || absIdx >= n) {
633+
throw new IllegalStateException(
634+
"fastlanes.bitpacked: patch index " + absIdx + " out of range [0," + n + ")");
635+
}
636+
MemorySegment.copy(valSeg, i * elemBytes, out, absIdx * elemBytes, elemBytes);
637+
}
638+
}
584639
}

core/src/main/proto/encodings.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ message ALPMetadata {
4444
message BitPackedMetadata {
4545
uint32 bit_width = 1;
4646
uint32 offset = 2;
47+
optional PatchesMetadata patches = 3;
4748
}
4849

4950
message SequenceMetadata {
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package io.github.dfa1.vortex.encoding;
2+
3+
import dev.vortex.proto.DTypeProtos;
4+
import dev.vortex.proto.EncodingProtos;
5+
import io.github.dfa1.vortex.core.Array;
6+
import io.github.dfa1.vortex.core.ArrayStats;
7+
import io.github.dfa1.vortex.core.DType;
8+
import io.github.dfa1.vortex.core.PType;
9+
import org.junit.jupiter.api.Test;
10+
11+
import java.lang.foreign.Arena;
12+
import java.lang.foreign.MemorySegment;
13+
import java.lang.foreign.ValueLayout;
14+
import java.nio.ByteBuffer;
15+
import java.nio.ByteOrder;
16+
17+
import static org.assertj.core.api.Assertions.assertThat;
18+
19+
class BitpackedCodecPatchesTest {
20+
21+
private static final DType I32_DTYPE = new DType.Primitive(PType.I32, false);
22+
private static final ValueLayout.OfInt LE_INT = ValueLayout.JAVA_INT_UNALIGNED.withOrder(ByteOrder.LITTLE_ENDIAN);
23+
24+
@Test
25+
void decode_appliesPatches_overridingBitPackedValues() {
26+
// Given — bit-pack [10, 20, 30, 40, 50] via the production encoder (bitWidth = 6),
27+
// then attach synthetic patches metadata that rewrites indices [1, 3] with [777, 999].
28+
int[] base = {10, 20, 30, 40, 50};
29+
BitpackedCodec sut = new BitpackedCodec();
30+
EncodeResult packed = sut.encode(I32_DTYPE, base);
31+
32+
ByteBuffer packedBuf = packed.buffers().get(0);
33+
byte[] packedBytes = new byte[packedBuf.remaining()];
34+
packedBuf.duplicate().get(packedBytes);
35+
36+
// Build new BitPackedMetadata that re-uses the packed bytes but advertises patches.
37+
EncodingProtos.PatchesMetadata patches = EncodingProtos.PatchesMetadata.newBuilder()
38+
.setLen(2)
39+
.setOffset(0)
40+
.setIndicesPtype(DTypeProtos.PType.U32)
41+
.build();
42+
byte[] metaBytes = EncodingProtos.BitPackedMetadata.newBuilder()
43+
.setBitWidth(6) // matches encoder's choice for max=50
44+
.setOffset(0)
45+
.setPatches(patches)
46+
.build()
47+
.toByteArray();
48+
49+
byte[] idxBuf = new byte[2 * 4];
50+
ByteBuffer.wrap(idxBuf).order(ByteOrder.LITTLE_ENDIAN).putInt(1).putInt(3);
51+
byte[] valBuf = new byte[2 * 4];
52+
ByteBuffer.wrap(valBuf).order(ByteOrder.LITTLE_ENDIAN).putInt(777).putInt(999);
53+
54+
ArrayNode idxNode = new ArrayNode(CodecId.VORTEX_PRIMITIVE, null,
55+
new ArrayNode[0], new int[]{1}, ArrayStats.empty());
56+
ArrayNode valNode = new ArrayNode(CodecId.VORTEX_PRIMITIVE, null,
57+
new ArrayNode[0], new int[]{2}, ArrayStats.empty());
58+
ArrayNode bpNode = new ArrayNode(CodecId.FASTLANES_BITPACKED,
59+
ByteBuffer.wrap(metaBytes),
60+
new ArrayNode[]{idxNode, valNode},
61+
new int[]{0},
62+
ArrayStats.empty());
63+
64+
MemorySegment[] segments = {
65+
MemorySegment.ofArray(packedBytes),
66+
MemorySegment.ofArray(idxBuf),
67+
MemorySegment.ofArray(valBuf)
68+
};
69+
70+
CodecRegistry registry = CodecRegistry.empty();
71+
registry.register(new BitpackedCodec());
72+
registry.register(new PrimitiveCodec());
73+
74+
DecodeContext ctx = new DecodeContext(
75+
bpNode, I32_DTYPE, base.length, segments, registry, Arena.global());
76+
77+
// When
78+
Array result = sut.decode(ctx);
79+
80+
// Then
81+
assertThat(result.length()).isEqualTo(base.length);
82+
assertThat(result.buffer(0).get(LE_INT, 0L)).isEqualTo(10);
83+
assertThat(result.buffer(0).get(LE_INT, 4L)).isEqualTo(777);
84+
assertThat(result.buffer(0).get(LE_INT, 8L)).isEqualTo(30);
85+
assertThat(result.buffer(0).get(LE_INT, 12L)).isEqualTo(999);
86+
assertThat(result.buffer(0).get(LE_INT, 16L)).isEqualTo(50);
87+
}
88+
}

0 commit comments

Comments
 (0)