BitReader reads a tight packet-sized buffer without an eight-byte slack - #10
Merged
Conversation
readBits always loaded an 8-byte little-endian window at the byte cursor, and the only guard on the slack past the data was an assert, so a correct packet in an exactly-sized byte[] threw ArrayIndexOutOfBoundsException on its last fields in a release JVM. ReadStream promises hostile bytes never throw, and its constructor forwards straight to reset. reset now copies the final min(bytes, 8) bytes into a zero-padded 16-byte tail whenever the array has fewer than 8 bytes of slack, and records the byte index the tail is based at; otherwise tailBase is Integer.MAX_VALUE. readBits selects the window source with one well-predicted branch and loads a single window either way: no allocation per read, and an array with slack never takes the tail. The padding is never interpreted, since bits past the end of the data cannot reach the output of a read, so the outputs match slack bytes exactly. This is the shape serialize.go uses. Two stream tests cover the reported shape, new ReadStream(packet, packet.length): a one-byte packet reading its eight bits, and an eleven-byte packet read to its last bit with windows starting inside the last eight bytes and on the last byte, plus the band with one to seven bytes of slack. Both throw on the unfixed reader. Fixes #9 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ReadStream's Javadoc still required eight bytes of slack, and BitReader's header still said it had no scratch state; both now say what the code does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9.
What the reader gets
new ReadStream( packet, packet.length )on an exactly-sizedbyte[]now reads every field, including the last one, instead of throwingArrayIndexOutOfBoundsExceptionfromBitReader.readBitsin a release JVM. Any array size works; eight or more bytes of slack past the data is still the fastest shape, and an array with that slack takes exactly the path it took before.Mechanism
resetcopies the finalmin( bytes, 8 )bytes into a zero-padded 16-bytetailwhenever the array has fewer than eight bytes of slack, recordingtailBase, the byte index the tail is based at, and otherwise setstailBase = Integer.MAX_VALUE.readBitspicks its window source with one well-predicted branch,i < tailBase ? LONG_LE.get( data, i ) : LONG_LE.get( tail, i - tailBase ), and loads a single window either way: no allocation per read, and the padding is never interpreted, since bits past the end of the data cannot reach a read's output. This is the serialize.go shape (bitpacker.go,fillTail/readBits), sized for this reader's[1,32]field width.The slack assert in
checkResetis gone; it now assertsbytes <= data.length. Version and STANDARD.md untouched, by house rule.ReadStream's constructor andresetJavadoc still say the array "must extend at least 8 bytes pastbytes"; the promise at the top of that file is now kept as written, so I left the file alone. That@paramwording is now a performance recommendation rather than a requirement and can follow in its own change if wanted.Tests
Two new tests in
test/serialize/tests/StreamTests.java, both on the reported shapenew ReadStream( packet, packet.length ):On the unfixed reader both throw: the one-byte read needs indices 0..7 of a one-byte array, and the eleven-byte packet's 17-bit read at byte 4 needs 4..11.
What was run, and what was not
Nothing was compiled or run here. This machine has no JDK:
/usr/bin/javais the macOS stub,dist/jdk-21.0.12.1is absent, and there is no/Library/Java, Homebrew, IntelliJ or Android Studio JDK, and no container runtime. So I have not runmake testormake test-release, and I have not confirmed the two tests fail on main by execution; the failure on main follows from the index arithmetic above. CI's checked and release runs are the verification for this PR.