david-mollitor-db opened a new pull request, #58669:
URL: https://github.com/apache/spark/pull/58669
### What changes were proposed in this pull request?
`UTF8String` lazily computes and caches several values in `volatile` fields:
`numChars`, `isFullAscii`, `isValid`, and `numBytesValid`. The accessor methods
read the `volatile` field two (or three) times per call — the "is it computed
yet" check plus the return — and two callers invoke `numChars()` twice. A
`volatile` read cannot be kept in a register or hoisted by the JIT, and on
AArch64 it compiles to a load-acquire (`ldar`) rather than a plain load. This
PR reads each field once into a local, halving the volatile reads on the warm
path.
```java
// before
public int numChars() {
if (numChars == -1) numChars = getNumChars();
return numChars; // second volatile read
}
// after
public int numChars() {
int chars = numChars; // single volatile read
if (chars == -1) {
chars = getNumChars();
numChars = chars;
}
return chars;
}
```
The same shape is applied to `isFullAscii()` and `isValid()`. In addition:
- `makeValidBytes()` reads `numBytesValid` once into a local (it was read by
both the `assert` and the array allocation).
- `split()` and `splitLegacyTruncate()` call `numChars()` once into a local
instead of twice.
Correctness is preserved: the field is still written on first computation,
so subsequent calls remain cached; in `isValid()` the write order is unchanged
(`getIsValid()` still sets `numBytesValid` before the `isValid` field is
assigned). Single-read sites (such as the ASCII fast-path checks in
`substring`/`getChar`) are unaffected — hoisting only helps where a field is
read more than once.
### Why are the changes needed?
These fields are read on hot paths in string-heavy workloads. Reading a
`volatile` field repeatedly within one call prevents the JIT from keeping the
value in a register and, on AArch64, incurs a load-acquire per read. Reading
once into a local removes the redundant reads at no cost and with no behavior
change. It is a small micro-optimization, most measurable on ARM.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing `UTF8StringSuite` passes (51/51). No new tests were added: these
are behavior-preserving refactors of accessors already covered by the suite
(`numChars`, `isValid`, `isFullAscii` via case conversion, `split`, and
`makeValid`).
### 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]
