On Tue, 27 May 2025 21:59:55 GMT, Roger Riggs <[email protected]> wrote:
>> I'd say "create" instead of "retrieve" in the first line comment. (Though
>> that word is used in the other static factories).
>>
>> The "sub-millisecond precision" can't be relied upon. Its the precision that
>> gives the impression that it can be added to the millisecond value and get
>> an exact time. But the nano-second value is read from a different clock and
>> the bits being used from it may wrap-around in the time between the reads of
>> the clocks.
>> The second description ("derived from") is makes fewer promises than the
>> first-line comment.
>> An application should not use them for any purpose other than random. And
>> for that the buffer already contains random bytes.
>>
>> An alternative is to capture the MS time and the nano-time on first use to
>> compute an offset and then use only the nano-time plus/minus the offset to
>> create the version 7 UUIDs.
>
> The `timestamp` method may mislead an app developer thinking its the time
> from the version 7 timestamp.
> The first-line comment might make it more obvious if it says:
> `The timestamp value associated with a version 1 UUID.`
>
> The Unix Epoch time would be easier to use if a method was added to return it.
> `long UnixEpochTimeNanos()` (name subject to bike-shedding).
> And throwing if it was not version 7.
The first 48 bits need to be from the unix epoch time stamp in ms only to
remain complaint. If I understand correctly, to be able to guarantee that the
nsBits are actually time orderable, System.nanoTime() would have to be pinned
to a point where System.currentTimeMillis() increments by 1ms (which doesnt
seem possible). Otherwise any offset could be at any arbitrary point in a given
millisecond and nsBits would wrap around from that.
I think the options are to either use the random only approach with ms
precision or an guaranteed monotonic counter based approach and take the hit on
performance.
I agree with updating to "create" from "retrieve" on the first line comment and
also with the method name change to avoid confusion with UUIDv1, however to
either unixEpochTimeNanos() or unixEpochTimeMillis() or otherwise depending on
which implementation we settle on.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25303#discussion_r2112383140
