RFR: 8317515: Unify the code of the parse*() families of methods in j.l.Integer and j.l.Long [v2]

[Raffaello Giulietti]

On Fri, 6 Oct 2023 03:16:50 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Avoid localized integers in radix-out-of-range exception messages.
>
> Could we parse the signed int by parsing the unsigned suffix then prepend the sign? It will unify the code path of `parseInt` and `parseUnsignedInt`. 
> 
> Thanks.

@merykitty For a `String` input that would mean copying the suffix, which could be quite long, or make use of the method that accepts a `CharSequence`, which has different (although more complete) exception messages. Perhaps in a followup PR.

What would be _really_ nice is to be able to write, for example (leaving apart the exception messages issue above)

public static int parseInt(String s, int radix)
                throws NumberFormatException {
    return parseInt(s, 0, s.length(), radix);  // delegate to the CharSequence method
}

and leave it to the runtime compiler to perform something similar to
* make a copy of the `parseInt(CharSequence,int,int,int)` code into `parseInt(String,int)`
* adapt the copy to the `String` case by replacing `invokeinterface` for `charAt()` with `invokevirtual`, which can be further optimized to direct invocation because `String` is final, and eventually inlined perfectly.

Currently, that does not seem to happen, but I might be wrong.
This would spare us 4 code duplications here, and perhaps in many other places where we have almost identical methods for `String` and `CharSequence`.

> src/java.base/share/classes/java/lang/Integer.java line 767:
> 
>> 765:             /* Use MIN_VALUE + x < MIN_VALUE + y as unsigned x < y comparison */
>> 766:             while (i < len && (digit = digit(s.charAt(i++), radix)) >= 0
>> 767:                     && (inRange = MIN_VALUE + result < MIN_VALUE + multmax
> 
> `compareUnsigned(result, multmax) < 0` would be better here.

Oops @merykitty, I see right now that `compareUnsigned` is an intrinsic, so I'll give it a try.

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

PR Comment: https://git.openjdk.org/jdk/pull/16050#issuecomment-1750314751
PR Review Comment: https://git.openjdk.org/jdk/pull/16050#discussion_r1348509327