On Fri, 10 Jul 2026 20:31:07 GMT, Justin Lu <[email protected]> wrote:
>> Sholto has updated the pull request incrementally with one additional commit
>> since the last revision:
>>
>> 8272194: introduce faster code path for LocalDate/LocalDateTime conversion
>
> src/java.sql/share/classes/java/sql/Timestamp.java line 529:
>
>> 527: // Given that we can comfortably state that any dates after the
>> unix epoch
>> 528: // are AD, we can use a much faster local date time derivation
>> for these dates.
>> 529: if (getTime() >= 0) {
>
>> We could potentially expand the number of dates covered by this faster
>> codepath by using a negative unix time representing a much earlier year.
>
> That's a good idea. I agree that representing some negative time with a magic
> number is not as straightforward as `0`, but considering we can retain almost
> the same performance for an extra ~1970 years, I think it is a worthwhile
> tradeoff. Granted we provide a big enough buffer to account for an arbitrary
> time zone offset, and the value is documented properly.
>
> That would pretty much make only the BC years suffer the slowdown, which
> seems extremely justifiable since those are exactly the cases where the
> behavior was incorrect.
I have pushed this date back to `0002-01-01`. While overkill, I think that
doing an entire year offset is cleaner than doing something like `0001-01-03`
which would yield the same result but wouldn't give us as comfortable a
distance from possible timezone problems.
I have also just gone with directly using the millis in a constant rather than
deriving them from a calendar and converting it into millis in a static
initialiser.
However, I am happy to change to that approach if you think it is clearer.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31808#discussion_r3570477448
