perf(common): avoid UTF-8 allocations in string comparator - #19414
Conversation
d9e0157 to
769684d
Compare
cshuo
changed the title
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for working on this! This PR replaces the allocating UTF-8 string comparator with an allocation-free algorithm ported from Firestore that compares UTF-16 code units directly while preserving HFile-compatible unsigned UTF-8 byte ordering. I traced the surrogate handling (BMP vs supplementary, surrogate-vs-surrogate ordering, and the length tiebreak) against the encoded-byte order and it holds for well-formed UTF-16, and the exhaustive pairwise test is solid validation. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.
cc @yihua
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19414 +/- ##
============================================
+ Coverage 74.83% 75.43% +0.60%
- Complexity 32332 32600 +268
============================================
Files 2574 2574
Lines 142978 142998 +20
Branches 17527 17576 +49
============================================
+ Hits 106992 107874 +882
+ Misses 27921 27085 -836
+ Partials 8065 8039 -26
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
voonhous
left a comment
voonhous
left a comment
There was a problem hiding this comment.
Took a deeper pass on this since I reviewed #18941. The algorithm itself checks out: I brute forced it against the encoded byte order for well-formed input, mutation tested the new sweep, and confirmed no call site depends on the return magnitude. The comments below are about attribution, the malformed-input contract this rewrite quietly changes, and a few ways to sharpen the tests.
Preserve the null-rejection contract, document malformed surrogate behavior, add Firebase attribution, and strengthen the HFile-ordering tests.
github-actions
Bot
added
size:M
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for working on this! This PR replaces the allocation-heavy UTF-8 string comparator with an allocation-free port of Firestore's compareUtf8Strings that derives the same ordering directly from UTF-16 code units, with exhaustive tests validating against the production HFile key comparator. I traced the supplementary-character special case and boundary interactions and they preserve the HFile-compatible unsigned UTF-8 byte order; the null contract is also preserved. The main points (unpaired-surrogate divergence, (null,null) handling, LICENSE attribution, test-oracle independence) were already raised and addressed in prior rounds. No new issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.
cc @yihua
* perf(common): avoid UTF-8 allocations in string comparator * fix(common): address UTF-8 comparator review feedback Preserve the null-rejection contract, document malformed surrogate behavior, add Firebase attribution, and strengthen the HFile-ordering tests. (cherry picked from commit 377fc04)
* perf(common): avoid UTF-8 allocations in string comparator * fix(common): address UTF-8 comparator review feedback Preserve the null-rejection contract, document malformed surrogate behavior, add Firebase attribution, and strengthen the HFile-ordering tests. (cherry picked from commit 377fc04)
6 participants
Describe the issue this Pull Request addresses
Closes #19409.
The UTF-8 string comparator introduced in #18941 encodes both operands on every comparison, allocating two temporary
byte[]arrays before scanning them in unsigned byte order. Sorting invokes the comparatorO(N log N)times, so large metadata record-index operations generate substantial short-lived allocations and GC pressure.The same UTF-8 ordering can be derived directly from well-formed UTF-16 code units. This avoids encoding and allocation while preserving the HFile-compatible ordering required for non-ASCII and supplementary record keys.
Summary and Changelog
compareUtf8Stringsalgorithm from Google Firebase Firestore, with the source recorded next to the implementation.Local benchmark
Environment: Java 11.0.27, macOS ARM64, G1 GC. Each microbenchmark result is the median of five measured trials and was repeated across three independent JVM forks. The benchmark harness is local-only and not included in this PR.
End-to-end
Arrays.sortresults:The approximately 1 MiB remaining in the sort benchmark comes from the sorting implementation's temporary storage; the new comparator itself allocates 0 B/op.
Validation:
Result: 24 tests run, 0 failures, 0 errors, 0 skipped; Checkstyle reported 0 violations.
Impact
Risk Level
Low. The implementation is ported from Firebase Firestore and is validated exhaustively against encoded unsigned UTF-8 byte order for valid Unicode strings. Like the Firestore implementation, it assumes well-formed UTF-16 input; malformed/unpaired surrogates are outside the expected Spark/Avro ingestion path.
Documentation Update
none
Contributor's checklist