On Mon, 2 Oct 2023 05:40:03 GMT, 温绍锦 <[email protected]> wrote:
> I submitted PR #15555 before, and there were too many changes. I split it
> into multiple PRs with small changes. This one is one of them.
>
> this PR removed the duplicate code for getChars in
> BigDecimal#StringBuilderHelper, i also make performance faster, Here are the
> numbers run on a MacBook M1 Pro:
>
>
> -Benchmark Mode Cnt Score Error
> Units (baseline)
> -BigDecimals.testHugeToEngineeringString avgt 15 228.102 ? 12.568 ns/op
> -BigDecimals.testLargeToEngineeringString avgt 15 53.814 ? 0.180 ns/op
> -BigDecimals.testSmallToEngineeringString avgt 15 17.521 ? 0.521 ns/op
> -BigDecimals.testToEngineeringString avgt 15 1814.858 ? 64.432 ns/op
>
> +Benchmark Mode Cnt Score Error
> Units ()
> +BigDecimals.testHugeToEngineeringString avgt 15 209.579 ? 5.037
> ns/op (+8.84)
> +BigDecimals.testLargeToEngineeringString avgt 15 29.617 ? 0.557
> ns/op (+81.70)
> +BigDecimals.testSmallToEngineeringString avgt 15 11.230 ? 0.075
> ns/op (+56.02)
> +BigDecimals.testToEngineeringString avgt 15 1732.913 ? 33.629
> ns/op (+4.73)
>
>
> Please review and don't hesitate to critique my approach and patch.
src/java.base/share/classes/java/math/BigDecimal.java line 4216:
> 4214: int lowInt = (int) intCompact - highInt * 100;
> 4215: int highIntSize = JLA.stringSize(highInt);
> 4216: byte[] buf = new byte[highIntSize + 3];
I think the String concat indy call site already generates code that computes
the size of the char array to be allocated (remember the lengthCoder from
String Concat). We can probably just use `highInt + '.' +
StringBuilderHelper.DIGIT_TENS[lowInt] +
StringBuilderHelper.DIGIT_ONES[lowInt]`.
src/java.base/share/classes/java/math/BigDecimal.java line 4253:
> 4251: buf.append('0');
> 4252: buf.append('.');
> 4253: for (; pad>0; pad--) {
Can probably use `buf.repeat('0', pad)` now
src/java.base/share/classes/java/math/BigDecimal.java line 4280:
> 4278: } else {
> 4279: offset = 0;
> 4280: coeff = intVal.abs().toString().toCharArray();
It seems `coeff` is no longer modified after this point. I think you can just
use a `String` for `coeff`, and `String` is already coder-optimized, you don't
need to expose a StringBuilder.append via JLA.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16006#discussion_r1349723167
PR Review Comment: https://git.openjdk.org/jdk/pull/16006#discussion_r1349712188
PR Review Comment: https://git.openjdk.org/jdk/pull/16006#discussion_r1349717218
