On Wed, 5 Feb 2025 00:19:31 GMT, Shaojin Wen <s...@openjdk.org> wrote:
>> This is an optimization for decimal Integer.parseInt and Long.parseLong,
>> which improves performance by about 10%. The optimization includes:
>> 1. Improve performance by parsing 2 numbers at a time, which has performance
>> improvements for numbers with length >= 3.
>> 2. It uses charAt(0) for the first number. Assuming that the optimization
>> can eliminate boundary checks, this will be more friendly to parsing numbers
>> with length 1.
>> 3. It removes the reliance on the Character.digit method and eliminates the
>> reliance on the CharacterDataLatin1#DIGITS cache array, which avoids
>> performance degradation caused by cache misses.
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fix JdbExprTest
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 143:
> 141: * @return If both characters are numbers, return d0 * 10 + d1,
> otherwise return -1
> 142: */
> 143: @ForceInline
Can we leave this out? From the doc: "This annotation must be used sparingly."
I tried without and on M1 performance does not seem to be affected.
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 175:
> 173: */
> 174: int d;
> 175: short x = UNSAFE.getShortUnaligned(str,
> Unsafe.ARRAY_BYTE_BASE_OFFSET + offset, false);
@wenshao I'm a bit worried about the use of Unsafe here.
This method is `public` (although in an internal package), and while it is used
correctly in this PR, there's no warning in the doc that `str` and `offset`
must come from a trusted caller that ensures that they are safe to use with,
well..., Unsafe.
Did you consider safer alternatives, like usage of `VarHandle`, even if that
might mean a slight performance degradation?
test/jdk/com/sun/jdi/JdbExprTest.java line 115:
> 113: .shouldMatch("InvalidTypeException: .* convert
> 2147483648 to int");
> 114: execCommand(JdbCommand.set("JdbExprTestTarg.anInt",
> "0x8000000000000000L"))
> 115: .shouldContain("java.lang.NumberFormatException: Error
> at index 15 in: \"8000000000000000\"");
Copyright year should be adjusted.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22919#discussion_r1942751206
PR Review Comment: https://git.openjdk.org/jdk/pull/22919#discussion_r1942775434
PR Review Comment: https://git.openjdk.org/jdk/pull/22919#discussion_r1942778070
