Skip to content

Commit 2ee7d87

Browse files
dfa1claude
andcommitted
fix(reader): decode fastlanes.delta into the arena, not four heap long[]
DeltaEncodingDecoder routed the whole column through four row-scaled heap long[] arrays before writing one arena segment: bases and deltas copied out of their segments, a `decoded` array for the reconstruction, and a `result` array whose only job was to drop `offset` leading elements — a second full traversal for a slice. Every value was widened to 8 bytes whatever the column's width, so an I8 delta column allocated 8x its natural size on the GC heap, three times over. That is CLAUDE.md's allocation rule violated four times at row scale. Values are now reconstructed into a single `ctx.arena()` segment at the ptype's real width. The per-chunk scratch stays on the heap, which is what it is for: fixed-size, cache-resident, reused across chunks. readLongs carried both hot-loop anti-patterns at once — an `i % cap` per element and a per-element `switch (ptype)`. Both are hoisted: readElements branch-splits on whether the segment physically holds the range, so the fast path is a uniform modulo-free loop per ptype and the wrap-around stays on the cold path, where only a vortex.constant child reaches it. The write side gets the same treatment, replacing PrimitiveArrays.fromLongs' per-element PTypeIO.set switch. Chunks are independent — each carries its own lane bases — so only those overlapping the requested row window are reconstructed at all. Reading the tail of a long column no longer walks every chunk before it. Fixes three ADR 0003 violations on the way: a row window past the elements the chunks reconstruct reached System.arraycopy as a raw ArrayIndexOutOfBoundsException; a negative deltas_len sized a heap array (NegativeArraySizeException); and an absurd one was truncated by an (int) cast into either a negative size or an OutOfMemoryError. The window is now validated before any child decode, so bogus metadata never drives an allocation, and a legal window over an absurd declared length simply decodes. Coverage: the decoder had three unit tests, all I64, all single-element children, none past one chunk. Adds a Java-write/Java-read round-trip over all eight integer widths across three chunks with full-width random bit patterns (the high bit is where sign- and zero-extension diverge), a test that a non-zero offset window is exactly the slice of the full decode (the writer always emits offset 0, so that path was unreachable from a file), and the malformed-metadata cases above. Closes #338 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 69aaa64 commit 2ee7d87

5 files changed

Lines changed: 423 additions & 57 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- A malformed `fastlanes.delta` column no longer fails with a raw JDK exception: a row window running past the elements the chunks reconstruct threw `ArrayIndexOutOfBoundsException`, and an absurd or negative declared element count sized a heap array before anything checked it (`NegativeArraySizeException`, or `OutOfMemoryError`). All now fail as `VortexException`. ([#338](https://github.com/dfa1/vortex-java/issues/338))
13+
- A `fastlanes.delta` column no longer routes its decode through four row-scaled heap `long[]` arrays, every value widened to 8 bytes whatever the column's width; values are reconstructed into a single arena segment at the ptype's real width, and only the chunks overlapping the requested rows are reconstructed at all. ([#338](https://github.com/dfa1/vortex-java/issues/338))
14+
1215
- A run-end-encoded Utf8/Binary column (`vortex.runend`) no longer expands every run into a fully materialized buffer on decode; rows now resolve through the runs lazily, removing an unbounded `sum(runLength * valueLength)` allocation that a crafted file could drive to `OutOfMemoryError`. ([#334](https://github.com/dfa1/vortex-java/issues/334))
1316
- A `vortex.sequence` column no longer materializes `base + i * multiplier` into a full buffer on decode; rows are computed on access, so the encoding allocates nothing regardless of row count — closing an `OutOfMemoryError` risk from a metadata-only encoding whose row count no buffer bounds. ([#335](https://github.com/dfa1/vortex-java/issues/335))
1417
- A primitive `vortex.dict` column decoded through the encoding path no longer expands its codes into an `n * elemSize` buffer; it now returns the same lazy `DictXxxArray` carriers the layout path already used, so a dict column keeps the dictionary's memory benefit however it is reached. ([#336](https://github.com/dfa1/vortex-java/issues/336))

docs/compatibility.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ decoder falls into one of three shapes:
159159
| `vortex.datetimeparts` | Lazy | Lazy | `LazyDateTimePartsLongArray` — reassembles parts on access |
160160
| `vortex.pco` | Materialized | Materialized | range-encoded decompression |
161161
| `fastlanes.bitpacked` | Materialized | Materialized | window unpacks bits |
162-
| `fastlanes.delta` | Materialized | Materialized | cumulative sum requires sequential decode |
162+
| `fastlanes.delta` | Materialized | Materialized | cumulative sum requires sequential decode; output is one arena segment at the ptype's width, and only the chunks the row window touches are reconstructed |
163163
| `fastlanes.for` | Lazy | Lazy | `LazyForXxxArray` (I8/U8/I16/U16/I32/U32/I64/U64), ADR 0010 + 0015 |
164164
| `fastlanes.rle` | Lazy | Lazy | `LazyRleXxxArray`; validity → `OffsetBoolArray`; empty → `LazyConstantXxxArray`, ADR 0015 |
165165
| `vortex.patched` | Materialized | Materialized | inner is full base + chunked patches (1024-elem blocks, lane-window-sorted); per-row access requires 2 laneOffsets reads + binary search inside the chunk window, so eager scatter wins for full scans |

integration/src/test/java/io/github/dfa1/vortex/integration/JavaRoundTripIntegrationTest.java

Lines changed: 178 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,61 @@
11
package io.github.dfa1.vortex.integration;
22

3+
import io.github.dfa1.vortex.core.compute.FastLanes;
34
import io.github.dfa1.vortex.core.model.ColumnName;
45
import io.github.dfa1.vortex.core.model.DType;
56
import io.github.dfa1.vortex.core.model.Editions;
7+
import io.github.dfa1.vortex.core.model.PType;
8+
import io.github.dfa1.vortex.core.model.EncodingId;
9+
import io.github.dfa1.vortex.core.proto.ProtoDeltaMetadata;
10+
import io.github.dfa1.vortex.inspect.InspectorTree;
611
import io.github.dfa1.vortex.reader.ReadRegistry;
712
import io.github.dfa1.vortex.reader.ScanOptions;
813
import io.github.dfa1.vortex.reader.VortexReader;
14+
import io.github.dfa1.vortex.reader.array.Array;
15+
import io.github.dfa1.vortex.reader.array.ByteArray;
916
import io.github.dfa1.vortex.reader.array.IntArray;
17+
import io.github.dfa1.vortex.reader.array.LongArray;
18+
import io.github.dfa1.vortex.reader.array.ShortArray;
19+
import io.github.dfa1.vortex.reader.decode.ArrayNode;
20+
import io.github.dfa1.vortex.reader.decode.DecodeContext;
21+
import io.github.dfa1.vortex.reader.decode.DeltaEncodingDecoder;
1022
import io.github.dfa1.vortex.writer.VortexWriter;
1123
import io.github.dfa1.vortex.writer.WriteOptions;
24+
import io.github.dfa1.vortex.writer.WriteRegistry;
25+
import io.github.dfa1.vortex.writer.encode.EncodeContext;
26+
import io.github.dfa1.vortex.writer.encode.EncodeResult;
27+
import io.github.dfa1.vortex.writer.encode.DeltaEncodingEncoder;
1228
import io.github.dfa1.vortex.writer.encode.PatchedEncodingEncoder;
1329
import org.junit.jupiter.api.Test;
1430
import org.junit.jupiter.api.io.TempDir;
31+
import org.junit.jupiter.params.ParameterizedTest;
32+
import org.junit.jupiter.params.provider.EnumSource;
1533

1634
import java.io.IOException;
35+
import java.lang.foreign.Arena;
36+
import java.lang.foreign.MemorySegment;
1737
import java.nio.channels.FileChannel;
1838
import java.nio.file.Path;
1939
import java.nio.file.StandardOpenOption;
40+
import java.util.Arrays;
2041
import java.util.ArrayList;
2142
import java.util.List;
2243
import java.util.Map;
44+
import java.util.Random;
2345

2446
import static org.assertj.core.api.Assertions.assertThat;
2547

26-
/// Java writer → Java reader round-trips for encodings the bundled `vortex-jni` build cannot read
27-
/// back, so they have no Java→Rust coverage. This is still a real cross-module integration test: it
28-
/// drives the writer's encode, the on-disk file format, and the reader's decode end to end.
48+
/// Java writer → Java reader round-trips for encodings whose Java *decode* has no other end-to-end
49+
/// cover. This is a real cross-module integration test either way: it drives the writer's encode,
50+
/// the on-disk file format, and the reader's decode end to end.
2951
///
30-
/// `vortex.patched` is the case here — the JNI reader rejects a standalone patched array with
31-
/// "Unknown encoding: vortex.patched", so the round-trip is asserted on the Java side instead.
52+
/// Two reasons land a case here:
53+
/// - the bundled `vortex-jni` build cannot read the encoding back, so there is no Java→Rust test.
54+
/// `vortex.patched` is this case — the JNI reader rejects a standalone patched array with
55+
/// "Unknown encoding: vortex.patched".
56+
/// - Java→Rust cover exists but only exercises the *encoder*. `fastlanes.delta` is this case:
57+
/// `JavaWritesRustReadsIntegrationTest#javaWriter_rustReader_delta_i64` proves what Java writes
58+
/// is readable, and says nothing about `DeltaEncodingDecoder`.
3259
class JavaRoundTripIntegrationTest {
3360

3461
private static final DType.Struct I32_SCHEMA = new DType.Struct(
@@ -62,6 +89,152 @@ void patched_i32_javaWriteJavaRead(@TempDir Path tmp) throws IOException {
6289
assertThat(decoded).containsExactly(data);
6390
}
6491

92+
/// `fastlanes.delta` decode across every width it accepts, over three FastLanes chunks.
93+
///
94+
/// The unit tests reach the decoder only with I64 and single-element (constant) children, so
95+
/// nothing covered the per-width read and write paths, and nothing covered more than one
96+
/// chunk — which is where the chunk-window arithmetic lives. Values are full-width random
97+
/// bit patterns, not a monotonic ramp: the high bit is exactly where a read that
98+
/// sign-extends and one that zero-extends diverge, and delta round-trips any values at all
99+
/// since encode and decode both wrap modulo the type width.
100+
@ParameterizedTest
101+
@EnumSource(value = PType.class, names = {"I8", "I16", "I32", "I64", "U8", "U16", "U32", "U64"})
102+
void delta_javaWriteJavaRead(PType ptype, @TempDir Path tmp) throws IOException {
103+
// Given — 2500 rows is three 1024-element chunks, the last one padded.
104+
long mask = FastLanes.lowMask(ptype.bits());
105+
Random rng = new Random(338);
106+
long[] expected = new long[2500];
107+
for (int i = 0; i < expected.length; i++) {
108+
expected[i] = rng.nextLong() & mask;
109+
}
110+
DType.Struct schema = new DType.Struct(List.of(ColumnName.of("v")),
111+
List.of(new DType.Primitive(ptype, false)), false);
112+
Path file = tmp.resolve("java_delta_" + ptype + ".vtx");
113+
114+
// When
115+
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
116+
var sut = VortexWriter.create(ch, schema,
117+
WriteOptions.defaults().withEdition(Editions.UNSTABLE_2025_05_0),
118+
List.of(new DeltaEncodingEncoder()))) {
119+
sut.writeChunk(Map.of(ColumnName.of("v"), narrow(expected, ptype)));
120+
}
121+
122+
// Then — the encoding is asserted too, so a writer that quietly stopped choosing delta
123+
// would fail here rather than leave the decoder untested
124+
try (var reader = VortexReader.open(file, ReadRegistry.loadAll())) {
125+
assertThat(InspectorTree.build(reader).usedEncodings()).contains("fastlanes.delta");
126+
}
127+
// compared as stored bit patterns, so signed and unsigned widths assert alike
128+
assertThat(readColumnBits(file, "v", mask)).containsExactly(expected);
129+
}
130+
131+
/// `fastlanes.delta`'s `offset` metadata — which makes a decode start partway into the
132+
/// reconstructed elements — has no round-trip cover, because the Java writer always emits 0;
133+
/// a non-zero offset only ever arrives on a Rust-written sliced array. So this drives the
134+
/// decoder directly over encoder-produced children instead of through a file, and asserts
135+
/// the window is exactly the corresponding slice of the full decode. The window arithmetic
136+
/// (which chunks to reconstruct, and where each lands in the output) is the part of decode
137+
/// that only a non-zero offset reaches.
138+
@Test
139+
void delta_offsetWindowIsTheSliceOfTheFullDecode() {
140+
// Given — 2500 rows, so the encoder pads to three chunks
141+
DType dtype = new DType.Primitive(PType.I64, false);
142+
Random rng = new Random(3381);
143+
long[] data = new long[2500];
144+
for (int i = 0; i < data.length; i++) {
145+
data[i] = rng.nextLong();
146+
}
147+
try (Arena arena = Arena.ofConfined()) {
148+
EncodeResult encoded = new DeltaEncodingEncoder().encode(dtype, data,
149+
EncodeContext.of(arena, WriteRegistry.builder().registerDefaults().build()));
150+
long padded = 3L * FastLanes.CHUNK;
151+
long[] full = decodeDelta(encoded, dtype, padded, 0, padded, arena);
152+
153+
// When — a window opening inside chunk 0 and closing inside chunk 2
154+
long[] result = decodeDelta(encoded, dtype, padded, 700, 1500, arena);
155+
156+
// Then
157+
assertThat(result).containsExactly(Arrays.copyOfRange(full, 700, 2200));
158+
}
159+
}
160+
161+
/// Decodes `encoded` as a `fastlanes.delta` array over the given window, bypassing the file
162+
/// format so the `offset` the writer never emits can be set.
163+
///
164+
/// @param encoded the encoder's output (bases buffer, deltas buffer)
165+
/// @param dtype logical element type
166+
/// @param deltasLen number of reconstructed elements the chunks cover
167+
/// @param offset absolute index the first returned row maps to
168+
/// @param rowCount number of rows to decode
169+
/// @param arena allocator for the decoded segment
170+
/// @return the decoded values
171+
private static long[] decodeDelta(EncodeResult encoded, DType dtype, long deltasLen,
172+
int offset, long rowCount, Arena arena) {
173+
MemorySegment meta = MemorySegment.ofArray(new ProtoDeltaMetadata(deltasLen, offset).encode());
174+
ArrayNode bases = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{0});
175+
ArrayNode deltas = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{1});
176+
ArrayNode node = new ArrayNode(EncodingId.FASTLANES_DELTA, meta,
177+
new ArrayNode[]{bases, deltas}, new int[0]);
178+
DecodeContext ctx = new DecodeContext(node, dtype, rowCount,
179+
encoded.buffers().toArray(new MemorySegment[0]), ReadRegistry.loadAll(), arena);
180+
LongArray decoded = (LongArray) new DeltaEncodingDecoder().decode(ctx);
181+
long[] out = new long[(int) decoded.length()];
182+
for (int i = 0; i < out.length; i++) {
183+
out[i] = decoded.getLong(i);
184+
}
185+
return out;
186+
}
187+
188+
/// Narrows logical values to the Java array type the writer expects for `ptype`.
189+
private static Object narrow(long[] values, PType ptype) {
190+
return switch (ptype) {
191+
case I8, U8 -> {
192+
byte[] out = new byte[values.length];
193+
for (int i = 0; i < values.length; i++) {
194+
out[i] = (byte) values[i];
195+
}
196+
yield out;
197+
}
198+
case I16, U16 -> {
199+
short[] out = new short[values.length];
200+
for (int i = 0; i < values.length; i++) {
201+
out[i] = (short) values[i];
202+
}
203+
yield out;
204+
}
205+
case I32, U32 -> {
206+
int[] out = new int[values.length];
207+
for (int i = 0; i < values.length; i++) {
208+
out[i] = (int) values[i];
209+
}
210+
yield out;
211+
}
212+
default -> values.clone();
213+
};
214+
}
215+
216+
/// Reads a primitive column back as raw bit patterns, masked to the type's width so a
217+
/// sign-extending accessor and a zero-extending one compare equal.
218+
private static long[] readColumnBits(Path file, String column, long mask) throws IOException {
219+
var out = new ArrayList<Long>();
220+
try (var vf = VortexReader.open(file, ReadRegistry.loadAll());
221+
var iter = vf.scan(ScanOptions.columns(column))) {
222+
iter.forEachRemaining(c -> {
223+
Array arr = c.column(column);
224+
for (long i = 0; i < arr.length(); i++) {
225+
out.add(switch (arr) {
226+
case ByteArray a -> a.getByte(i) & mask;
227+
case ShortArray a -> a.getShort(i) & mask;
228+
case IntArray a -> a.getInt(i) & mask;
229+
case LongArray a -> a.getLong(i) & mask;
230+
default -> throw new IllegalStateException("unexpected array " + arr.getClass());
231+
});
232+
}
233+
});
234+
}
235+
return out.stream().mapToLong(Long::longValue).toArray();
236+
}
237+
65238
@SuppressWarnings("SameParameterValue")
66239
private static int[] readIntColumn(Path file, String column) throws IOException {
67240
try (var vf = VortexReader.open(file, ReadRegistry.loadAll());

0 commit comments

Comments
 (0)