On Sun, 28 Jul 2024 15:52:03 GMT, Shaojin Wen <d...@openjdk.org> wrote:

>> By removing the redundant code logic in 
>> DateTimeFormatterBuilder$InstantPrinterParser#formatTo, the codeSize can be 
>> reduced and the performance can be improved.
>
> Shaojin Wen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   1. fix handle fraction == -1
>   2. Split two methods to make codeSize less than 325

I'm still skeptical of some parts of this PR as it makes the code harder to 
folow. The new tests are a good addition and should be merged.

Have you tried the performance of simply breaking out the 
currentEra/beforeCurrentEra methods *without making any other changes*? Since 
the logic to produce the nano fraction is going to stay in this method, I don't 
really see the advantage in trying to get LocalDateTime to do the fraction some 
of the time.

src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 
3818:

> 3816:             if (fractionalDigits == 0) {
> 3817:                 inNano = 0;
> 3818:             }

Suggestion:

            if (fractionalDigits == 0) {
                inNano = 0;
            }
            boolean printNanoInLocalDateTime = fractionalDigits == -2
                    || (inNano == 0 && (fractionalDigits == 0 || 
fractionalDigits == -1));

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

PR Review: https://git.openjdk.org/jdk/pull/20353#pullrequestreview-2231779313
PR Review Comment: https://git.openjdk.org/jdk/pull/20353#discussion_r1712950065

Reply via email to