RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

Roger Riggs rriggs at openjdk.org
Tue Jun 27 14:22:19 UTC 2023


On Tue, 27 Jun 2023 11:27:18 GMT, 温绍锦 <duke at openjdk.org> wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark                     Mode  Cnt      Score      Error   Units
>> UUIDUtilsBenchmark.new       thrpt    5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt    5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix UUID.java import, rename jdk.util.HexDigits to jdk.util.Hex256 and make private constructor.

in general, do not rush to integrate, given the number and variety of comments and commenters, it is good practice to wait 24 hours after the last comment to see that comments have stabilized. 
Given the global scope of OpenJDK, reviewers are in most/every timezone.
And formally, every PR must have the required number of approvals from official Reviewers.
We're not there yet.

src/java.base/share/classes/jdk/internal/util/Hex256.java line 31:

> 29: 
> 30: /**
> 31:  * Provides a hexadecimal cache array of values from 0 to 255

A bit more information about the contents and use would be useful for future developers and perhaps an example of the use so they don't have to go find where it is used and decipher the code.
Each element contains the ascii encoded hex characters for the index. It may be obvious which character is the low nibble for the low byte but it might be good to say so.

src/java.base/share/classes/jdk/internal/util/Hex256.java line 38:

> 36: 
> 37:     @Stable
> 38:     public static final short[] DIGITS;

If UUID shows up in startup class loading this array would be a candidate for loading by CDS, as mentioned in earlier review comments. Please create a separate issue to evaluate and add the code to load the array using CDS.

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

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609604257
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243790783
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243817685


More information about the core-libs-dev mailing list