Skip to content

Commit f91e8ef

Browse files
dfa1claude
andcommitted
docs(adr): ADR 0010 — final design, kernel follow-up, drop of() factory
Updates after PR #36 (array interface refactor) and the lazy-ALP PoC in PR #35 settled the shape: - Phase 1 swaps the earlier "open interface + package-private BufferedDoubleArray + static of()" sketch for the chosen design: non-sealed interface + public MaterializedXxxArray record, no static factory on the interface, ~115 construction sites updated in PR #36. - Adds a "Possible follow-up: kernel-based MaterializedXxxArray" subsection that explores generalising the per-encoding eager loop into a kernel constructor, with the trade-offs (loss of encoding-specific branch-split tricks, per-row interface dispatch risk). - Phase 2 drops the hasFilter() gate that the early draft proposed. Measurement showed lazy is +9.5% on the no-filter full fold, so gating is unnecessary and was rejected. The fused chain detector description is folded into Phase 2. - Phase 3 explicitly notes that compute pushdown lives as direct methods on the encoding-specific concrete (sumWhereGt) rather than a Kernel SPI; the SPI is deferred until a second encoding ships its lazy variant. - Consequences section rewritten to match: public Materialized* types, no factory, method-on-concrete pushdown. - Removes a duplicate Phase 2 — compute pushdown stub that earlier edits had left behind. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5ba2ae3 commit f91e8ef

1 file changed

Lines changed: 179 additions & 121 deletions

File tree

docs/adr/0010-lazy-decode.md

Lines changed: 179 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,9 @@ like a regression on the only number we measure.
100100
nothing. (TODO.)
101101

102102
Keep the existing `javaReadClose` (full fold) as the **negative test**.
103-
No phase may regress it — the no-filter path stays bit-for-bit eager
104-
(see the API gate below).
103+
Initial fear was that lazy-by-default would regress it; PoC measurement
104+
showed the opposite (+9.5% on the OHLC chain), so the "gate on
105+
`hasFilter()`" idea is dropped — see Phase 2.
105106

106107
#### Phase 0 baseline (10M rows, OHLC, `close` column)
107108

@@ -116,96 +117,132 @@ Two observations:
116117
1. Java loses 3.5–4× to JNI at low selectivity. The whole loss is eager
117118
decode of rejected rows.
118119
2. Java *already* wins at 100% selectivity — JNI's per-batch Arrow
119-
marshalling costs more than Java's tight fold. **This is the reason
120-
to gate lazy on `hasFilter()` instead of making it the default.**
120+
marshalling costs more than Java's tight fold.
121121

122-
### API gate — eager unless a filter is present
122+
Initial reading was "gate lazy behind `hasFilter()` so the full-fold
123+
path stays eager and the filter path switches to lazy." Measurement
124+
rejected the gate (see below — lazy is faster on both paths).
125+
126+
### API gate (rejected by PoC measurement)
127+
128+
Early draft proposed gating lazy decode behind
129+
`ScanOptions.hasFilter()` so the no-filter path stayed bit-for-bit
130+
eager:
123131

124132
```
125133
ScanOptions.hasFilter() == false → eager path (today), zero change
126134
ScanOptions.hasFilter() == true → lazy + compute pushdown
127135
```
128136

129-
Consequences of the gate:
137+
The motivation was protecting `javaReadClose` (the README full-fold
138+
bench) from any regression caused by virtual dispatch on the lazy
139+
variant.
130140

131-
- `javaReadClose` (no filter) is **untouched**. No `DoubleArray`
132-
polymorphism, no virtual call, no patch-bitmap allocation. Eliminates
133-
the "negative consequence" that worried earlier drafts.
134-
- `javaFilterClose` switches to the pushdown path. The user's loop does
135-
not change: the chunk it receives is already **compacted** to matching
136-
rows, so the per-row `if (v > threshold)` check goes away.
137-
138-
```java
139-
// Today — user pays the per-row predicate check
140-
for (long i = 0; i < close.length(); i++) {
141-
double v = close.getDouble(i);
142-
if (v > threshold) sum += v;
143-
}
144-
145-
// After phase 2 — chunk is pre-filtered, length = matched rows only
146-
for (long i = 0; i < close.length(); i++) {
147-
sum += close.getDouble(i);
148-
}
149-
```
150-
151-
The filter is applied inside `ScanIterator.next()` *before* the chunk is
152-
returned. `close.length()` reports the matched row count for that chunk.
153-
Empty chunks are skipped inside `next()`.
141+
**PoC measurement rejected this gate.** Lazy decode is *strictly
142+
faster* than eager on full fold (+9.5% on `javaReadClose`) because
143+
the materialisation write/read intermediate buffer disappears — the
144+
lazy variant returns the encoded segment directly and applies the
145+
transform on access; the fused variant unpacks bitpacked → double in
146+
one pass. Net halving of memory traffic on the OHLC chain. The gate
147+
is dropped; lazy is the default whenever the chain pattern matches.
154148

155149
### Phase 1 — Array hierarchy refactor (no behavior change)
156150

157151
Today each primitive Array is a `public final class` with a
158152
`MemorySegment` buffer field. Lazy variants cannot extend it. Convert
159-
every numeric Array to an **open interface**; keep the current concrete
160-
behavior as a package-private default record exposed only via a static
161-
factory:
153+
every numeric and bool Array to a **non-sealed interface** and move
154+
the current behaviour into a **public** `MaterializedXxxArray` record.
155+
No static factory on the interface — encoders construct
156+
`new MaterializedXxxArray(...)` directly, keeping the interface a
157+
pure contract:
162158

163159
```java
164-
public interface DoubleArray extends Array {
160+
public non-sealed interface DoubleArray extends Array {
165161
double getDouble(long i);
166162
void forEachDouble(DoubleConsumer c);
167163
double fold(double identity, DoubleBinaryOperator op);
168-
169-
/// Default impl backed by a materialized MemorySegment.
170-
/// Clients never reference the concrete type.
171-
static DoubleArray of(DType dtype, long length, MemorySegment buffer) {
172-
return new BufferedDoubleArray(dtype, length, buffer);
173-
}
174164
}
175165

176-
// package-private — name does not appear in the public API
177-
record BufferedDoubleArray(DType dtype, long length, MemorySegment buffer)
178-
implements DoubleArray {
179-
public double getDouble(long i) {
180-
return buffer.getAtIndex(PTypeIO.LE_DOUBLE, i);
181-
}
182-
// forEachDouble, fold — same body as today's DoubleArray
166+
public final class MaterializedDoubleArray implements DoubleArray {
167+
public MaterializedDoubleArray(DType dtype, long length, MemorySegment buffer) { ... }
168+
@Override public double getDouble(long i) { ... } // existing body
169+
@Override public void forEachDouble(DoubleConsumer c) { ... }
170+
@Override public double fold(double identity, DoubleBinaryOperator op) { ... }
183171
}
184172
```
185173

186-
Scope: `DoubleArray`, `FloatArray`, `LongArray`, `IntArray`,
187-
`ShortArray`, `ByteArray`. Defer `BoolArray`, `VarBinArray`,
188-
`VarBinViewArray`, `MaskedArray` — different shapes, no lazy candidate
189-
encoding for them yet.
174+
Scope: `BoolArray`, `ByteArray`, `ShortArray`, `IntArray`, `LongArray`,
175+
`Float16Array`, `FloatArray`, `DoubleArray`. Defer `VarBinArray`,
176+
`VarBinViewArray`, `NullArray`, `EmptyArray` — no lazy candidate
177+
encoding for them yet. Container types (`StructArray`, `MaskedArray`,
178+
`ListArray`, etc.) stay `final class` — not lazy candidates.
190179

191-
Rewrite ~69 `new DoubleArray(...)` (and sibling) call sites in
192-
`reader.decode.*` and `ScanIterator` to use `DoubleArray.of(...)`.
193-
Pure textual change; the factory returns the same buffer-backed
194-
implementation as before.
180+
Rewrite all `new XxxArray(...)` call sites (~115 across reader,
181+
writer, integration, performance, core tests) to
182+
`new MaterializedXxxArray(...)`. Pure textual change.
183+
`ArraySegments` pattern matches resolve to the concrete
184+
`MaterializedXxxArray` so its `buffer()` accessor stays
185+
package-private.
195186

196187
**Behavior unchanged.** Every existing test, integration test, and
197-
benchmark sees `BufferedDoubleArray` everywhere via the interface. JIT
198-
call sites stay monomorphic until a second impl is introduced. Run
199-
`./mvnw verify` and `RustVsJavaReadBenchmark` to confirm zero
200-
regression before moving on.
188+
benchmark sees `MaterializedXxxArray` everywhere via the interface.
189+
JIT call sites stay monomorphic until a second impl is introduced.
201190

202191
**Not sealed.** Custom encoding-specific concretes (phase 2+) just
203-
`implements DoubleArray` — no permits edit, no exhaustive switch
192+
`implements XxxArray` — no permits edit, no exhaustive switch
204193
required in callers. Kernels use `instanceof` + `default` fallback so
205194
unknown impls degrade to the generic per-row path automatically. This
206195
keeps third-party encodings on equal footing with built-in ones.
207196

208-
### Phase 2 — Lazy ALP variant + filter gate
197+
**No `static of(...)` on the interface.** An earlier draft added a
198+
factory returning a (then package-private) buffer-backed default.
199+
That coupled the interface to its concrete and proved unused once
200+
decoders were rewritten to call `new MaterializedXxxArray(...)`
201+
directly. The interface stays a pure contract.
202+
203+
#### Possible follow-up: kernel-based MaterializedXxxArray
204+
205+
Every encoding's eager decode is the same pattern: allocate a buffer,
206+
loop over rows, write `kernel(i)`. Generalising would let the
207+
constructor own the loop:
208+
209+
```java
210+
public MaterializedDoubleArray(DType dtype, long length,
211+
LongToDoubleFunction kernel, Arena arena) {
212+
MemorySegment dst = arena.allocate(length * 8, 8);
213+
for (long i = 0; i < length; i++) {
214+
dst.setAtIndex(LE_DOUBLE, i, kernel.applyAsDouble(i));
215+
}
216+
// store dst
217+
}
218+
```
219+
220+
Then `AlpEncodingDecoder.decodeF64` shrinks to ~4 lines:
221+
222+
```java
223+
return new MaterializedDoubleArray(dtype, n,
224+
i -> (double) src.getAtIndex(LE_LONG, i) * scale, arena);
225+
```
226+
227+
Trade-offs:
228+
229+
- **Win:** dedup. Every eager decoder collapses to one expression +
230+
one constructor call.
231+
- **Cost — per-row `invokeinterface`.** C2 inlines if the kernel
232+
call site is monomorphic (one decoder = one kernel impl). Bimorphic
233+
risk if multiple decoders alias the loop body.
234+
- **Lost flexibility.** Encoding-specific tricks — `i % srcCap`
235+
branch-split, broadcast-cap branches, in-place writes when source
236+
is writable — don't fit a pure kernel. Forcing them into the kernel
237+
means a per-row `cap` check (banned by the hot-loop rule); keeping
238+
them out means the kernel constructor only covers the boring case.
239+
240+
Add the kernel constructor *alongside* the existing buffer
241+
constructor. Decoders that want the loop dedup opt in; decoders with
242+
quirky paths keep their own loop. Tracked as a follow-up; not part of
243+
this phase.
244+
245+
### Phase 2 — Lazy ALP + fused chain
209246

210247
Add the first lazy implementation:
211248

@@ -238,46 +275,77 @@ Patches index: two options for O(1) lookup:
238275

239276
Use the bitmap. Predictable per-access cost.
240277

241-
**Filter gate** in `AlpEncodingDecoder.decode`:
278+
**No filter gate.** PoC measurement showed lazy is *strictly faster*
279+
than the eager path even on full-fold workloads (+10.6% on
280+
`javaReadClose`), because the materialisation write/read intermediate
281+
buffer is skipped entirely. The earlier "gate lazy behind
282+
`hasFilter()`" idea is dropped in the final design — lazy is the
283+
default whenever the chain pattern matches:
242284

243285
```java
244-
return ctx.hasFilter()
245-
? new AlpDoubleArray(dtype, n, encoded, scale, patches) // lazy
246-
: MaterializedDoubleArray.of(...); // today's eager path
286+
return new AlpDoubleArray(dtype, n, encoded, scale, patches); // always lazy
287+
// fallback to: new MaterializedDoubleArray(...) when not lazy-eligible
288+
// (read-only source, broadcast source, patched chunk)
247289
```
248290

249-
`DecodeContext` gains an `hasFilter()` hint propagated from
250-
`ScanOptions`. No `RowFilter` parsing inside the decoder — only the
251-
boolean signal.
291+
No `DecodeContext.hasFilter()` plumbing is needed — the gate is gone.
292+
293+
#### Fused chain detection
294+
295+
When the ALP child layout is `FoR(Bitpacked)` (or `Bitpacked` directly
296+
when the writer dropped FoR for `ref==0`), the decoder skips the
297+
intermediate FoR/ALP buffers entirely and returns a
298+
`FusedAlpForBitpackedDoubleArray` that holds the raw packed buffer +
299+
`(bitWidth, offset, ref, scale)`. `sumWhereGt` unpacks each row,
300+
applies `+ref` and the threshold compare inline, decodes
301+
`(double)(val+ref)*scale` only for matches. The full-fold path
302+
(`getDouble`/`fold`/`forEachDouble`) lazily materialises through one
303+
pass that writes doubles directly from the bitpacked unpack —
304+
halving the memory traffic vs the old eager chain (one decode pass
305+
instead of bitpacked→FoR→ALP).
306+
307+
`AlpEncodingDecoder` walks the `ArrayNode` tree to detect the chain
308+
(no decoding cost to peek). Falls back to `AlpDoubleArray` for the
309+
bare-ALP case, `MaterializedDoubleArray` for patched chunks.
252310

253311
### Phase 3 — compute pushdown
254312

255-
`ScanIterator` routes `ScanOptions.rowFilter()` through a kernel SPI
256-
before falling back to materialization. Initial kernels:
313+
PoC chose **direct methods on the encoding-specific concrete** over a
314+
dedicated Kernel SPI. `AlpDoubleArray.sumWhereGt(threshold)` and
315+
`FusedAlpForBitpackedDoubleArray.sumWhereGt(threshold)` are public
316+
methods; callers (today: the filter bench; tomorrow: `ScanIterator`)
317+
pattern-match on the concrete type and call the right method:
318+
319+
```java
320+
DoubleArray col = chunk.column("close");
321+
double sum = switch (col) {
322+
case FusedAlpForBitpackedDoubleArray fused -> fused.sumWhereGt(threshold);
323+
case AlpDoubleArray alp -> alp.sumWhereGt(threshold);
324+
default -> Filters.scalarSumGt(col, threshold);
325+
};
326+
```
327+
328+
The Kernel SPI from earlier drafts is **deferred** until a second
329+
encoding (FoR, ZigZag, AlpRd) ships its own lazy variant and the
330+
combinatorial explosion of `(kernel × encoding)` becomes visible.
331+
Until then, method-on-concrete is simpler to read, faster to inline
332+
(no SPI dispatch), and lets each encoding expose the operators that
333+
make sense for its math without an upstream interface lock-in.
334+
335+
When the SPI eventually lands, candidates are:
257336

258337
- `CompareKernel`: `compare(arr, scalar, op) → BoolArray`. For
259338
`AlpDoubleArray`, encode the scalar to the int domain
260-
(`enc = round(scalar / scale)`) and compare ints. For
339+
(`enc = floor(scalar / scale)`) and compare ints. For
261340
`ForLongArray` (when it lands), subtract the reference and compare
262341
ints. Falls back to materialization when the scalar does not
263342
round-trip through the encoding.
264343
- `BetweenKernel`: same approach for two scalars.
265344
- `TakeKernel`: `take(arr, indices)` — decode only the requested
266345
indices. Unblocks the take/slice/projection wins from phase 0.
267-
- `SumKernel`, `MinKernel`, `MaxKernel`: deferred.
268-
`sum(AlpDoubleArray) = sum(int) * scale + patch_correction` is
269-
straightforward but not on the critical path.
270-
271-
Pattern-match dispatch in `ScanIterator` with a `default` fallback —
272-
no exhaustiveness required, so unknown future impls degrade gracefully:
273-
274-
```java
275-
DoubleArray col = chunk.column("close");
276-
BoolArray sel = switch (col) {
277-
case AlpDoubleArray alp -> alp.compareGt(threshold, arena);
278-
default -> Filters.scalarGt(col, threshold); // generic via getDouble
279-
};
280-
```
346+
- `SumKernel`, `MinKernel`, `MaxKernel`: `sum(AlpDoubleArray) =
347+
sum(int) * scale + patch_correction` is straightforward but not on
348+
the critical path.
281349

282350
For multi-column filters: `AND` evaluates kernels in column order,
283351
intersecting selection vectors; `OR` unions them. Columns referenced
@@ -286,36 +354,20 @@ and are not delivered to the consumer.
286354

287355
### Future — extend the lazy family
288356

289-
Once ALP proves the shape, add (no API change, just new permits):
357+
Once ALP proves the shape (which it has, on PR #35), add the same
358+
pattern per encoding. No interface change — each new variant is just
359+
another `implements DoubleArray` (or `LongArray`, `IntArray`):
290360

291361
- `AlpRdDoubleArray` — same idea for ALP-RD
292-
- `ForLongArray`, `ForIntArray` — Frame-of-Reference, in-place lazy
293-
- `ZigZagLongArray`, `ZigZagIntArray` — XOR/shift on access
294-
- Composed: `AlpForBitpackedDoubleArray` fuses three transforms into
295-
one expression evaluated per access
296-
297-
### Phase 2 — compute pushdown
298-
299-
`ScanIterator` routes `ScanOptions.rowFilter()` through a kernel SPI
300-
before falling back to materialization. Initial kernels:
301-
302-
- `CompareKernel`: `compare(arr, scalar, op) → BoolArray`. For ALP,
303-
encode the scalar to the int domain (`enc = round(scalar / scale)`)
304-
and compare ints. For FoR, subtract the reference and compare ints.
305-
Falls back to materialization when the scalar does not round-trip
306-
through the encoding (e.g. ALP threshold that is not representable as
307-
`int * 10^(f-e)` exactly).
308-
- `BetweenKernel`: same approach for two scalars.
309-
- `TakeKernel`: `take(arr, indices)` — decode only the requested
310-
indices. Unblocks the take/slice/projection wins from phase 0.
311-
- `SumKernel`, `MinKernel`, `MaxKernel`: deferred. `sum(ALP) =
312-
sum(int) * scale + patch_correction` is straightforward but not on the
313-
critical path.
314-
315-
For multi-column filters: `AND` evaluates kernels in column order,
316-
intersecting selection vectors; `OR` unions them. Columns referenced
317-
only by the filter (not by projection) are decoded just enough to test
318-
and are not delivered to the consumer.
362+
- `ForLongArray`, `ForIntArray` — Frame-of-Reference, lazy
363+
- `ZigZagLongArray`, `ZigZagIntArray` — XOR/shift on access (order
364+
not preserved, so no pushdown — but lazy still skips the
365+
materialisation pass)
366+
- Composed: `FusedAlpForBitpackedDoubleArray` (already exists) fuses
367+
three transforms; analogous fused classes for other common chains
368+
- Extended fusion: handle bitpacked patches inside the fused kernel,
369+
closing the remaining 64% of OHLC chunks that today fall back to
370+
`AlpDoubleArray`
319371

320372
## Consequences
321373

@@ -335,18 +387,24 @@ and are not delivered to the consumer.
335387

336388
### Negative
337389

338-
- **API surface grows minimally.** Every numeric `*Array` becomes an
339-
open interface; the default buffer-backed impl is package-private,
340-
accessed via `*Array.of(...)`. Downstream consumers that constructed
341-
`new DoubleArray(...)` directly must switch to the factory; consumers
342-
that only *receive* arrays from `Chunk.column(...)` see no change.
343-
Encoding-specific concretes (`AlpDoubleArray`, etc.) become first-class
344-
public types that kernels can pattern-match against.
390+
- **API surface grows.** Every numeric `*Array` becomes a
391+
`non-sealed interface`. The default buffer-backed impl becomes a
392+
public `MaterializedXxxArray` record. Downstream consumers that
393+
constructed `new DoubleArray(...)` directly must switch to
394+
`new MaterializedDoubleArray(...)`; consumers that only *receive*
395+
arrays from `Chunk.column(...)` see no change. Encoding-specific
396+
concretes (`AlpDoubleArray`, `FusedAlpForBitpackedDoubleArray`,
397+
etc.) are first-class public types that callers pattern-match
398+
against. The interface stays a pure contract — no `static of(...)`
399+
factory, no permits, no encoder coupling.
345400
- **Patch lookup is per-access in the lazy path.** Today patches are
346401
applied once at decode time. Lazy needs an index structure (cost
347402
above) and pays per-row. Only the filter path triggers this.
348-
- **Kernel SPI is a non-trivial design.** Initial scope must be small:
349-
compare, between, take. Sum/min/max can wait.
403+
- **Compute pushdown lives as methods on the encoding-specific
404+
concrete.** No Kernel SPI yet — `AlpDoubleArray.sumWhereGt(...)`,
405+
etc., are direct public methods. Cleaner to inline, looser to
406+
extend. SPI lands when a second encoding's lazy variant proves the
407+
shape repeats.
350408
- **Filter semantics change.** Today `RowFilter` is a zone-map prune
351409
hint; the consumer still re-checks every row. After phase 3 the chunk
352410
returned by `next()` is already filtered. This is a breaking change

0 commit comments

Comments
 (0)