david-mollitor-db opened a new pull request, #58667:
URL: https://github.com/apache/spark/pull/58667
### What changes were proposed in this pull request?
`UTF8String.numChars()` scans every lead byte of the string to count code
points. Determining whether a string is full ASCII (`isFullAscii()` /
`getIsFullAscii()`) requires the same kind of full scan. When both are needed
for the same string, the string is scanned twice.
This PR computes the full-ASCII flag as a byproduct of the code-point count
in `getNumChars()` and caches it in the existing `isFullAscii` field, so a
subsequent `isFullAscii()` call is answered from cache instead of triggering a
second scan.
```java
private int getNumChars() {
int len = 0;
boolean ascii = true;
int i = 0;
while (i < numBytes) {
byte b = getByte(i);
ascii &= b >= 0;
i += numBytesForFirstByte(b);
len += 1;
}
if (isFullAscii == IsFullAscii.UNKNOWN) {
isFullAscii = ascii ? IsFullAscii.FULL_ASCII : IsFullAscii.NOT_ASCII;
}
return len;
}
```
Notes for reviewers:
- The scan reads each lead byte once into a local and uses it both for the
width step (`numBytesForFirstByte`) and the ASCII check.
- The ASCII predicate is `b >= 0` per lead byte, which is **exactly
equivalent** to `getIsFullAscii()`'s `getByte(i) < 0` check, including for
invalid UTF-8. Any multi-byte lead (`0xC2`–`0xF4`) or invalid byte
(`0x80`–`0xC1`, `0xF5`–`0xFF`) is negative and trips the flag; continuation
bytes of a valid sequence are skipped, but their lead byte already tripped it.
- This is **not** a `numChars == numBytes` shortcut. That test would
misclassify invalid UTF-8 such as `[0x61, 0x80]` (count 2 == 2 bytes) as ASCII,
because `numBytesForFirstByte` treats invalid bytes as width 1. Mis-flagging
would send case-conversion down the ASCII path (`convertAscii`) and corrupt the
non-ASCII byte.
- The flag is only set when still `UNKNOWN`, preserving any value already
computed by `getIsFullAscii()`. Both compute the same predicate, so they never
disagree.
- `getIsFullAscii()` is retained: it short-circuits on the first non-ASCII
byte, whereas `numChars()` must visit every code point to produce a correct
count. For callers that only need ASCII-ness, the short-circuit is strictly
cheaper, so `isFullAscii()` continues to use it. The two are complementary
writers of the same cached flag, not duplicates.
### Why are the changes needed?
`numChars()` and `isFullAscii()` each perform a full scan of the same bytes.
Callers that need both a code-point count and ASCII-ness on the same string
(for example case-conversion helpers, and prospective ASCII fast-paths)
currently pay for two scans. Folding the cheap ASCII check into the scan
`numChars()` already performs removes the redundant second scan at no extra
cost.
### Does this PR introduce _any_ user-facing change?
No. This is an internal caching optimization with no observable behavior
change.
### How was this patch tested?
- Existing `UTF8StringSuite` passes (`build/sbt 'unsafe/testOnly
org.apache.spark.unsafe.types.UTF8StringSuite'`).
- Adds `numCharsCachesAsciiness`, which asserts the flag cached by
`numChars()` matches a fresh `isFullAscii()` (i.e. `getIsFullAscii()`) across
ASCII, empty, valid multi-byte, and invalid inputs, and specifically that
`[0x61, 0x80]` is not flagged as ASCII.
### 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]
