On Fri, 31 Jan 2025 10:29:53 GMT, Shaojin Wen <s...@openjdk.org> wrote:
> By using the Class File API to dynamically generate a CompositePrinterParser,
> and defining DateTimePrinterParser[] printerParsers as a specific field, C2
> can do TypeProfile optimization.
>
> Since the CompositePrinterParser is generated based on the pattern, we can
> make the following optimizations:
>
> 1. For example, for the parse and print of
> Month/DayOfMonth/Hour/Minute/Second with a fixed length of 2, do targeted
> parse and print optimization.
>
> 2. Parse uses LocalDate/LocalTime/LocalDateTime/OffsetDateTime for
> TemporalQuery to avoid the overhead of constructing DateTimeParseContext.
>
> These optimizations can significantly improve performance, with more than
> 100% performance improvement in many scenarios.
My comments refer to the outer parts of the PR. I haven't reviewed the class
file generation part.
With these changes, please ensure that the same field can be output twice. Not
sure if any tests cover that.
Thanks for the performance boost.
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
2594:
> 2592: }
> 2593:
> 2594: public <T> T parse(CharSequence text, DateTimeFormatter
> formatter, TemporalQuery<T> query) {
Would package or protected scope be sufficient?
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
2635:
> 2633: }
> 2634:
> 2635: static int litteral(CharSequence text, int pos, char literlal) {
`literal`
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
3062:
> 3060:
> 3061: protected final void
> printValueFixWidth3NotNegative(StringBuilder buf, int value) {
> 3062: if (value > 1000) {
Should this be `>=` ?
src/java.base/share/classes/java/time/temporal/TemporalAccessor.java line 327:
> 325: * @return the year, from MIN_YEAR to MAX_YEAR
> 326: */
> 327: default int getYear() {
The whole point of `TemporalAccessor` is that it does not have methods like
these - it does not assume anything about the temporal.
-------------
PR Review: https://git.openjdk.org/jdk/pull/23384#pullrequestreview-2589155307
PR Review Comment: https://git.openjdk.org/jdk/pull/23384#discussion_r1938938121
PR Review Comment: https://git.openjdk.org/jdk/pull/23384#discussion_r1938998806
PR Review Comment: https://git.openjdk.org/jdk/pull/23384#discussion_r1939005206
PR Review Comment: https://git.openjdk.org/jdk/pull/23384#discussion_r1938934674
