On Tue, 31 Jan 2023 13:39:46 GMT, Raffaello Giulietti <rgiulie...@openjdk.org> wrote:
>> A reimplementation of `BigDecimal.[double|float]Value()` to enhance >> performance, avoiding an intermediate string and its subsequent parsing on >> the slow path. > > This PR is waiting for a review @rgiulietti Thanks for keeping making JDK faster! I have a couple of review comments: #### 1) For the line https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3746 It seems that removing this check for 22-bit number introduces an extra rounding error. Please consider just increasing the constant up to `1L << 32` and using `DOUBLE_10_POW` instead of `FLOAT_10_POW`. Bellow are REPL expressions that illustrate my thoughts (sorry for using Scala): $ scala Welcome to Scala 3.2.0 (17.0.5, Java OpenJDK 64-Bit Server VM). Type in expressions for evaluation. Or try :help. scala> new java.math.BigDecimal("167772170E-1").floatValue val res0: Float = 1.6777216E7 scala> 167772170E-1f val res1: Float = 1.6777216E7 scala> 167772170 / 10.0f val res2: Float = 1.6777218E7 scala> (167772170 / 10.0).toFloat val res3: Float = 1.6777216E7 #### 2) For the line: https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3791 It seems that removing this check for 52-bit number introduces an extra rounding error: $ scala Welcome to Scala 3.2.0 (17.0.6, Java OpenJDK 64-Bit Server VM). Type in expressions for evaluation. Or try :help. scala> new java.math.BigDecimal("90071992547409930E-1").doubleValue val res0: Double = 9.007199254740992E15 scala> 90071992547409930E-1 val res1: Double = 9.007199254740992E15 scala> 90071992547409930L / 10.0 val res2: Double = 9.007199254740994E15 #### 3) For the line: https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3800 Here before falling back to the costly fallback with big number operations we can [get a number of digits in intCompact](https://github.com/plokhotnyuk/jsoniter-scala/blob/6f702ce5cae05df91b5aa6e4bd61acdf43bf18f6/jsoniter-scala-core/jvm/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonWriter.scala#L1930) and use [a trick with two multiplications](https://github.com/plokhotnyuk/jsoniter-scala/blob/6f702ce5cae05df91b5aa6e4bd61acdf43bf18f6/jsoniter-scala-core/jvm/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonReader.scala#L1459) ------------- PR: https://git.openjdk.org/jdk/pull/9410
