Skip to content

Commit d8f8408

Browse files
dfa1claude
andcommitted
refactor(core): add PType.isUnsigned, drop 3 private copies; test new helpers
isUnsigned(ptype) was copy-pasted in the Delta, Bitpacked and FrameOfReference encoders. It is exactly the complement of the existing PType.isSigned() (every non-unsigned ptype is a signed integer or floating-point), so add PType.isUnsigned() returning !isSigned() and route the call sites through it. Tests: - PTypeTest: isUnsigned true/false partitions + an exact-complement-of-isSigned property over every enum constant. - PrimitiveArraysTest (new, for the prior extraction): toLongs sign/zero-extension per width, I64 passthrough identity, floating-ptype throw carrying the caller's EncodingId; fromLongs round-trip through toLongs, little-endian I64 layout, and low-byte-only narrowing. Ground truth green: JavaWritesRustReads (213) + JavaRoundTrip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a74263c commit d8f8408

6 files changed

Lines changed: 204 additions & 25 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ public boolean isSigned() {
5656
|| this == F16 || this == F32 || this == F64;
5757
}
5858

59+
/// Returns `true` for the unsigned integer types (`U8`–`U64`) — the complement of
60+
/// [#isSigned()], since every non-unsigned ptype is either a signed integer or floating-point.
61+
///
62+
/// @return `true` if this ptype is an unsigned integer
63+
public boolean isUnsigned() {
64+
return !isSigned();
65+
}
66+
5967
/// Returns the [PType] for the given enum ordinal — the integer value the wire format
6068
/// uses to identify a physical type.
6169
///

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,27 @@ void isSigned_falseForUnsigned(PType ptype) {
5151
assertThat(ptype.isSigned()).isFalse();
5252
}
5353

54+
@ParameterizedTest
55+
@EnumSource(value = PType.class, names = {"U8", "U16", "U32", "U64"})
56+
void isUnsigned_trueForUnsigned(PType ptype) {
57+
// Given / When / Then
58+
assertThat(ptype.isUnsigned()).isTrue();
59+
}
60+
61+
@ParameterizedTest
62+
@EnumSource(value = PType.class, names = {"I8", "I16", "I32", "I64", "F16", "F32", "F64"})
63+
void isUnsigned_falseForSignedAndFloats(PType ptype) {
64+
// Given / When / Then
65+
assertThat(ptype.isUnsigned()).isFalse();
66+
}
67+
68+
@ParameterizedTest
69+
@EnumSource(PType.class)
70+
void isUnsigned_isExactComplementOfIsSigned(PType ptype) {
71+
// Given / When / Then — the two must partition every ptype; isUnsigned is defined as !isSigned
72+
assertThat(ptype.isUnsigned()).isNotEqualTo(ptype.isSigned());
73+
}
74+
5475
@ParameterizedTest
5576
@EnumSource(PType.class)
5677
void fromOrdinal_roundTrips(PType ptype) {
Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
package io.github.dfa1.vortex.encoding;
2+
3+
import io.github.dfa1.vortex.core.PType;
4+
import io.github.dfa1.vortex.core.VortexException;
5+
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.params.ParameterizedTest;
7+
import org.junit.jupiter.params.provider.EnumSource;
8+
9+
import java.lang.foreign.Arena;
10+
import java.lang.foreign.MemorySegment;
11+
import java.lang.foreign.ValueLayout;
12+
13+
import static org.assertj.core.api.Assertions.assertThat;
14+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
15+
16+
class PrimitiveArraysTest {
17+
18+
@Test
19+
void toLongs_i8_signExtends() {
20+
// Given a byte array with a negative value
21+
byte[] data = {0, 1, -1, Byte.MIN_VALUE, Byte.MAX_VALUE};
22+
23+
// When
24+
long[] result = PrimitiveArrays.toLongs(data, PType.I8, EncodingId.FASTLANES_DELTA);
25+
26+
// Then negatives sign-extend to 64 bits
27+
assertThat(result).containsExactly(0L, 1L, -1L, -128L, 127L);
28+
}
29+
30+
@Test
31+
void toLongs_u8_zeroExtends() {
32+
// Given a byte array whose high bit is set (would be negative if signed)
33+
byte[] data = {0, 1, -1, Byte.MIN_VALUE};
34+
35+
// When
36+
long[] result = PrimitiveArrays.toLongs(data, PType.U8, EncodingId.FASTLANES_DELTA);
37+
38+
// Then the raw byte is zero-extended into 0..255
39+
assertThat(result).containsExactly(0L, 1L, 255L, 128L);
40+
}
41+
42+
@Test
43+
void toLongs_i16_signExtends() {
44+
// Given
45+
short[] data = {0, -1, Short.MIN_VALUE, Short.MAX_VALUE};
46+
47+
// When
48+
long[] result = PrimitiveArrays.toLongs(data, PType.I16, EncodingId.FASTLANES_DELTA);
49+
50+
// Then
51+
assertThat(result).containsExactly(0L, -1L, -32768L, 32767L);
52+
}
53+
54+
@Test
55+
void toLongs_u16_zeroExtends() {
56+
// Given a value with the high bit set
57+
short[] data = {-1, Short.MIN_VALUE};
58+
59+
// When
60+
long[] result = PrimitiveArrays.toLongs(data, PType.U16, EncodingId.FASTLANES_DELTA);
61+
62+
// Then zero-extended into 0..65535
63+
assertThat(result).containsExactly(65535L, 32768L);
64+
}
65+
66+
@Test
67+
void toLongs_i32_signExtends() {
68+
// Given
69+
int[] data = {0, -1, Integer.MIN_VALUE, Integer.MAX_VALUE};
70+
71+
// When
72+
long[] result = PrimitiveArrays.toLongs(data, PType.I32, EncodingId.FASTLANES_DELTA);
73+
74+
// Then
75+
assertThat(result).containsExactly(0L, -1L, (long) Integer.MIN_VALUE, (long) Integer.MAX_VALUE);
76+
}
77+
78+
@Test
79+
void toLongs_u32_zeroExtends() {
80+
// Given a value with the high bit set
81+
int[] data = {-1, Integer.MIN_VALUE};
82+
83+
// When
84+
long[] result = PrimitiveArrays.toLongs(data, PType.U32, EncodingId.FASTLANES_DELTA);
85+
86+
// Then zero-extended into 0..2^32-1
87+
assertThat(result).containsExactly(0xFFFF_FFFFL, 0x8000_0000L);
88+
}
89+
90+
@Test
91+
void toLongs_i64_returnsSameArrayNoCopy() {
92+
// Given a long array
93+
long[] data = {1L, -1L, Long.MIN_VALUE, Long.MAX_VALUE};
94+
95+
// When
96+
long[] result = PrimitiveArrays.toLongs(data, PType.I64, EncodingId.FASTLANES_DELTA);
97+
98+
// Then the I64/U64 path is a passthrough — no copy
99+
assertThat(result).isSameAs(data);
100+
}
101+
102+
@ParameterizedTest
103+
@EnumSource(value = PType.class, names = {"F16", "F32", "F64"})
104+
void toLongs_floatingPtypes_throwWithSuppliedEncodingId(PType ptype) {
105+
// Given floating ptypes are not integer-widen targets; When/Then it throws, attributed to
106+
// the caller's encoding id (here FrameOfReference) rather than a hardcoded one
107+
assertThatThrownBy(() -> PrimitiveArrays.toLongs(new float[1], ptype, EncodingId.FASTLANES_FOR))
108+
.isInstanceOf(VortexException.class)
109+
.hasMessageContaining("unsupported ptype: " + ptype);
110+
}
111+
112+
@ParameterizedTest
113+
@EnumSource(value = PType.class, names = {"I8", "U8", "I16", "U16", "I32", "U32", "I64", "U64"})
114+
void fromLongs_roundTripsThroughToLongs(PType ptype) {
115+
// Given values that exercise the low bytes at each width
116+
long[] original = {0L, 1L, 2L, 7L, 42L};
117+
118+
try (Arena arena = Arena.ofConfined()) {
119+
// When written to a segment and read back at the ptype's width
120+
MemorySegment seg = PrimitiveArrays.fromLongs(original, ptype, arena);
121+
122+
// Then the segment has one element per value at the expected width...
123+
assertThat(seg.byteSize()).isEqualTo((long) original.length * ptype.byteSize());
124+
// ...and each element round-trips (values are small + positive, so width-narrowing is lossless)
125+
for (int i = 0; i < original.length; i++) {
126+
assertThat(readElement(seg, ptype, i)).isEqualTo(original[i]);
127+
}
128+
}
129+
}
130+
131+
@Test
132+
void fromLongs_i64_writesLittleEndian() {
133+
// Given a single value with distinct bytes
134+
long[] original = {0x0102_0304_0506_0708L};
135+
136+
try (Arena arena = Arena.ofConfined()) {
137+
// When written via the bulk I64 path
138+
MemorySegment seg = PrimitiveArrays.fromLongs(original, PType.I64, arena);
139+
140+
// Then it is stored little-endian (lowest byte first)
141+
assertThat(seg.get(ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) 0x08);
142+
assertThat(seg.getAtIndex(PTypeIO.LE_LONG, 0)).isEqualTo(0x0102_0304_0506_0708L);
143+
}
144+
}
145+
146+
@Test
147+
void fromLongs_narrowWidth_keepsOnlyLowBytes() {
148+
// Given a value whose high bytes exceed the target width
149+
long[] original = {0x1234_5678L};
150+
151+
try (Arena arena = Arena.ofConfined()) {
152+
// When narrowed to I8 (1 byte/elem)
153+
MemorySegment seg = PrimitiveArrays.fromLongs(original, PType.I8, arena);
154+
155+
// Then only the low byte survives
156+
assertThat(seg.byteSize()).isEqualTo(1L);
157+
assertThat(seg.get(ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) 0x78);
158+
}
159+
}
160+
161+
private static long readElement(MemorySegment seg, PType ptype, int i) {
162+
return switch (ptype) {
163+
case I8, U8 -> seg.get(ValueLayout.JAVA_BYTE, i);
164+
case I16, U16 -> seg.getAtIndex(PTypeIO.LE_SHORT, i);
165+
case I32, U32 -> seg.getAtIndex(PTypeIO.LE_INT, i);
166+
case I64, U64 -> seg.getAtIndex(PTypeIO.LE_LONG, i);
167+
default -> throw new IllegalArgumentException("not an integer ptype: " + ptype);
168+
};
169+
}
170+
}

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
4848
int n = longs.length;
4949
int typeBits = ptype.byteSize() * 8;
5050
long typeMask = typeMask(typeBits);
51-
boolean unsign = isUnsigned(ptype);
51+
boolean unsign = ptype.isUnsigned();
5252

5353
long signedMin = 0L;
5454
long signedMax = 0L;
@@ -243,15 +243,8 @@ private static long typeMask(int typeBits) {
243243
return typeBits == 64 ? -1L : (1L << typeBits) - 1L;
244244
}
245245

246-
private static boolean isUnsigned(PType ptype) {
247-
return switch (ptype) {
248-
case U8, U16, U32, U64 -> true;
249-
default -> false;
250-
};
251-
}
252-
253246
private static byte[] statsBytes(PType ptype, long value) {
254-
if (isUnsigned(ptype)) {
247+
if (ptype.isUnsigned()) {
255248
return ScalarValue.ofUint64Value(value).encode();
256249
}
257250
return ScalarValue.ofInt64Value(value).encode();

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
4242
int typeBits = typeBits(ptype);
4343
int lanes = lanes(ptype);
4444
long mask = typeMask(ptype);
45-
boolean unsign = isUnsigned(ptype);
45+
boolean unsign = ptype.isUnsigned();
4646

4747
long minVal = 0L;
4848
long maxVal = 0L;
@@ -117,15 +117,8 @@ private static void deltaChunk(long[] transposed, long[] bases, int lanes, int t
117117
}
118118
}
119119

120-
private static boolean isUnsigned(PType ptype) {
121-
return switch (ptype) {
122-
case U8, U16, U32, U64 -> true;
123-
default -> false;
124-
};
125-
}
126-
127120
private static byte[] statsBytes(PType ptype, long value) {
128-
if (isUnsigned(ptype)) {
121+
if (ptype.isUnsigned()) {
129122
return ScalarValue.ofUint64Value(value).encode();
130123
}
131124
return ScalarValue.ofInt64Value(value).encode();

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public CascadeStep encodeCascade(DType dtype, Object data, EncodeContext encodeC
6060
// Skip when ref == 0 and ptype is unsigned: residuals == input, so FOR adds metadata
6161
// overhead (ref scalar + extra node) for zero compression benefit over plain bitpack.
6262
// Matches Rust IntFoRScheme's skip estimate for this case.
63-
if (ref == 0L && isUnsigned(ptype)) {
63+
if (ref == 0L && ptype.isUnsigned()) {
6464
return CascadeStep.notApplicable();
6565
}
6666
ByteBuffer meta = buildForMeta(ref, ptype);
@@ -70,12 +70,6 @@ public CascadeStep encodeCascade(DType dtype, Object data, EncodeContext encodeC
7070
return new CascadeStep(partialRoot, List.of(), List.of(slot), null, null, true);
7171
}
7272

73-
private static boolean isUnsigned(PType ptype) {
74-
return switch (ptype) {
75-
case U8, U16, U32, U64 -> true;
76-
default -> false;
77-
};
78-
}
7973

8074
private static long computeRef(long[] longs, int n) {
8175
long ref = n > 0 ? longs[0] : 0L;

0 commit comments

Comments
 (0)