On Fri, 30 May 2025 10:48:20 GMT, kieran-farrell <d...@openjdk.org> wrote:

>> With the recent approval of UUIDv7 
>> (https://datatracker.ietf.org/doc/rfc9562/), this PR aims to add a new 
>> static method UUID.timestampUUID() which constructs and returns a UUID in 
>> support of the new time generated UUID version. 
>> 
>> The specification requires embedding the current timestamp in milliseconds 
>> into the first bits 0–47. The version number in bits 48–51, bits 52–63 are 
>> available for sub-millisecond precision or for pseudorandom data. The 
>> variant is set in bits 64–65. The remaining bits 66–127 are free to use for 
>> more pseudorandom data or to employ a counter based approach for increased 
>> time percision 
>> (https://www.rfc-editor.org/rfc/rfc9562.html#name-uuid-version-7).
>> 
>> The choice of implementation comes down to balancing the sensitivity level 
>> of being able to distingush UUIDs created below <1ms apart with performance. 
>> A test simulating a high-concurrency environment with 4 threads generating 
>> 10000 UUIDv7 values in parallel to measure the collision rate of each 
>> implementation (the amount of times the time based portion of the UUID was 
>> not unique and entries could not distinguished by time) yeilded the 
>> following results for each implemtation:
>> 
>> 
>> - random-byte-only - 99.8%
>> - higher-precision - 3.5%
>> - counter-based - 0%
>> 
>> 
>> Performance tests show a decrease in performance as expected with the 
>> counter based implementation due to the introduction of synchronization:
>> 
>> - random-byte-only   143.487 ± 10.932  ns/op
>> - higher-precision      149.651 ±  8.438 ns/op
>> - counter-based         245.036 ±  2.943  ns/op
>> 
>> The best balance here might be to employ a higher-precision implementation 
>> as the large increase in time sensitivity comes at a very slight performance 
>> cost.
>
> kieran-farrell has updated the pull request incrementally with four 
> additional commits since the last revision:
> 
>  - update test
>  - update test
>  - updates
>  - update method to lsb and msb with arraybytes

src/java.base/share/classes/java/util/UUID.java line 195:

> 193:      * @return a {@code UUID} generated with the current Unix timestamp
> 194:      *
> 195:      * @spec RFC 9562

The sections of the RFC describing the required monotonicity seem to require 
measures be taken in case of leap seconds and other clock setting cases.  
System.currentTimeMillis() does not ensure a monotonic time.
Something using `nanoTime` could do better since nanoTime is monotonic.

  // Record a fixed point in nano-time corresponding to a fix point in 
System.currentTimeMillis()
  // and use the delta from the current nano-time as the current nano-time.
  private static final long ORIGIN_MS = System.currentTimeMillis();
  private static final long ORIGIN_NS = System.nanoTime();
  
  private static long monotonicMS() {
      return  ORIGIN_MS + (System.nanoTime() - ORIGIN_NS) / 1_000_000;
  }

src/java.base/share/classes/java/util/UUID.java line 226:

> 224:         if (timestamp < 0 || (timestamp >>> 48) != 0) {
> 225:             throw new IllegalArgumentException("Timestamp must be a 
> 48-bit Unix Epoch time in milliseconds.");
> 226:         }

Sadly, the RFC does not make any provision for times before 1/1/1970, so there 
is an incomplete mapping between epochTime in other contexts that allows 
negative values to represent earlier times.
The test for positive 48 bits could be as simple as:
Suggestion:

        if (timestamp >> 48) != 0) {
            throw new IllegalArgumentException("Timestamp must be an unsigned 
48-bit Unix Epoch time in milliseconds.");
        }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25303#discussion_r2116209786
PR Review Comment: https://git.openjdk.org/jdk/pull/25303#discussion_r2116189540

Reply via email to