On Tue, 30 May 2023 20:55:39 GMT, Roger Riggs <[email protected]> wrote:
>> Aleksey Shipilev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review comments
>
> test/jdk/java/util/UUID/UUIDTest.java line 79:
>
>> 77: }
>> 78: if (!set.add(u)) {
>> 79: throw new Exception("UUID collision: " + u);
>
> I would be concerned that if this failure was reported, it would be
> intermittent, hard to track down, and not reproducible. Without a hook for
> the generator and the seed, its just going to be noise in the testing.
This is a non-practical concern, IMO. By spec, `UUID.randomUUID` is generated
from the cryptographically secure random, with >120 bits of randomness, so the
collision is extremely unlikely. Collision math involves birthday paradox, but
Wikipedia article on UUIDs fortunately gives us the approximated solutions
already:
https://en.wikipedia.org/wiki/Universally_unique_identifier#Collisions
Quote: "Thus, the probability to find a duplicate within 103 trillion version-4
UUIDs is one in a billion."
In other words, finding a collision in this test with 1M UUIDs points to the
implementation issue, not a test bug, with a very high probability. In yet
another words, if a unit test with 1M UUIDs is able to find a collision, then
this is a strong signal that many production systems that assume extremely low
collision probability are up for subtle misbehavior.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14134#discussion_r1211223450
