RFR: 8341402: BigDecimal's square root optimization [v17]

Raffaello Giulietti rgiulietti at openjdk.org
Tue Nov 19 09:23:58 UTC 2024


On Mon, 18 Nov 2024 14:21:29 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:

>> src/java.base/share/classes/java/math/BigDecimal.java line 2248:
>> 
>>> 2246:                             }
>>> 2247:                         }
>>> 2248:                     } else { // mc.roundingMode == RoundingMode.UP || mc.roundingMode == RoundingMode.CEILING
>> 
>> It would be more robust to make all enum constant explicit, and reserve the `else` to throw a `RuntimeException` of some sort, e.g., `MatchException`. Should an additional enum constant be added to `RoundingMode` in the future, this code will then throw instead of silently producing a wrong result.
>> 
>> If possible at all, it would be preferable to use a `switch` rather than `if`s.
>
> Also, please merge master into this.

While you are at it, it would be useful to add one test for each of the paths in the new algorithm (including the exception cases) in the existing [SquareRootTests](https://github.com/openjdk/jdk/blob/master/test/jdk/java/math/BigDecimal/SquareRootTests.java) This helps to identify future regressions when the code is later modified for any reason.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21301#discussion_r1847964055


More information about the core-libs-dev mailing list