On Wed, 24 May 2023 19:18:33 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
> UUID is very important class that is used to track identities of objects in > large scale systems. Yet, the coverage in JDK test is disappointing: it tests > only 100 of UUID instances per test, which is way too small to detect > collisions due to the bad randomness for example. > > I have some pending work to improve UUID performance, and tests should be > improved. > > The improved test still runs in less than 5 seconds on my laptop. Overall, this test-only change look OK to me. It updates the test to use a `Set` instead of `List` for "contains" checks, which detect random generated UUID collisions. The other part of this change is increasing the UUID generation count/repeatition and a new test method which generates the UUIDs in parallel and tries to detect any unexpected collisions. I just have a couple of minor suggestions, which I have noted inline. test/jdk/java/util/UUID/UUIDTest.java line 40: > 38: private static final int COUNT = 1_000_000; > 39: > 40: static final Random generator = new Random(); Hello Aleksey, I realize this is unrelated to the changes in this PR, but since we are updating this test, would using `jdk.test.lib.RandomFactory.getRandom()` from test library be a better idea here? That would then print the seed in the logs and could help debug any collisions more easily? test/jdk/java/util/UUID/UUIDTest.java line 316: > 314: UUID u2 = UUID.fromString(u1.toString()); > 315: if (u1.hashCode() != u2.hashCode()) { > 316: throw new Exception("Equal UUIDs with different hash > codes: " + u1 + " and " + u2); Perhaps we should even print the hash codes that weren't matching to provide assistance when debugging? ------------- PR Comment: https://git.openjdk.org/jdk/pull/14134#issuecomment-1566645445 PR Review Comment: https://git.openjdk.org/jdk/pull/14134#discussion_r1208977029 PR Review Comment: https://git.openjdk.org/jdk/pull/14134#discussion_r1208978832