On Fri, 17 Jan 2025 00:09:11 GMT, Shaojin Wen <s...@openjdk.org> wrote:
>> This PR is a resubmission after PR #21593 was rolled back, and the unsafe
>> offset overflow issue has been fixed.
>>
>> 1) Move getChars methods of StringLatin1 and StringUTF16 to DecimalDigits to
>> reduce duplication.
>>
>> 2) HexDigits and OctalDigits also include getCharsLatin1 and getCharsUTF16
>>
>> 3) Putting these two methods into DecimalDigits can avoid the need to expose
>> them in JavaLangAccess
>> Eliminate duplicate code in BigDecimal
>>
>> 4) This PR will improve the performance of Integer/Long.toString and
>> StringBuilder.append(int/long) scenarios. This is because Unsafe.putByte is
>> used to eliminate array bounds checks, and of course this elimination is
>> safe. In previous versions, in Integer/Long.toString and
>> StringBuilder.append(int/long) scenarios, -COMPACT_STRING performed better
>> than +COMPACT_STRING. This is because StringUTF16.getChars uses
>> StringUTF16.putChar, which is similar to Unsafe.putChar, and there is no
>> bounds check.
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fix comment, from @rgiulietti
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 171:
> 169:
> 170: // We know there are at most two digits left at this point.
> 171: if (i < -9) {
I know this was just copy&paste, but it would be more stylistically more
consistent with the `while` above to have
Suggestion:
if (i <= -10) {
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 213:
> 211:
> 212: // Get 2 digits/iteration using longs until quotient fits into
> an int
> 213: while (i <= Integer.MIN_VALUE) {
I think this can be
Suggestion:
while (i < Integer.MIN_VALUE) {
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 231:
> 229:
> 230: // We know there are at most two digits left at this point.
> 231: if (i2 < -9) {
Same as above
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 267:
> 265: while (i <= -100) {
> 266: q = i / 100;
> 267: r = (q * 100) - i;
Can we eliminate `r` to have the same code shape as the rest?
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 274:
> 272:
> 273: // We know there are at most two digits left at this point.
> 274: if (i < -9) {
Same as above
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 308:
> 306:
> 307: // Get 2 digits/iteration using longs until quotient fits into
> an int
> 308: while (i <= Integer.MIN_VALUE) {
Same as above
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 326:
> 324:
> 325: // We know there are at most two digits left at this point.
> 326: if (i2 < -9) {
Same as above
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 359:
> 357:
> 358: // Get 2 digits/iteration using longs until quotient fits into
> an int
> 359: while (i <= Integer.MIN_VALUE) {
Same as above
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 377:
> 375:
> 376: // We know there are at most two digits left at this point.
> 377: if (i2 < -9) {
Same as above
test/micro/org/openjdk/bench/java/lang/StringBuilders.java line 76:
> 74: int size = 16;
> 75: intsArray = new int[size];
> 76: longArray = new long[size];
Suggestion:
intArray = new int[size];
longArray = new long[size];
or
Suggestion:
intsArray = new int[size];
longsArray = new long[size];
Either both plural, or both singular
test/micro/org/openjdk/bench/java/lang/StringBuilders.java line 240:
> 238: StringBuilder buf = sbLatin1;
> 239: buf.setLength(0);
> 240: for (long l : longArray) {
I believe this should be
Suggestion:
for (int l : intsArray) {
test/micro/org/openjdk/bench/java/lang/StringBuilders.java line 251:
> 249: buf.setLength(0);
> 250: buf.setLength(0);
> 251: for (long l : longArray) {
Same as above
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920365244
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920366336
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920365341
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920368290
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920368614
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920368895
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920369116
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920369796
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920369660
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920377205
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920375564
PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1920375823
