RFR: 8315968: Consolidate java.util.Digits and StringLatin1::PACKED_DIGITS [v7]

[Chen Liang]

On Mon, 11 Sep 2023 12:23:36 GMT, 温绍锦 <duke at openjdk.org> wrote:

>> Some codes in core libs are duplicated, including:
>> java.util.DecimalDigits::DIGITS -> java.lang.StringLatin1.PACKED_DIGITS
>> java.util.DecimalDigits::size -> java.lang.Long.stringSize
>> 
>> We can reduce duplication through JavaLangAccess, which is also needed in other places, such as:
>> https://github.com/openjdk/jdk/pull/15555
>
> 温绍锦 has updated the pull request incrementally with one additional commit since the last revision:
> 
>   move java.lang.StringLatin1::getChars to jdk.internal.util.DecimalDigits::getCharsLatin1

In short: I suggest splitting the change of moving `getChars` `stringSize` into another patch; they are sufficiently distinct and can shrink this PR's size by a half at least.

src/java.base/share/classes/java/lang/StringLatin1.java line 35:

> 33: import java.util.stream.Stream;
> 34: import java.util.stream.StreamSupport;
> 35: import jdk.internal.util.ArraysSupport;

Suggestion:

import jdk.internal.util.ArraysSupport;
import jdk.internal.util.DecimalDigits;

Assuming you will restore the `getChars` here.

src/java.base/share/classes/java/lang/StringLatin1.java line 42:

> 40: import static java.lang.String.checkIndex;
> 41: import static java.lang.String.checkOffset;
> 42: import static jdk.internal.util.DecimalDigits.digitPair;

Suggestion:


Redundant.

src/java.base/share/classes/java/lang/StringUTF16.java line 45:

> 43: 
> 44: import static jdk.internal.util.DecimalDigits.digitPair;
> 45: 

Suggestion:


Per previous comments.

Remember to add another line of 

import jdk.internal.util.DecimalDigits;

 above!

src/java.base/share/classes/java/lang/StringUTF16.java line 1549:

> 1547:             i = q;
> 1548: 
> 1549:             int packed = (int) digitPair(r);

Suggestion:

            int packed = (int) DecimalDigits.digitPair(r);

src/java.base/share/classes/java/lang/StringUTF16.java line 1558:

> 1556:         // We know there are at most two digits left at this point.
> 1557:         if (i < -9) {
> 1558:             int packed = (int) digitPair(-i);

Suggestion:

            int packed = (int) DecimalDigits.digitPair(-i);

src/java.base/share/classes/java/lang/StringUTF16.java line 1596:

> 1594:             q = i / 100;
> 1595: 
> 1596:             int packed = (int) digitPair((int)((q * 100) - i));

Suggestion:

            int packed = (int) DecimalDigits.digitPair((int)((q * 100) - i));

src/java.base/share/classes/java/lang/StringUTF16.java line 1610:

> 1608:             q2 = i2 / 100;
> 1609: 
> 1610:             int packed = (int) digitPair((q2 * 100) - i2);

Suggestion:

            int packed = (int) DecimalDigits.digitPair((q2 * 100) - i2);

src/java.base/share/classes/java/lang/StringUTF16.java line 1622:

> 1620:             charPos -= 2;
> 1621: 
> 1622:             int packed = (int) digitPair(-i2);

Suggestion:

            int packed = (int) DecimalDigits.digitPair(-i2);

src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 43:

> 41:     }
> 42: 
> 43:     public int digits(long value, byte[] buffer, int index, MethodHandle putCharMH) throws Throwable {

`@Override`

src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 143:

> 141:      * code after loop unrolling.
> 142:      */
> 143:     public static int stringSize(int x) {

I suggest splitting the moves of `stringSize` `getChars` into a new PR dependent on this one; your future date and time optimizations can depend on that one, which exposes stringSize.

Having the DecimalDigits package move and `stringSize` `getChars` moves together complicates the file changes, and it's hard to detect if there's any accidental typo/malicious code in the new additions.

src/java.base/share/classes/jdk/internal/util/OctalDigits.java line 68:

> 66: 
> 67:     @Override
> 68:     public int digits(long value, byte[] buffer, int index, MethodHandle putCharMH) throws Throwable {

Can you revert these IDE reformatting changes? So that we don't pollute the changeset and the Git blame log, making reviews and reverting patches easier.

For example, only 3 lines of changes are truly necessary in this class:
Changing the package, `public ` before `final class OctalDigits` and `static final Digits INSTANCE`.

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

PR Review: https://git.openjdk.org/jdk/pull/15651#pullrequestreview-1619894567
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321648971
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321650127
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321646310
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321647751
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321647883
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321648031
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321648149
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321648317
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321487786
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321644436
PR Review Comment: https://git.openjdk.org/jdk/pull/15651#discussion_r1321640635