david-mollitor-db opened a new pull request, #59224:
URL: https://github.com/apache/spark/pull/59224
### What changes were proposed in this pull request?
This adds a private `UTF8String.getByteUnchecked(int)` that reads a byte
without `getByte`'s range check, and uses it at the internal call sites whose
loop already keeps the index in range.
- The public `getByte` keeps its range check, still returns 0 for an
out-of-range index, and now calls `getByteUnchecked` for the read itself.
`getByteUnchecked` has no check of its own, not even an `assert`: an `assert`
makes it 41 bytes of bytecode, over the JIT's 35-byte `MaxInlineSize`, and the
JIT then sometimes left it as a real call (`callee is too large`) in loops such
as `getIsValid`.
- 45 internal call sites switch to `getByteUnchecked`. Each one's loop
condition, or a check just before it, proves `0 <= index < numBytes`. They are
in `numChars`, `isValid` / `makeValid`, `isFullAscii`, the code point
iterators, `substring`, `codePointFrom`, ASCII case conversion, `findInSet`,
`trim` / `trimAll` / `trimLeft` / `trimRight` (with and without a trim string),
`reverse`, `indexOf`, `charPosToByte` / `bytePosToChar`, `toInt` / `toLong` and
`soundex`.
- 11 call sites stay on `getByte`, because they can index past the end or
their range depends on an invariant outside the loop:
- `contains`, which #59222 (SPARK-59967) replaces
- `getChar`, whose walk is bounded by the cached `numChars` rather than by
`numBytes`
- `trimTrailingSpaces`, whose loop bound `numBytes - numSpaces` can be
negative
- `prevCharStart`, whose range depends on its callers
- the read after a match in `indexOf(pattern, start, occurrence)`, which
can run past the end when a truncated multi-byte sequence ends the string
- the walks in `levenshteinDistance`, which are bounded by character
counts, and one of which reads one character past the end
### Why are the changes needed?
These loops back common string functions and casts (`length`, `substring`,
`trim`, `instr` / `locate`, `CAST(string AS INT/BIGINT)`, case conversion,
UTF-8 validation). The range check costs a compare and a branch on every byte
read. The JIT can't always remove it, especially in character walks, where each
step advances by the width of the current character.
Local JMH results (JDK 17, ns per call, ~128-byte rows; a fresh `UTF8String`
per call so cached `numChars` / `isValid` / `isFullAscii` don't skip the scan):
_Results table to follow: a final interleaved before/after run is in
progress._
### Does this PR introduce _any_ user-facing change?
No. Only performance changes; the public `getByte` behaves the same.
### How was this patch tested?
- New `UTF8StringSuite.internalByteReadsOnEdgeCaseInputs` runs every method
that uses `getByteUnchecked` on empty, all-whitespace, sign-only, and truncated
or invalid multi-byte inputs, both on heap at a non-zero offset and off heap.
It checks the results for the trim methods and for `toInt` / `toLong` on
whitespace and lone signs.
- Each converted call site was checked by hand: its loop condition, or a
check just before it, keeps the index in `[0, numBytes)`. During development,
`getByteUnchecked` temporarily asserted its index, and this test failed when
the read after a match in `indexOf(pattern, start, occurrence)` was switched
over. That is why that read stays on `getByte`.
- Existing `UTF8StringSuite`, `UTF8StringPropertyCheckSuite`,
`ByteArraySuite`, `CollationSupportSuite` and `CollationFactorySuite` pass.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Isaac (Claude Code, Claude Opus 5.5)
This pull request and its description were written by Isaac.
--
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]
