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

[Shaojin Wen]

On Tue, 12 Mar 2024 09:41:46 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   easier to compare
>
> Sorry, when I got pinged in here the earlier comments didn't render and I missed the conversation.
> 
> So first off I think it's probably acceptable to regress the `char[]`-based constructors and gently push people towards other options. In few applications are `char[]` the source, and in the example mentioned there's a transform from `byte[]` to `char[]` which would probably end up being faster now with `new BigDecimal(new String(bytes, start, end, ISO_8859_1))`. 
> 
> That said perhaps there's room for improvement. Looking at `CharBuffer::charAt` it does surprisingly complicated logic and bounds checks. And the `CharBuffer` wrapper itself is a bit bloated due quite a few state variables (that we don't need for this use case). I did an experiment with a specialized implementation here: https://github.com/wenshao/jdk/pull/7 which yields good results. Not sure this is a great idea but something to consider (note that it's not general purpose since it's exploiting a deliberately missing bounds check in `charAt`).

Thanks to @cl4es for pointing out my problem. I removed the curly braces not for change, and I don’t like this coding style either, but to try to be consistent with the coding style of the context.

I found that the CharArraySequence implementation without offset and length has better performance, so there are two implementations.

In the current version, the testConstructorWithSmallCharArray scene has a performance drop of 5.73%, and the performance of other scenes has improved.


-Benchmark                                      Mode  Cnt    Score   Error  Units (base)
-BigDecimals.testConstructorWithSmallCharArray  avgt   15   16.445 ? 0.050  ns/op
-BigDecimals.testConstructorWithLargeCharArray  avgt   15   90.470 ? 1.250  ns/op
-BigDecimals.testConstructorWithHugeCharArray   avgt   15   90.324 ? 1.398  ns/op
-BigDecimals.testConstructorWithCharArray       avgt   15   48.392 ? 1.284  ns/op
-BigDecimals.testConstructorWithSmallString     avgt   15   19.598 ? 0.084  ns/op
-BigDecimals.testConstructorWithLargeString     avgt   15  124.449 ? 6.672  ns/op
-BigDecimals.testConstructorWithHugeString      avgt   15  130.646 ? 3.427  ns/op
-BigDecimals.testConstructorWithString          avgt   15   68.975 ? 2.721  ns/op

+Benchmark                                      Mode  Cnt    Score   Error  Units (v08 #8b2a762c)
+BigDecimals.testConstructorWithSmallCharArray  avgt   15   17.446 ? 0.054  ns/op -5.73%
+BigDecimals.testConstructorWithLargeCharArray  avgt   15   71.501 ? 0.684  ns/op +26.52%
+BigDecimals.testConstructorWithHugeString      avgt   15   63.814 ? 0.243  ns/op +41.54%
+BigDecimals.testConstructorWithCharArray       avgt   15   41.408 ? 0.830  ns/op +16.86%
+BigDecimals.testConstructorWithSmallString     avgt   15   16.668 ? 0.527  ns/op +17.57%
+BigDecimals.testConstructorWithLargeString     avgt   15   64.055 ? 0.183  ns/op +94.28%
+BigDecimals.testConstructorWithHugeCharArray   avgt   15   70.691 ? 0.682  ns/op +84.81%
+BigDecimals.testConstructorWithString          avgt   15   35.946 ? 0.291  ns/op +91.88%

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

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