RFR: 8327791: Optimization for new BigDecimal(String) [v4]

[Joe Darcy]

On Mon, 11 Mar 2024 20:41:25 GMT, Shaojin Wen <duke at openjdk.org> wrote:

>> The current BigDecimal(String) constructor calls String#toCharArray, which has a memory allocation.
>> 
>> 
>> public BigDecimal(String val) {
>>     this(val.toCharArray(), 0, val.length()); // allocate char[]
>> }
>> 
>> 
>> When the length is greater than 18, create a char[]
>> 
>> 
>> boolean isCompact = (len <= MAX_COMPACT_DIGITS); // 18
>> if (!isCompact) {
>>     // ...
>> } else {
>>     char[] coeff = new char[len]; // allocate char[]
>>     // ...
>> }
>> 
>> 
>> This PR eliminates the two memory allocations mentioned above, resulting in an approximate 60% increase in performance..
>
> Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   1. bug fix for CharBuffer catch IndexOutOfBoundsException
>   2. reorder if statement
>   3. Improve the performance of !isCompact branch

A general comment here: good performance on micro (or even nano) benchmark provides information to help evaluate a change. However, an improved benchmark results it is neither strictly necessary and certainly not sufficient for the change to go back. In other words, it is acceptable and expected that some changesets that improve performance on a chosen small bechmarks will not no back into mainline.

One reason is simple: even if a change is statistically significant on a benchmark, it may not be significant in practice on more application level workloads and may not provide enough benefit to overcome expected increased complexities in maintenance, etc.

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

PR Comment: https://git.openjdk.org/jdk/pull/18177#issuecomment-1989897315