RFR: 8308803: Improve java/util/UUID/UUIDTest.java

Jaikiran Pai jpai at openjdk.org
Mon May 29 06:55:56 UTC 2023


On Wed, 24 May 2023 19:18:33 GMT, Aleksey Shipilev <shade at 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


More information about the core-libs-dev mailing list