GH-3530: Optimize PLAIN encoding and decoding with direct ByteBuffer I/O#3565
Open
iemejia wants to merge 1 commit into
Open
GH-3530: Optimize PLAIN encoding and decoding with direct ByteBuffer I/O#3565iemejia wants to merge 1 commit into
iemejia wants to merge 1 commit into
Conversation
…uffer I/O Replace ByteBufferInputStream and LittleEndianDataInputStream wrappers with direct ByteBuffer access for all PLAIN value readers and writers. Readers (PlainValuesReader, BooleanPlainValuesReader, BinaryPlainValuesReader, FixedLenByteArrayPlainValuesReader) now hold a little-endian ByteBuffer obtained from initFromPage() and call getInt/getLong/getFloat/getDouble directly, eliminating per-value stream overhead. Writers (PlainValuesWriter, BooleanPlainValuesWriter, FixedLenByteArrayPlainValuesWriter) write through CapacityByteArrayOutputStream's new writeInt/writeLong methods, which put values directly into the NIO slab buffer in little-endian order, avoiding temporary byte-array allocation. Supporting changes: - CapacityByteArrayOutputStream: allocate slabs with ByteOrder.LITTLE_ENDIAN, add writeInt(int) and writeLong(long) for single-value NIO writes. - BytesInput: add zero-copy writeTo(ByteBuffer) and toByteArray() using bulk ByteBuffer.get() instead of stream copy. - LittleEndianDataOutputStream: batch single-byte writes into single write(buf, 0, N) calls for writeShort/writeInt. Includes JMH benchmarks (PlainEncodingBenchmark, PlainDecodingBenchmark) covering all 7 primitive types for both encoding and decoding.
This was referenced May 17, 2026
Closed
Closed
Fokko
reviewed
May 22, 2026
| int length = BytesUtils.readIntLittleEndian(in); | ||
| return Binary.fromConstantByteBuffer(in.slice(length)); | ||
| } catch (IOException | RuntimeException e) { | ||
| throw new ParquetDecodingException("could not read bytes at offset " + in.position(), e); |
Contributor
There was a problem hiding this comment.
Should we keep the ParquetDecodingException? Otherwise we're throwing the raw {IOException,RuntimeException} which is a behavioral change.
| if (available > 0) { | ||
| this.buffer = stream.slice(available).order(ByteOrder.LITTLE_ENDIAN); | ||
| } else { | ||
| this.buffer = ByteBuffer.allocate(0).order(ByteOrder.LITTLE_ENDIAN); |
Contributor
There was a problem hiding this comment.
Should we create a constant for the ByteBuffer.allocate(0).order(ByteOrder.LITTLE_ENDIAN);?
| try { | ||
| return Binary.fromConstantByteBuffer(in.slice(length)); | ||
| } catch (IOException | RuntimeException e) { | ||
| throw new ParquetDecodingException("could not read bytes at offset " + in.position(), e); |
Contributor
There was a problem hiding this comment.
Same as above, should we keep the wrapped ParquetDecodingException?
| try { | ||
| skipBytesFully(n * 8); | ||
| } catch (IOException e) { | ||
| throw new ParquetDecodingException("could not skip " + n + " double values", e); |
Contributor
There was a problem hiding this comment.
Same here, do we want to keep the ParquetDecodingException?
| public abstract class PlainValuesReader extends ValuesReader { | ||
| private static final Logger LOG = LoggerFactory.getLogger(PlainValuesReader.class); | ||
|
|
||
| protected LittleEndianDataInputStream in; |
Contributor
There was a problem hiding this comment.
We should go through the deprecation cycle here, but is anything using this outside of the project itself?
| * mutable {@code BAOS.getBuf()}. | ||
| */ | ||
| @Override | ||
| public byte[] toByteArray() { |
Contributor
There was a problem hiding this comment.
This overrides a deprecated API, as a follow-up we probably should move the internal calls to the new API:
@deprecated Use {@link #toByteBuffer(ByteBufferAllocator, Consumer)}| public int getNextOffset() { | ||
| return in.getNextOffset(); | ||
| public void skip(int n) { | ||
| bitIndex += n; |
Contributor
There was a problem hiding this comment.
Should we check for bounds, and throw a ParquetDecodingException in case of out of bounds?
| } catch (IOException e) { | ||
| throw new ParquetDecodingException("could not skip " + n + " double values", e); | ||
| } | ||
| buffer.position(buffer.position() + n * 8); |
Contributor
There was a problem hiding this comment.
Should we use Math.multiplyExact here and below?
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
2 participants
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.
Part of #3530 — Apache Parquet Java Performance Improvements
Summary
Replace
ByteBufferInputStreamandLittleEndianDataInputStreamwrappers with directByteBufferaccess for all PLAIN value readers and writers.Readers (
PlainValuesReader,BooleanPlainValuesReader,BinaryPlainValuesReader,FixedLenByteArrayPlainValuesReader): hold a little-endianByteBufferfrominitFromPage()and callgetInt/getLong/getFloat/getDoubledirectly, eliminating per-value stream overhead.Writers (
PlainValuesWriter,BooleanPlainValuesWriter,FixedLenByteArrayPlainValuesWriter): write throughCapacityByteArrayOutputStream's newwriteInt/writeLongmethods which put values directly into the NIO slab buffer in little-endian order, avoiding temporary byte-array allocation.Supporting changes:
CapacityByteArrayOutputStream: allocate slabs withByteOrder.LITTLE_ENDIAN, addwriteInt(int)andwriteLong(long)for single-value NIO writes.BytesInput: add zero-copywriteTo(ByteBuffer)andtoByteArray()using bulkByteBuffer.get()instead of stream copy.LittleEndianDataOutputStream: batch single-byte writes into singlewrite(buf, 0, N)calls forwriteShort/writeInt.Includes JMH benchmarks (
PlainEncodingBenchmark,PlainDecodingBenchmark) covering all 7 primitive types for both encoding and decoding.Benchmark results
Environment: JDK 25.0.3 (Temurin), OpenJDK 64-Bit Server VM, JMH 1.37, Linux x86_64.
Decoding (100K values/iteration, 3 forks x 5 iterations, throughput mode):
Encoding:
(*) decodeLong/Double show JIT variance across forks (error bars >20%); true steady-state likely ~13x consistent with INT32/FLOAT.