iemejia opened a new pull request, #3521: URL: https://github.com/apache/parquet-java/pull/3521
## Summary Resolves #3520. Two small cleanups in the binary write path. ### 1. `DeltaByteArrayWriter`: avoid unconditional copy of input bytes The first line of `writeBytes(Binary v)` is: ```java byte[] vb = v.getBytes(); ``` `Binary.getBytes()` is contractually required to return a fresh array the caller can keep and mutate. For `ByteArrayBackedBinary` (the most common case from `Binary.fromConstantByteArray()` and similar), the implementation does an unconditional `Arrays.copyOf` of the backing array — but `DeltaByteArrayWriter` only reads `vb` and then drops it. The copy is wasted work on every value. The right call here is `v.copy().getBytesUnsafe()`: - `Binary.copy()` is a no-op (`return this`) for constant Binaries that are already independent of any reused buffer. - For reused-buffer Binaries (e.g. `ByteBufferBackedBinary` over a slab being mutated), `copy()` snapshots them — preserving correctness. - `getBytesUnsafe()` then returns the backing array directly without a defensive copy. For the common `ByteArrayBackedBinary` case this skips the entire copy. For other implementations the copy still happens but only when it's actually needed for correctness. ### 2. `FixedLenByteArrayPlainValuesWriter`: drop the unused `LittleEndianDataOutputStream` wrapper Same pattern as #3517 fixed in `DeltaLengthByteArrayValuesWriter`: the writer wraps its `CapacityByteArrayOutputStream` with a `LittleEndianDataOutputStream` that's only used to call `Binary.writeTo()` — i.e. the LE wrapper adds a layer of dispatch on every value but never uses any LE-specific functionality (`writeInt`/`writeLong`/etc.). `Binary.writeTo(arrayOut)` works directly with the underlying stream. The trailing `out.flush()` in `getBytes()` is also dead — `CapacityByteArrayOutputStream` doesn't buffer. ## Benchmark `BinaryEncodingBenchmark.encodeDeltaByteArray` (short strings): roughly **+5% to +10%** standalone — the per-value `getBytes()` copy is one of several overheads; this PR removes one of them, stacking with #3517 which removed the suffix-side allocation. `FixedLenByteArrayPlainValuesWriter`: code-quality cleanup; removes a per-value layer of dispatch and an unnecessary `flush()` call. No headline benchmark. ## Validation - `parquet-column`: 573 tests pass - Built with `-Dspotless.check.skip=true -Drat.skip=true -Djapicmp.skip=true` ## User-facing changes None. No public API change. No file format change. ### Closes #3520 Part of a small series of focused performance PRs from work in [parquet-perf](https://github.com/iemejia/parquet-perf). Previous: #3494, #3496, #3500, #3504, #3506, #3510, #3514, #3517, #3519. Companion benchmarks contribution: #3512. -- 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]
