david-mollitor-db opened a new pull request, #58793:
URL: https://github.com/apache/spark/pull/58793
### What changes were proposed in this pull request?
`SparkDateTimeUtils.getZoneId` normalizes the single-digit UTC-offset
formats that
`ZoneId.of` does not accept -- a single-digit hour (`+7:30` -> `+07:30`) and
a single-digit
trailing minute (`+07:3` -> `+07:03`). It previously did this by running two
regexes on
every call:
```scala
final val singleHourTz = Pattern.compile("(\\+|\\-)(\\d):")
final val singleMinuteTz = Pattern.compile("(\\+|\\-)(\\d\\d):(\\d)$")
// ...
var formattedZoneId = singleHourTz.matcher(timeZoneId).replaceFirst("$10$2:")
formattedZoneId =
singleMinuteTz.matcher(formattedZoneId).replaceFirst("$1$2:0$3")
```
This PR replaces the two regexes with a small string walk
(`normalizeLegacyZoneOffset`)
that applies the same zero-padding only when a single-digit offset is
actually present and
returns the input string unchanged (the same instance) otherwise. The
`Pattern` fields are
removed.
### Why are the changes needed?
`Pattern.matcher()` allocates a fresh `Matcher` on every call, and
`getZoneId` is on the
per-row timestamp-parse hot path (`UnivocityParser` ->
`TimestampFormatter.parse` ->
`stringToTimestamp` -> `parseTimestampString` -> `getZoneId`). A timestamp
column therefore
allocates two throwaway `Matcher`s per row to run a normalization that
changes nothing for
the common inputs (`Z`, `+07:30`, named zones, `UTC`).
JFR profiling of a CSV timestamp-parsing workload showed `getZoneId` ->
`Pattern.matcher`
as the single largest `java.util.regex.Matcher` allocation site (~97% of all
`Matcher`
allocation). Re-profiling the same workload after this change shows the
per-call `Matcher`
allocation eliminated (~98% drop in total `Matcher` allocation) with no CPU
regression
(`getZoneId` is not a hot method before or after). The change also benefits
the many
one-time callers (`CSVOptions` / `JSONOptions` / `XmlOptions`,
session-timezone
resolution, etc.).
### Does this PR introduce _any_ user-facing change?
No. Behavior is preserved exactly, including the single-digit offset
normalization and the
`INVALID_TIMEZONE` error raised for invalid zones.
### How was this patch tested?
- Existing `DateTimeUtilsSuite` and `TimestampFormatterSuite` pass; they
cover the
single-digit offset formats (e.g. `-1:0`, `+7:30`, `+8:00`).
- Added a test in `DateTimeUtilsSuite` that asserts the string walk produces
results
identical to an independent regex reference across a broad set of offset
shapes,
`GMT`/`UTC` prefixes, named zones, invalid strings, and a non-ASCII-digit
input.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Isaac
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]
