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