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