On Tue, 27 Jun 2023 11:27:18 GMT, 温绍锦 <d...@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
