RFR: 8077587: BigInteger Roots [v50]

[fabioromano1]

On Sun, 27 Jul 2025 08:26:59 GMT, Thomas Zimmermann <duke at openjdk.org> wrote:

>> fabioromano1 has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   A zero root's shift can't be excluded
>
> src/java.base/share/classes/java/math/BigInteger.java line 2773:
> 
>> 2771:      */
>> 2772:     public BigInteger nthRoot(int n) {
>> 2773:         return n == 1 ? this : (n == 2 ? sqrt() : nthRootAndRemainder(n, false)[0]);
> 
> I'm not a reviewer, but I noticed that inlining the `needRemainder`-flag into the callers arguably yields clearer flow, obviates the "Assume n != 1 && n != 2" javadoc and removes the array allocation for `nthRoot`:
> 
> 
>     public BigInteger nthRoot(int n) {
>         if (n == 1)
>             return this;
> 
>         if (n == 2)
>             return sqrt();
> 
>         checkNthRoot(n);
>         MutableBigInteger[] rootRem = new MutableBigInteger(this.mag).nthRootRem(n);
>         return rootRem[0].toBigInteger(signum);
>     }
> 
>     public BigInteger[] nthRootAndRemainder(int n) {
>         if (n == 1)
>             return new BigInteger[] { this, ZERO };
> 
>         if (n == 2)
>             return sqrtAndRemainder();
> 
>         checkNthRoot(n);
>         MutableBigInteger[] rootRem = new MutableBigInteger(this.mag).nthRootRem(n);
>         return new BigInteger[] {
>                 rootRem[0].toBigInteger(signum),
>                 rootRem[1].toBigInteger(signum)
>         };
>     }
> 
>     private static void checkNthRoot(int n) {
>         if (n <= 0)
>             throw new ArithmeticException("Non-positive root degree");
> 
>         if ((n & 1) == 0 && this.signum < 0)
>             throw new ArithmeticException("Negative radicand with even root degree");
>     }
> 
> 
> What do you think?

> @zimmi @fabioromano1 I'm fine with this change. However, I wonder if it might be beneficial to avoid computing the remainder downstream when invoking `nthRoot(int n)`, returning `null` in the array.

Why do useless work to cast the remainder to `BigInteger` when it is not needed, considering that it could cost an allocation of a new `int[]` array?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24898#discussion_r2236387303