Skip to content

Commit ad1956f

Browse files
Fix Java BitsWriter.writeBitsSlow bug (#271)
Fixes #268 Fixes #267 The bug was in BitsWriter that was using >>> shift operator without checking that the operand was 64. This was directly copied from Go implementation where semantics of shift operator is different.
1 parent b0f0b3c commit ad1956f

File tree

3 files changed

+115
-7
lines changed

3 files changed

+115
-7
lines changed

java/src/main/java/net/stef/BitsWriter.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ public void writeBits(long val, int nbits) {
5858
private void writeBitsSlow(long val, int nbits) {
5959
// Complete bitsBuf to 64 bits.
6060
int bitsBufFree = 64 - bitsBufUsed;
61-
bitsBuf |= val >>> (nbits - bitsBufFree);
61+
if (bitsBufFree>0) {
62+
bitsBuf |= val >>> (nbits - bitsBufFree);
63+
}
6264

6365
// And append 64 bits to stream.
6466
writeUint64(bitsBuf);

java/src/test/java/net/stef/BitsWriterTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ void testRandWriteReadBits() throws IOException {
8484
final long seed1 = System.nanoTime();
8585
Random random = new Random(seed1);
8686

87-
final long count = random.nextLong(0, 1000);
87+
final long count = random.nextLong(0, 1000000);
8888

8989
for (long i = 1; i <= count; i++) {
9090
int shift = random.nextInt(64);
91-
long v = (random.nextLong()& Long.MAX_VALUE) >>> shift;
92-
int bitCount = (v == 0) ? 0 : (int) (Math.floor(Math.log( v) / Math.log(2)) + 1);
91+
long v = (random.nextLong()) >>> shift;
92+
int bitCount = 64-Long.numberOfLeadingZeros(v);
9393
bw.writeBits(v, bitCount);
9494
}
9595
bw.close();
@@ -98,12 +98,12 @@ void testRandWriteReadBits() throws IOException {
9898
br.reset(bw.toBytes());
9999

100100
random = new Random(seed1);
101-
random.nextLong(0, 1000); // ignore, make sure we start reading from the same point
101+
random.nextLong(0, 1000000); // ignore, make sure we start reading from the same point
102102

103103
for (long i = 1; i <= count; i++) {
104104
int shift = random.nextInt(64);
105-
long v = (random.nextLong()& Long.MAX_VALUE) >>> shift;
106-
int bitCount = (v == 0) ? 0 : (int) (Math.floor(Math.log(v) / Math.log(2)) + 1);
105+
long v = (random.nextLong()) >>> shift;
106+
int bitCount = 64-Long.numberOfLeadingZeros(v);
107107
long val = br.readBits(bitCount);
108108
assertEquals(v, val, "Mismatch at index " + i + " with seed " + seed1);
109109
}
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
package net.stef;
2+
3+
import net.stef.codecs.Float64Decoder;
4+
import net.stef.codecs.Float64Encoder;
5+
import org.junit.jupiter.api.Test;
6+
7+
import java.io.ByteArrayInputStream;
8+
import java.io.ByteArrayOutputStream;
9+
import java.io.IOException;
10+
11+
import static org.junit.jupiter.api.Assertions.assertEquals;
12+
13+
public class Float64EncoderTest {
14+
// This sequence was failing before bug fix https://github.com/splunk/stef/issues/268
15+
private double testVals[] = {
16+
0.516459,
17+
0.516459,
18+
0.516459,
19+
0.026014,
20+
0.516459,
21+
0.026014,
22+
0.516459,
23+
0.026014,
24+
0.516459,
25+
0.026014,
26+
0.516459,
27+
0.026014,
28+
0.516459,
29+
0.026014,
30+
0.516459,
31+
0.026014,
32+
0.516459,
33+
0.026014,
34+
0.516459,
35+
0.026014,
36+
0.516459,
37+
0.026014,
38+
0.516459,
39+
0.026014,
40+
0.516459,
41+
0.026014,
42+
0.516459,
43+
0.404796,
44+
0.516459,
45+
0.404796,
46+
0.516459,
47+
0.404796,
48+
0.516459,
49+
};
50+
51+
@Test
52+
void testEncodeDecode() throws IOException {
53+
Float64Encoder encoder = new Float64Encoder();
54+
WriteBufs writeBufs = new WriteBufs();
55+
encoder.init(new SizeLimiter(), writeBufs.columns);
56+
for (int i = 0; i < testVals.length; i++) {
57+
encoder.encode(testVals[i]);
58+
}
59+
60+
encoder.collectColumns(writeBufs.columns);
61+
ByteArrayOutputStream out = new ByteArrayOutputStream();
62+
writeBufs.writeTo(out);
63+
64+
ReadBufs readBufs = new ReadBufs();
65+
ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
66+
readBufs.readFrom(in);
67+
68+
Float64Decoder decoder = new Float64Decoder();
69+
decoder.init(readBufs.columns);
70+
decoder.continueDecoding();
71+
for (int i = 0; i < testVals.length; i++) {
72+
double decodedVal = decoder.decode();
73+
assertEquals(testVals[i], decodedVal, "decoded value at index " + i + " does not match");
74+
}
75+
}
76+
77+
@Test
78+
void testEncodeDecodeRandomSequence() throws IOException {
79+
final int N = 100000;
80+
final long seed = System.nanoTime();;
81+
java.util.Random rand = new java.util.Random(seed);
82+
Float64Encoder encoder = new Float64Encoder();
83+
WriteBufs writeBufs = new WriteBufs();
84+
encoder.init(new SizeLimiter(), writeBufs.columns);
85+
for (int i = 0; i < N; i++) {
86+
encoder.encode(rand.nextDouble()*rand.nextInt());
87+
}
88+
encoder.collectColumns(writeBufs.columns);
89+
ByteArrayOutputStream out = new ByteArrayOutputStream();
90+
writeBufs.writeTo(out);
91+
92+
ReadBufs readBufs = new ReadBufs();
93+
ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
94+
readBufs.readFrom(in);
95+
96+
Float64Decoder decoder = new Float64Decoder();
97+
decoder.init(readBufs.columns);
98+
decoder.continueDecoding();
99+
java.util.Random rand2 = new java.util.Random(seed);
100+
for (int i = 0; i < N; i++) {
101+
double expected = rand2.nextDouble()*rand2.nextInt();
102+
double decoded = decoder.decode();
103+
assertEquals(expected, decoded, "seed "+seed+", index " + i + " does not match");
104+
}
105+
}
106+
}

0 commit comments

Comments
 (0)