RFR: 8276220: Reduce excessive allocations in DateTimeFormatter [v3]
Claes Redestad
redestad at openjdk.java.net
Wed Nov 3 12:25:15 UTC 2021
On Wed, 3 Nov 2021 11:53:52 GMT, Stephen Colebourne <scolebourne at openjdk.org> wrote:
>> Claes Redestad has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Add fallback for values outside the allowable range
>
> 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.
Yes, I'll make sure to read into a local variable.
> 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.
I'll see to it.
> 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.
I think the main engineering challenge is writing tests that ensure that we don't lose precision on an arbitrary fractional field.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6188
More information about the core-libs-dev
mailing list