On Tue, 2 Nov 2021 11:03:02 GMT, Claes Redestad <redes...@openjdk.org> wrote:

>> Prompted by a request from Volkan Yazıcı I took a look at why the java.time 
>> formatters are less efficient for some common patterns than custom 
>> formatters in apache-commons and log4j. This patch reduces the gap, without 
>> having looked at the third party implementations. 
>> 
>> When printing times:
>> - Avoid turning integral values into `String`s before appending them to the 
>> buffer 
>> - Specialize `appendFraction` for `NANO_OF_SECOND` to avoid use of 
>> `BigDecimal`
>> 
>> This means a speed-up and reduction in allocations when formatting almost 
>> any date or time pattern, and especially so when including sub-second parts 
>> (`S-SSSSSSSSS`).
>> 
>> Much of the remaining overhead can be traced to the need to create a 
>> `DateTimePrintContext` and adjusting `Instant`s into a `ZonedDateTime` 
>> internally. We could likely also win performance by specializing some common 
>> patterns.
>> 
>> Testing: tier1-3
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add fallback for values outside the allowable range

Changes requested by scolebourne (Author).

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

> 3156: 
> 3157:         // only instantiated and used if there's ever a value outside 
> the allowed range
> 3158:         private FractionPrinterParser fallback;

This class has to be safe in a multi-threaded environment. I'm not convinced it 
is safe right now, as the usage doesn't follow the standard racy single check 
idiom. At a minimum, there needs to be a comment explaining the thread-safety 
issues.

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

> 3264:             if (!field.range().isValidIntValue(value)) {
> 3265:                 if (fallback == null) {
> 3266:                     fallback = new FractionPrinterParser(field, 
> minWidth, maxWidth, decimalPoint, subsequentWidth);

It would be nice to see a test case cover this.

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

> 3288:             range.checkValidValue(value, field);
> 3289:             BigDecimal minBD = BigDecimal.valueOf(range.getMinimum());
> 3290:             BigDecimal rangeBD = 
> BigDecimal.valueOf(range.getMaximum()).subtract(minBD).add(BigDecimal.ONE);

I wouldn't be surprised if there is a way to replace the use of `BigDecimal` 
with calculations using `long`.  Fundamentally, calculations like 15/60 -> 0.25 
are not hard, but it depends on whether the exact results can be matched across 
a wide range of possible inputs.

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

> 3542:             BigDecimal valueBD = 
> BigDecimal.valueOf(value).subtract(minBD);
> 3543:             BigDecimal fraction = valueBD.divide(rangeBD, 9, 
> RoundingMode.FLOOR);
> 3544:             // stripTrailingZeros bug

I believe this bug was fixed a while back, so this code could be simplified.

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

PR: https://git.openjdk.java.net/jdk/pull/6188

Reply via email to