RFR: 8327791: Optimization for new BigDecimal(String) [v12]
Shaojin Wen
duke at openjdk.org
Fri Mar 15 05:41:42 UTC 2024
On Wed, 13 Mar 2024 15:43:25 GMT, Joe Darcy <darcy at openjdk.org> wrote:
>> Relying on the upper bounds check of `charAt` doesn't work well with the `CharArraySequence` whose `charAt` deliberately does not throw an IIOBE if the array is longer than the provided length, ie, it'll look at chars beyond the provided range. The examples I tested still end up as a NFE, but it's clear from the cause that we're running past the length:
>>
>> jshell> new BigDecimal(new char[] { '-', '1', 'e'}, 0, 1);
>> | Exception java.lang.NumberFormatException
>> | at BigDecimal.<init> (BigDecimal.java:754)
>> | at BigDecimal.<init> (BigDecimal.java:543)
>> | at BigDecimal.<init> (BigDecimal.java:518)
>> | at (#4:1)
>> | Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds for length 3
>> | at BigDecimal$CharArraySequence.charAt (BigDecimal.java:559)
>> | at BigDecimal.parseExp (BigDecimal.java:772)
>> | at BigDecimal.<init> (BigDecimal.java:619)
>> | ...
>>
>> Baseline/expected:
>>
>> jshell> new BigDecimal(new char[] { '-', '1', 'e'}, 0, 1);
>> | Exception java.lang.NumberFormatException: No digits found.
>> | at BigDecimal.<init> (BigDecimal.java:635)
>> | at BigDecimal.<init> (BigDecimal.java:518)
>> | at (#1:1)
>>
>> Having a check on `len > 0` is more robust - and I'd be surprised if avoiding a redundant check on the loop entry is affecting performance?
>
>> Relying on the upper bounds check of `charAt` doesn't work well with the `CharArraySequence` whose `charAt` deliberately does not throw an IIOBE if the array is longer than the provided length, ie, it'll look at chars beyond the provided range. The examples I tested still end up as a NFE, but it's clear from the cause that we're running past the length:
>>
>> ```
>> jshell> new BigDecimal(new char[] { '-', '1', 'e'}, 0, 1);
>> | Exception java.lang.NumberFormatException
>> | at BigDecimal.<init> (BigDecimal.java:754)
>> | at BigDecimal.<init> (BigDecimal.java:543)
>> | at BigDecimal.<init> (BigDecimal.java:518)
>> | at (#4:1)
>> | Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds for length 3
>> | at BigDecimal$CharArraySequence.charAt (BigDecimal.java:559)
>> | at BigDecimal.parseExp (BigDecimal.java:772)
>> | at BigDecimal.<init> (BigDecimal.java:619)
>> | ...
>> ```
>>
>> Baseline/expected:
>>
>> ```
>> jshell> new BigDecimal(new char[] { '-', '1', 'e'}, 0, 1);
>> | Exception java.lang.NumberFormatException: No digits found.
>> | at BigDecimal.<init> (BigDecimal.java:635)
>> | at BigDecimal.<init> (BigDecimal.java:518)
>> | at (#1:1)
>> ```
>>
>> Having a check on `len > 0` is more robust - and I'd be surprised if avoiding a redundant check on the loop entry is affecting performance?
>
> If the likely error/boundary conditions change, those changed conditions should be added to the regression tests.
Thanks for pointing out this BUG. I hadn't considered it before. I added a boundary check in the CharArraySequence#charAt method and added regression testing.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18177#discussion_r1525782394
More information about the core-libs-dev
mailing list