On Tue, 12 Mar 2024 09:41:46 GMT, Claes Redestad <redes...@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
