Skip to content

Commit 7af2a78

Browse files
dfa1claude
andcommitted
refactor: dedupe narrowest-unsigned-ptype selection into one helper
Three writer-side encoders each hand-rolled byte-for-byte identical logic to pick the narrowest unsigned PType (U8/U16/U32) that fits a non-negative value: BitpackedEncodingEncoder.chooseIdxPtype, SparseEncodingEncoder. chooseIdxPtype, and FsstEncodingEncoder.narrowestUnsignedPType. A future tiering change (e.g. a U64 tier) would have to touch three sites and could silently drift. Extract once as PType.narrowestUnsigned(long), matching the enum's existing static-factory/classification idiom (fromOrdinal, isUnsigned). The long parameter accepts all current int call sites unchanged and leaves room for a wider tier. Delete the three private duplicates; all five call sites now delegate. Boundary tests (0, 0xFF, 0x100, 0xFFFF, 0x10000, 0xFFFFFFFF) added to PTypeTest. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 0268805 commit 7af2a78

5 files changed

Lines changed: 43 additions & 41 deletions

File tree

core/src/main/java/io/github/dfa1/vortex/core/model/PType.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,23 @@ public static PType fromOrdinal(int ordinal) {
9494
}
9595
return all[ordinal];
9696
}
97+
98+
/// Returns the narrowest unsigned integer ptype that can hold every value in
99+
/// `0..maxValue` — `U8` up to `0xFF`, `U16` up to `0xFFFF`, else `U32`.
100+
///
101+
/// Used writer-side to size index/length/offset buffers (patch indices, FSST row
102+
/// lengths and code offsets) to the smallest width that still fits, instead of
103+
/// always paying for a 4-byte `U32`.
104+
///
105+
/// @param maxValue the largest value the buffer must represent, `>= 0`
106+
/// @return `U8`, `U16`, or `U32`, whichever is smallest and still fits
107+
public static PType narrowestUnsigned(long maxValue) {
108+
if (maxValue <= 0xFFL) {
109+
return U8;
110+
}
111+
if (maxValue <= 0xFFFFL) {
112+
return U16;
113+
}
114+
return U32;
115+
}
97116
}

core/src/test/java/io/github/dfa1/vortex/core/model/PTypeTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,21 @@ void fromOrdinal_zeroIsU8() {
9595
// Given / When / Then — wire-format anchor: U8 must remain ordinal 0
9696
assertThat(PType.fromOrdinal(0)).isSameAs(PType.U8);
9797
}
98+
99+
@ParameterizedTest
100+
@CsvSource({
101+
// boundary values: each tier's inclusive ceiling and the first value that spills to the next
102+
"0, U8",
103+
"255, U8", // 0xFF — last U8
104+
"256, U16", // 0x100 — first U16
105+
"65535, U16", // 0xFFFF — last U16
106+
"65536, U32", // 0x10000 — first U32
107+
"4294967295, U32", // 0xFFFFFFFF — beyond U16, still U32
108+
})
109+
void narrowestUnsigned_picksSmallestFittingTier(long maxValue, PType expected) {
110+
// Given / When
111+
PType result = PType.narrowestUnsigned(maxValue);
112+
// Then
113+
assertThat(result).isSameAs(expected);
114+
}
98115
}

writer/src/main/java/io/github/dfa1/vortex/writer/encode/BitpackedEncodingEncoder.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
118118
}
119119

120120
int numPatches = patchIdx.size();
121-
PType idxPtype = chooseIdxPtype(n);
121+
PType idxPtype = PType.narrowestUnsigned(n);
122122
MemorySegment idxBuf = buildPatchIdxBuf(patchIdx, idxPtype, ctx.arena());
123123
MemorySegment valBuf = buildPatchValBuf(patchVal, ptype, ctx.arena());
124124

@@ -157,16 +157,6 @@ private static int bestBitWidth(int[] bitWidthFreq, int bytesPerException, int n
157157
return bestWidth;
158158
}
159159

160-
private static PType chooseIdxPtype(int n) {
161-
if (n <= 0xFF) {
162-
return PType.U8;
163-
}
164-
if (n <= 0xFFFF) {
165-
return PType.U16;
166-
}
167-
return PType.U32;
168-
}
169-
170160
private static MemorySegment buildPatchIdxBuf(List<Integer> idx, PType idxPtype, Arena arena) {
171161
int numPatches = idx.size();
172162
int elemBytes = idxPtype.byteSize();

writer/src/main/java/io/github/dfa1/vortex/writer/encode/FsstEncodingEncoder.java

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
118118
// typically far below the 4-byte ceiling this always used to pay (e.g. a 6-byte string
119119
// column needs only U8 lengths, not I32), and the wire format carries the chosen ptype
120120
// per FSSTMetadata specifically so a reader never has to guess.
121-
PType uncompLenPType = narrowestUnsignedPType(maxUncompLen);
122-
PType codesOffPType = narrowestUnsignedPType(totalCompressed);
121+
PType uncompLenPType = PType.narrowestUnsigned(maxUncompLen);
122+
PType codesOffPType = PType.narrowestUnsigned(totalCompressed);
123123

124124
MemorySegment uncompLenBuf = arena.allocate(Math.max((long) n * uncompLenPType.byteSize(), 1));
125125
for (int i = 0; i < n; i++) {
@@ -152,24 +152,10 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
152152
null, null);
153153
}
154154

155-
/// Narrowest unsigned `PType` that can hold every value up to `maxValue`.
156-
///
157-
/// @param maxValue the largest value the buffer must represent, `>= 0`
158-
/// @return `U8`, `U16`, or `U32`, whichever is smallest and still fits
159-
private static PType narrowestUnsignedPType(int maxValue) {
160-
if (maxValue <= 0xFF) {
161-
return PType.U8;
162-
}
163-
if (maxValue <= 0xFFFF) {
164-
return PType.U16;
165-
}
166-
return PType.U32;
167-
}
168-
169155
/// Writes `value` into `seg` at row `idx`, using `ptype`'s byte width.
170156
///
171157
/// @param seg destination segment
172-
/// @param ptype `U8`, `U16`, or `U32` (whatever [#narrowestUnsignedPType(int)] returned)
158+
/// @param ptype `U8`, `U16`, or `U32` (whatever [PType#narrowestUnsigned(long)] returned)
173159
/// @param idx row index (not a byte offset)
174160
/// @param value the value to write
175161
private static void writeUnsigned(MemorySegment seg, PType ptype, long idx, long value) {

writer/src/main/java/io/github/dfa1/vortex/writer/encode/SparseEncodingEncoder.java

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public CascadeStep encodeCascade(DType dtype, Object data, EncodeContext ctx) {
7575
return CascadeStep.notApplicable();
7676
}
7777
int elemBytes = ptype.byteSize();
78-
PType idxPtype = chooseIdxPtype(n);
78+
PType idxPtype = PType.narrowestUnsigned(n);
7979
int idxBytes = idxPtype.byteSize();
8080
int patchCost = idxBytes + elemBytes;
8181
int maxPatches = (int) Math.min(Integer.MAX_VALUE, ((long) n * elemBytes / 2L) / patchCost);
@@ -227,7 +227,7 @@ static EncodeResult encodeBool(boolean[] validity, EncodeContext ctx) {
227227
boolean[] patchVals = new boolean[numPatches];
228228
java.util.Arrays.fill(patchVals, !fillValue);
229229

230-
PType idxPtype = chooseIdxPtype(n);
230+
PType idxPtype = PType.narrowestUnsigned(n);
231231
Object idxArr = idxArr(patchIdx, idxPtype);
232232

233233
ProtoScalarValue fillScalar = ProtoScalarValue.ofBoolValue(fillValue);
@@ -299,7 +299,7 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
299299
}
300300

301301
int numPatches = patchIdx.size();
302-
PType idxPtype = chooseIdxPtype(n);
302+
PType idxPtype = PType.narrowestUnsigned(n);
303303

304304
ProtoScalarValue fillScalar = zeroScalar(ptype);
305305
byte[] fillBytes = fillScalar.encode();
@@ -353,16 +353,6 @@ private static long readBits(Object data, PType ptype, int i) {
353353
};
354354
}
355355

356-
private static PType chooseIdxPtype(int n) {
357-
if (n <= 0xFF) {
358-
return PType.U8;
359-
} else if (n <= 0xFFFF) {
360-
return PType.U16;
361-
} else {
362-
return PType.U32;
363-
}
364-
}
365-
366356
private static ProtoScalarValue zeroScalar(PType ptype) {
367357
return switch (ptype) {
368358
case I8, I16, I32, I64 -> ProtoScalarValue.ofInt64Value(0L);

0 commit comments

Comments
 (0)