On Mon, 9 Oct 2023 19:20:29 GMT, Shaojin Wen <d...@openjdk.org> 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.
>> Please review and don't hesitate to critique my approach and patch.
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> refactor based on @liach 's review
I'm not really qualified to review the floating point code. Simplifying away
offset and getting rid of the StringBuilderHelper all seem like good
improvements, though I think it'd be good if we could either avoid or split out
the `JLA` changes.
src/java.base/share/classes/java/lang/System.java line 2548:
> 2546:
> 2547: public int stringSize(int i) {
> 2548: return Long.stringSize(i);
Shouldn't this be `return Integer.stringSize(i);`?
src/java.base/share/classes/java/math/BigDecimal.java line 4168:
> 4166: int lowInt = (int)intCompact % 100;
> 4167: int highInt = (int)intCompact / 100;
> 4168: int highIntSize = JLA.stringSize(highInt);
Which micros cover performance of this branch?
What performance would you get if you simplified this to, say, `return highInt
+ (lowInt < 10 ? ".0" : ".") + lowInt;`?
`lowInt` is currently unused since you use `(int) intCompact - highInt * 100`
instead below. (While a clever optimization in theory, I believe the JIT should
handle a pair of integer modulo and division operations (`a = intValue % 100; b
= intValue / 100`) so that it only has to do one division, so please measure
that whatever you do here has a significant benefit).
-------------
Changes requested by redestad (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/16006#pullrequestreview-1668452019
PR Review Comment: https://git.openjdk.org/jdk/pull/16006#discussion_r1353035743
PR Review Comment: https://git.openjdk.org/jdk/pull/16006#discussion_r1354592018
