On Wed, 11 Oct 2023 22:10:45 GMT, Shaojin Wen <d...@openjdk.org> wrote:
>> 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). > > Performance numbers run on MacBook M1 Max: > > -Benchmark Mode Cnt Score Error Units > -BigDecimals.testHugeToEngineeringString avgt 15 217.619 ? 4.617 ns/op > -BigDecimals.testLargeToEngineeringString avgt 15 65.183 ? 4.368 ns/op > -BigDecimals.testSmallToEngineeringString avgt 15 17.084 ? 0.056 ns/op > -BigDecimals.testToEngineeringString avgt 15 1856.849 ? 63.571 ns/op > > +Benchmark Mode Cnt Score Error > Units (eec631c) > +BigDecimals.testHugeToEngineeringString avgt 15 164.910 ? 1.514 > ns/op (+31.97) > +BigDecimals.testLargeToEngineeringString avgt 15 20.826 ? 0.066 > ns/op (+212.99) > +BigDecimals.testSmallToEngineeringString avgt 15 11.987 ? 0.080 > ns/op (+42.53) > +BigDecimals.testToEngineeringString avgt 15 1854.372 ? 63.810 > ns/op (+0.14) > > > The microben covered by fast-path is > BigDecimals.testSmallToEngineeringString, highInt + (lowInt < 10 ? ".0" : > ".") + lowInt is actually implemented based on StringBuilder and will be much > slower than the current baseline. > > Benchmark Mode Cnt Score Error Units > (use StringConcat) > BigDecimals.testSmallToEngineeringString avgt 15 41.876 ? 0.182 ns/op I tested it again and found that the remainder is not slow. This was my wrong impression. I have changed it according to your suggestion. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16006#discussion_r1355834888