Uh oh!
There was an error while loading. Please reload this page.
GH-3499: Cache hashCode() for non-reused Binary instances (up to 73x dictionary-encode speedup) - #3500
GH-3499: Cache hashCode() for non-reused Binary instances (up to 73x dictionary-encode speedup)#3500iemejia wants to merge 1 commit into
Conversation
arouel
left a comment
There was a problem hiding this comment.
I use a similar optimization already in a patched parquet-column version on my side and verified the improvement.
Uh oh!
There was an error while loading. Please reload this page.
e1c3ed9 to
a8152c9Compare… shaded jar The parquet-benchmarks pom is missing the JMH annotation-processor configuration and the AppendingTransformer entries for BenchmarkList / CompilerHints. As a result, the shaded jar built from master fails at runtime with "Unable to find the resource: /META-INF/BenchmarkList". This commit: - Fixes parquet-benchmarks/pom.xml so the shaded jar is runnable: adds jmh-generator-annprocess to maven-compiler-plugin's annotation processor paths, and adds AppendingTransformer entries for META-INF/BenchmarkList and META-INF/CompilerHints to the shade plugin. - Adds 11 JMH benchmarks covering the encode/decode paths used by the pending performance optimization PRs (apache#3494, apache#3496, apache#3500, apache#3504, apache#3506, apache#3510), so reviewers can reproduce the reported numbers and detect regressions: IntEncodingBenchmark, BinaryEncodingBenchmark, ByteStreamSplitEncodingBenchmark, ByteStreamSplitDecodingBenchmark, FixedLenByteArrayEncodingBenchmark, FileReadBenchmark, FileWriteBenchmark, RowGroupFlushBenchmark, ConcurrentReadWriteBenchmark, BlackHoleOutputFile, TestDataFactory. After this change the shaded jar registers 87 benchmarks (was 0 from a working build, or unrunnable at all from a default build).
… shaded jar The parquet-benchmarks pom is missing the JMH annotation-processor configuration and the AppendingTransformer entries for BenchmarkList / CompilerHints. As a result, the shaded jar built from master fails at runtime with "Unable to find the resource: /META-INF/BenchmarkList". This commit: - Fixes parquet-benchmarks/pom.xml so the shaded jar is runnable: adds jmh-generator-annprocess to maven-compiler-plugin's annotation processor paths, and adds AppendingTransformer entries for META-INF/BenchmarkList and META-INF/CompilerHints to the shade plugin. - Adds 11 JMH benchmarks covering the encode/decode paths used by the pending performance optimization PRs (apache#3494, apache#3496, apache#3500, apache#3504, apache#3506, apache#3510), so reviewers can reproduce the reported numbers and detect regressions: IntEncodingBenchmark, BinaryEncodingBenchmark, ByteStreamSplitEncodingBenchmark, ByteStreamSplitDecodingBenchmark, FixedLenByteArrayEncodingBenchmark, FileReadBenchmark, FileWriteBenchmark, RowGroupFlushBenchmark, ConcurrentReadWriteBenchmark, BlackHoleOutputFile, TestDataFactory. After this change the shaded jar registers 87 benchmarks (was 0 from a working build, or unrunnable at all from a default build).
a8152c9 to
75bd3c7ComparePLAIN_DICTIONARY encoding of BINARY columns repeatedly hashes Binary keys during dictionary map lookups, but the existing Binary.hashCode() implementations (in ByteArraySliceBackedBinary, ByteArrayBackedBinary, and ByteBufferBackedBinary) recompute the hash byte-by-byte on every call. For columns with many repeated values this is the dominant cost of encodeDictionary -- we observed up to 101x slowdown vs. the cached version on the existing JMH benchmark. Cache the hash code in a single int field on Binary. Reused Binary instances (those whose backing array can be mutated by the producer between calls) do not cache, preserving the existing mutable-buffer semantics. Thread safety follows the java.lang.String.hashCode() idiom: the cache is a single int field with sentinel value 0 meaning "not yet computed". Two threads racing on the first hashCode() call may both compute and write the same deterministic value, which is benign. A Binary whose true hash equals 0 is recomputed on every call (acceptably rare and still correct). No volatile or synchronization is needed; both the field load and the field store are atomic per JLS, and the value is deterministic given the immutable byte content. Implementation notes: - The cache field is package-private (not private) so the three nested Binary subclasses can read it directly in their hashCode() hot path, avoiding an extra method-call layer that would otherwise be needed since inherited private fields are not accessible from nested subclasses. - A package-private cacheHashCode(int) helper centralises the isBackingBytesReused check on the slow path. - New tests in TestBinary cover (a) cached-and-stable hashCode for the three constant Binary impls, and (b) reused Binary not returning a stale hash after the backing buffer is replaced. Benchmark (BinaryEncodingBenchmark.encodeDictionary, 100k BINARY values per invocation, JMH -wi 3 -i 5 -f 1): Param Before (ops/s) After (ops/s) Improvement LOW / 10 66,500,137 118,856,578 +79% (1.79x) LOW / 100 13,068,757 102,086,531 +681% (7.81x) LOW / 1000 1,442,427 145,961,605 +10,019% (101.2x) HIGH / 10 6,732,437 9,108,731 +35% (1.35x) The relative gain grows with string length because the per-value hash cost (byte-loop length) grows linearly while the cached lookup is O(1). LOW cardinality benefits even more because each unique key is hashed many more times (once per insertion check across the 100k values). Negative control: BinaryEncodingBenchmark.encodePlain (which writes Binary without dictionary lookups, so does not exercise hashCode) is unchanged within noise across all parameter combinations. All 575 parquet-column tests pass (was 573; +2 new tests for the cache).
75bd3c7 to
769086bCompare… shaded jar The parquet-benchmarks pom is missing the JMH annotation-processor configuration and the AppendingTransformer entries for BenchmarkList / CompilerHints. As a result, the shaded jar built from master fails at runtime with "Unable to find the resource: /META-INF/BenchmarkList". This commit: - Fixes parquet-benchmarks/pom.xml so the shaded jar is runnable: adds jmh-generator-annprocess to maven-compiler-plugin's annotation processor paths, and adds AppendingTransformer entries for META-INF/BenchmarkList and META-INF/CompilerHints to the shade plugin. - Adds 11 JMH benchmarks covering the encode/decode paths used by the pending performance optimization PRs (apache#3494, apache#3496, apache#3500, apache#3504, apache#3506, apache#3510), so reviewers can reproduce the reported numbers and detect regressions: IntEncodingBenchmark, BinaryEncodingBenchmark, ByteStreamSplitEncodingBenchmark, ByteStreamSplitDecodingBenchmark, FixedLenByteArrayEncodingBenchmark, FileReadBenchmark, FileWriteBenchmark, RowGroupFlushBenchmark, ConcurrentReadWriteBenchmark, BlackHoleOutputFile, TestDataFactory. After this change the shaded jar registers 87 benchmarks (was 0 from a working build, or unrunnable at all from a default build).
iemejia
commented
May 17, 2026
Closing in favor of #3566. I initially submitted a series of small, focused PRs thinking they'd be easier to review. In practice the sheer number (~16 PRs, with more pending) made things harder to follow — even for me. I've regrouped the changes by encoding type / performance area so that each PR is self-contained with its own benchmarks and test coverage, which should make review and performance analysis much more straightforward. Apologies for the churn. If you've been reviewing this PR, please continue the discussion on #3566 which supersedes it. Thank you. |
Summary
Closes#3499.
Caches
Binary.hashCode()per instance for non-reused (immutable-backing)Binaryvalues. Eliminates repeated full-buffer hash recomputation duringPLAIN_DICTIONARYencoding, where the same key is hashed many times across a 100k-value page. Reused (mutable-backing) instances skip the cache to preserve their existing semantics.Uses the
java.lang.String.hashCode()idiom — a singleintfield with sentinel0meaning "not yet computed" — so the cache is race-safe withoutvolatile(concurrent first calls compute the same deterministic value; either ordering is correct).Benchmark
BinaryEncodingBenchmark.encodeDictionary, 100k BINARY values per invocation, JDK 18, JMH-wi 5 -i 10 -f 3(30 samples per row):The relative gain grows with string length (per-call hash work is O(N), cache lookup is O(1)) and with low cardinality (each unique key is hashed many more times).
Negative control:
encodePlain(writes Binary without dictionary lookups, so doesn't exercisehashCode) is unchanged within ±2.5% across all parameter combinations. Allocation rate per op (gc.alloc.rate.norm) is identical between baseline and optimized — speedup is pure CPU saved.Implementation notes
transient int cachedHashCodeonBinary(package-private so the three nested subclasses can read it directly on the hot path; inherited private fields are not accessible from nested subclasses without a method-call indirection).!isBackingBytesReusedinside a small package-privatecacheHashCode(int)helper that runs only on the cache-miss path.TestBinary:testHashCodeCachedForConstantBinary: constant Binary returns stablehashCode, equal across the three impls (ByteArraySliceBackedBinary,ByteArrayBackedBinary,ByteBufferBackedBinary).testHashCodeNotCachedForReusedBinary: reused Binary returns the new hash after the backing buffer is replaced.Validation
parquet-column: 575 tests pass (was 573; +2 new tests for the cache).-Dspotless.check.skip=true -Drat.skip=true -Djapicmp.skip=true.Related
This is the third in a small series of focused performance PRs from work in https://github.com/iemejia/parquet-perf. Previous: #3494 (PlainValuesReader), #3496 (PlainValuesWriter).
How to reproduce the benchmarks
The JMH benchmarks cited above are being added to
parquet-benchmarksin #3512. Once that lands, reproduce with:Compare runs against
master(baseline) and this branch (optimized).