On Wed, 13 Sep 2023 18:52:21 GMT, ExE Boss <d...@openjdk.org> wrote:

>> In many scenarios, DateTimeFormatter::format is a slower operation.  
>> 
>> For example, the following business scenarios
>> 1. The json library 
>> gson/jackson/[fastjson2](https://github.com/alibaba/fastjson2) formats 
>> Instant/LocalDate/LocalTime/LocalDateTime/ZonedDateTim into strings.
>> 2. In data integration scenarios, for projects like  
>> [datax](https://github.com/alibaba/datax)/[canal](https://github.com/alibaba/canal),
>>  if the input type is Date/Instant and the output type is String, formatting 
>> is required.
>> 
>> This PR provides format performance optimization for commonly used date 
>> patterns.
>> 
>> ISO_INSTANT
>> ISO_LOCAL_TIME
>> ISO_LOCAL_DATE
>> ISO_LOCAL_DATETIME
>> HH:mm:ss
>> HH:mm:ss.SSS
>> yyyy-MM-dd
>> yyyy-MM-dd HH:mm:ss
>> yyyy-MM-dd'T'HH:mm:ss
>> yyyy-MM-dd HH:mm:ss.SSS
>> yyyy-MM-dd'T'HH:mm:ss.SSS
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 2661:
> 
>> 2659:             } else {
>> 2660:                 return super.format(context, buf);
>> 2661:             }
> 
> This can use `instanceof <pattern>`:
> Suggestion:
> 
>             if (temporal instanceof LocalTime lt) {
>                 time = lt;
>             } else if (temporal instanceof LocalDateTime ldt) {
>                 time = ldt.toLocalTime();
>             } else if (temporal instanceof ZonedDateTime zdt) {
>                 time = zdt.toLocalTime();
>             } else if (temporal instanceof OffsetDateTime odt) {
>                 time = odt.toLocalTime();
>             } else if (temporal instanceof OffsetTime ot) {
>                 time = ot.toLocalTime();
>             } else {
>                 return super.format(context, buf);
>             }
> 
> 
> Or even a pattern switch:
> Suggestion:
> 
>             switch (temporal) {
>                 case LocalTime lt -> time = lt;
>                 case LocalDateTime ldt -> time = ldt.toLocalTime();
>                 case ZonedDateTime zdt -> time = zdt.toLocalTime();
>                 case OffsetDateTime odt -> time = odt.toLocalTime();
>                 case OffsetTime ot -> time = ot.toLocalTime();
>                 default -> {
>                     return super.format(context, buf);
>                 }
>             }

It's a good idea to use the new switch syntax

> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 2683:
> 
>> 2681:      * Composite printer and parser.
>> 2682:      */
>> 2683:     static class CompositePrinterParser implements 
>> DateTimePrinterParser {
> 
> This class can be `sealed`:
> Suggestion:
> 
>     static sealed class CompositePrinterParser implements 
> DateTimePrinterParser {

Changes have been made based on your suggestions

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15722#discussion_r1325113374
PR Review Comment: https://git.openjdk.org/jdk/pull/15722#discussion_r1325112780

Reply via email to