On Tue, 27 Jun 2023 11:25:32 GMT, 温绍锦 <d...@openjdk.org> wrote:
>> 温绍锦 has updated the pull request incrementally with one additional commit >> since the last revision: >> >> use ISO_8859_1.INSTANCE directly instead of StandardCharsets.ISO_8859_1 > >> I'm looking through >> [[6b61a71](https://github.com/openjdk/jdk/commit/6b61a715caeeb586d5480a3a70f90bd1811d1335)], >> update 24. >> >> Moving fastUUID out of Long is good. >> >> UUID::toString can use StandardCharsets. ISO_8859_1, no need to use >> sun.nio.cs code here. >> >> The list of imports in java.util.UUID is a bit messy now, can you clean this >> up. >> >> It's confusing to have java.util.HexDigits and jdk.internal.util.HexDigits. >> It also creates an inconsistency with DecimalDigits and OctalDigits as the >> latter encapsulate their arrays. I assume you've done this to allow sharing >> of the 256 element array. I don't object to move the array to a supporting >> class but it needs to be renamed and the class description replaced as it is >> not a Digits implementation. Adding a private constructor would help make it >> clear that it should not be instantiated. > > very good suggestion, i have modified it according to your suggestion. > @wenshao - adding a jdk.internal.digits package for this tiny class is > overkill. For this PR, the simplest thing is to revert the changes to > HexDigit, remove jdk.internal.digits.Hex256 and just put a small holder class > in UUID for use by UUID.toString. That would be my preference to avoid > splitting up HexDigits and creating inconsistencies in code used for string > templates. Wenshao hopes to use this array for other optimizations in `java.lang` in the future. But as for this PR, I suggest wenshao not to do so much. I suggest moving `DIGITS` back to `HexDigits` first to make the modifications in this PR smaller. Reducing modifications helps with faster review. When you need to optimize the classes in `java.lang`, we will continue to discuss how to share these arrays more safely and efficiently. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609857054
