iemejia opened a new pull request, #3494: URL: https://github.com/apache/parquet-java/pull/3494
### Rationale for this change Closes #3493. `PlainValuesReader` (used for PLAIN-encoded INT32, INT64, FLOAT, and DOUBLE columns, and for decoding the dictionary page of every dictionary-encoded numeric column) currently reads each value through a `LittleEndianDataInputStream` wrapper around a `ByteBufferInputStream`. Per value, `readInt()` performs 4 separate virtual `in.read()` calls and reassembles the result with bit shifts. The `LittleEndianDataInputStream.readInt()` method itself carries a TODO comment from years ago suggesting exactly this kind of replacement. ### What changes are included in this PR? **Commit 1** — `Optimize PlainValuesReader with direct ByteBuffer reads` In `PlainValuesReader.initFromPage()`, obtain the page data as a single contiguous `ByteBuffer` via `stream.slice(stream.available())` with `ByteOrder.LITTLE_ENDIAN`, and call the corresponding `ByteBuffer` accessor directly per value: - `readInteger()` → `buffer.getInt()` - `readLong()` → `buffer.getLong()` - `readFloat()` → `buffer.getFloat()` - `readDouble()` → `buffer.getDouble()` - `skip(n)` → `buffer.position(buffer.position() + n * typeSize)` `ByteBuffer.getInt()` with the appropriate byte order is a HotSpot intrinsic that compiles to a single unaligned load instruction on x86/ARM — no virtual dispatch, no per-byte assembly, no checked `IOException` on the per-value path. `ByteBufferInputStream.slice()` already handles both single-buffer (zero-copy view) and multi-buffer (single contiguous copy) cases transparently. **Commit 2** — `Deprecate LittleEndianDataInputStream and migrate last test usage` After commit 1, `LittleEndianDataInputStream` has no remaining production usages. This commit: - Adds `@Deprecated` and detailed javadoc pointing to the faster `ByteBuffer` + `LITTLE_ENDIAN` alternative - Migrates the only remaining usage in `TestColumnChunkPageWriteStore.intValue()` to `BytesInput.toByteBuffer().order(LITTLE_ENDIAN).getInt()`, eliminating a `BytesInput → ByteArrayOutputStream → ByteArrayInputStream → LittleEndianDataInputStream` round-trip The class is left in place (only `@Deprecated`) for source/binary compatibility of any downstream code that may still reference it. It can be removed in a future major release. ### Benchmark results `IntEncodingBenchmark.decodePlain` (100,000 INT32 values per invocation, JMH `-wi 3 -i 5 -f 1`): | Pattern | Before (ops/s) | After (ops/s) | Speedup | |------------------|---------------:|----------------:|--------:| | SEQUENTIAL | 92,918,297 | 1,143,149,235 | **12.3x** | | RANDOM | 92,126,888 | 1,147,547,093 | **12.5x** | | LOW_CARDINALITY | 93,005,451 | 1,142,666,760 | **12.3x** | | HIGH_CARDINALITY | 93,312,596 | 1,144,681,876 | **12.3x** | The speedup is consistent across data patterns because the bottleneck is entirely in the per-value dispatch overhead, not the data itself. All four numeric plain reader types (int, long, float, double) benefit equally. ### Are these changes tested? Yes. All 573 `parquet-column` and 308 `parquet-common` tests pass. The migrated `TestColumnChunkPageWriteStore.testColumnOrderV1` test passes; the two pre-existing `getSubject` failures in `TestColumnChunkPageWriteStore` on JDK 18+ are unrelated and reproduce on `master` without these changes. ### Are there any user-facing changes? `LittleEndianDataInputStream` is now `@Deprecated`. No behavioral or binary-compatibility changes for existing callers. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
