On Wed, 28 Jun 2023 07:57:25 GMT, Tingjun Yuan <d...@openjdk.org> wrote:
>> This PR changes the implementation of `Long.toUnsignedString(long, int)` for
>> "default" radices, which avoids creating a `BigInteger` and makes use of
>> `Long.divideUnsigned` and `Long.remainderUnsigned`.
>>
>> I've run the test on `test/jdk/java/lang/Long/Unsigned.java` and it works
>> correctly. I believe that there is no need to add more test cases.
>>
>> I've added a benchmark case to
>> `test/micro/org/openjdk/bench/java/lang/Longs.java` for this method. Here is
>> the benchmark result tested on my machine:
>>
>> Before (JDK 20.0.1):
>>
>>
>> Benchmark (size) Mode Cnt Score Error Units
>> Longs.toUnsignedStringNegative 500 avgt 15 6428.711 ± 63.142 us/op
>>
>>
>> After:
>>
>>
>> Benchmark (size) Mode Cnt Score Error Units
>> Longs.toUnsignedStringNegative 500 avgt 15 3823.655 ± 146.171 us/op
>>
>>
>> No CSR needed since the behavior is not changed.
>
> Tingjun Yuan has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update copyright year
src/java.base/share/classes/java/lang/Long.java line 242:
> 240: default -> {
> 241: long leadingDigits = divideUnsigned(i, radix); //
> always positive
> 242: int lastDigit = (int)remainderUnsigned(i, radix);
You can avoid the additional division from `remainderUnsigned()` like so
Suggestion:
int lastDigit = (int)(i - leadingDigits * radix);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14654#discussion_r1434242670
