RFR: 8276220: Reduce excessive allocations in DateTimeFormatter [v2]

[Claes Redestad]

On Mon, 1 Nov 2021 22:27:08 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 3269:
>> 
>>> 3267:                 return false;
>>> 3268:             }
>>> 3269:             int val = value.intValue(); // NANO_OF_SECOND must fit in an int and can't be negative
>> 
>> Unfortunately, this is not a valid assumption, and it affects the logic of the optimization more generally (methods where non-negative is assumed).
>> 
>> Basically, NANO_OF_SECOND (and all other fields in the formatter) can have any `long` value. Despite immediate appearances, the value is not limited to 0 to 999,999,999. This is because you are allowed to create an implementation of `Temporal` that returns values outside that range. No such class exists in the JDK, but such a class would be perfectly legal. As a related example, it would be perfectly value to write a time class that ran from 03:00 to 26:59 each day, thus HOUROF_DAY cannot be assumed by the formatter to be between 0 and 23.
>
> The commentary on this line could probably be improved, but this is in a private printer-parser that will only be used for NANO_OF_SECOND and not any arbitrary `TemporalField` (see line 704), thus I fail to see how this assumption can fail (since NANO_OF_SECOND specifies a value range from 0 to 999,999,999).
> 
> I considered writing a more generic integral-fraction printer parser that would optimize for any value-range that fits in an int, but seeing how NANO_OF_SECOND is likely the only one used in practice and with a high demand for better efficiency I opted to specialize for it more directly.

I see what you're saying that an arbitrary `Temporal` could define its own fields with its own ranges, but I would consider it a design bug if such an implementation at a whim redefines the value ranges of well-defined constants such as `ChronoField.NANO_OF_SECOND` or `HOUR_OF_DAY`. I'd expect such a `Temporal` would have to define its own enumeration of allowed `TemporalField`s.

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

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