david-mollitor-db opened a new pull request, #58668:
URL: https://github.com/apache/spark/pull/58668
### What changes were proposed in this pull request?
Several `UTF8String` methods convert a character index to/from a byte offset
by walking the string one code point at a time (`i +=
numBytesForFirstByte(getByte(i))`). For a full-ASCII string every code point is
exactly one byte, so **character index == byte index** and that scan can be
replaced with O(1) arithmetic.
This PR adds an ASCII fast-path to the four char<->byte "locate" methods:
`substring(int, int)`, `getChar(int)`, `charPosToByte(int)`, and
`bytePosToChar(int)`. The fast-path is gated on the cached ASCII flag already
being warm, read without forcing a scan:
```java
if (isFullAscii == IsFullAscii.FULL_ASCII) {
// ASCII: char index == byte index, so locate directly.
...
}
```
Notes for reviewers:
- Pure win with **no regression**: when the flag is cold (`UNKNOWN`) the
methods fall back to the existing per-code-point scan. It deliberately does NOT
call `isFullAscii()`, which would force a full scan and could regress cold long
strings (e.g. `SUBSTR(s, 1, 5)`).
- The arithmetic replicates the scan's behavior exactly: a negative index
clamps to 0, the `until == Integer.MAX_VALUE` sentinel maps to `numBytes`, and
endpoints clamp to `numBytes`. For example `substring` computes `j = max(start,
0)` and `i = (until == Integer.MAX_VALUE) ? numBytes : min(until, numBytes)`;
`getChar` returns `codePointFrom(charIndex)` directly (the existing
`Objects.checkIndex` guarantees the index is in range).
- Non-ASCII and invalid-UTF-8 strings have the flag set to `NOT_ASCII` when
warmed, so they take the unchanged scan path.
**Relationship to SPARK-59378 (#58667):** the benefit depends on the ASCII
flag being warm. SPARK-59378 makes `numChars()` cache the flag as a byproduct
of its scan, which warms it for the natural callers here (`getChar()` calls
`numChars()` for its bounds check; `substringSQL(pos < 0)` calls `numChars()`
before `substring()`). Until SPARK-59378 lands, `numChars()` does not warm the
flag, so these fast-paths are **dormant** on master (still fully correct; they
fire whenever `isFullAscii()` or a case-conversion warmed the flag) and
activate automatically once it merges.
### Why are the changes needed?
`substring` and the char/byte position lookups are hot in string-heavy
workloads and currently pay an O(n) code-point walk even for ASCII data, where
the position is directly computable. This removes that walk when the string is
already known to be full ASCII, at no cost when it is not.
### Does this PR introduce _any_ user-facing change?
No. Internal optimization with no observable behavior change.
### How was this patch tested?
- Existing `UTF8StringSuite` passes, including the
`substring()`/`substringSQL()` cases.
- Adds `asciiLocateFastPathMatchesScan`, which for each input compares a
cold instance (scan path) against a warmed instance (fast-path) and asserts
identical results for `substring`, `getChar`, `charPosToByte`, and
`bytePosToChar`, over ASCII, empty, multi-byte, and invalid UTF-8 inputs and a
range of boundary positions (negatives, `Integer.MAX_VALUE`, out-of-range).
### Was this patch authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code
--
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]
