8077587: BigInteger Roots

[fabioromano1]

This PR implements nth root computation for BigIntegers using Newton method.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8077587: BigInteger Roots (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24898/head:pull/24898
$ git checkout pull/24898

Update a local copy of the PR:
$ git checkout pull/24898
$ git pull https://git.openjdk.org/jdk.git pull/24898/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24898

View PR using the GUI difftool:
$ git pr show -t 24898

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24898.diff

Using Webrev

Link to Webrev Comment

[rgiulietti]

@fabioromano1 Is this proof still relevant?

[fabioromano1]

@rgiulietti No, the proof of the recurrence used is the one by Zimmermann linked in the code.

[rgiulietti]

@fabioromano1 I guess this work will be the basis for a future work on BigDecimal n-th root?

Otherwise, there needs to be some evidence for useful use cases of general interest to justify adding n-th root to BigInteger.

[fabioromano1]

I guess this work will be the basis for a future work on BigDecimal n-th root?

@rgiulietti Yes, it is. Some use cases could be primality testing, like AKS, where part of the algorithm detect composite numbers by checking whether an integer is an exact power or not.

[rgiulietti]

I doubt that the AKS primality test is of any practical relevance, although it's a great theoretical achievement.

So if you plan to add a n-th root to BigDecimal in a followup PR, then please keep going on this one.
Otherwise, I can't see a strong need for a BigInteger n-th root in the platform, although I might be wrong.

[fabioromano1]

I doubt that the AKS primality test is of any practical relevance, although it's a great theoretical achievement.

Yes, indeed it was just an example, but the exact power detection can be used also as a test apart from the entire AKS algorithm.

[rgiulietti]

The Rampdown Phase One for JDK 25 is about 3 weeks from now.

I think it's a bit too late for API additions to be approved in due time. And even if we could rush this work for 25, it makes little sense to have this in 25 and the future one in 26. IMO, they should be released together. So this and the followup PR for BigDecimal will probably be integrated only in 26.

In other words, take your time ;-)

[fabioromano1]

@rgiulietti Anyway, I think the code has reached a stable version, so it is ready for review.

[fabioromano1]

@rgiulietti Regarding the tests for n-th root, the tests for sqrt() could be extended in order to test also nthRoot().

[rgiulietti]

@fabioromano1 Now that the jdk25 stabilization branch has been created, work on mainline (branch master) will no longer affect JDK25 (except for backports of bug fixes).
I'll soon start reviewing this PR based on the work of Brent and Zimmermann.

[bridgekeeper]

@fabioromano1 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[fabioromano1]

/touch

[openjdk]

@fabioromano1 The pull request is being re-evaluated and the inactivity timeout has been reset.

[rgiulietti]

@fabioromano1 IIUC, the bulk of the code in the PR is aimed at finding a good starting estimate.
Algorithm 1.14 RootInt in Brent & Zimmermann, p. 27-28, however, works for any initial estimate $u$ meeting $u \ge \lfloor m^{1/k}\rfloor$.

Now, assume $2^{l-1} \le m < 2^l$. We have $m^{1/k} < 2^{l/k} \le 2^{\lceil l/k\rceil}$
Thus, the initial estimate could be simple as $u = 2^{\lceil l/k\rceil}$.

This has the following advantages:

• It's super-easy to determine.
• Computing the first $t = (k - 1) s + \lfloor m/s^{k-1}\rfloor$ (L.4 of RootInt) is quite efficient and can be done separately using shifts and additions.

I don't know if this has some negative impact on performance in practice, but IMO the resulting code is easier to review, understand, and maintain.

Maybe worth giving it a try.
WDYT?

[fabioromano1]

@rgiulietti It should be noted that the convergence of Newton's method is quadratic, meaning the number of correct bits doubles with each iteration. This means that, to keep the number of iterations low, the larger the input, the greater the need for as many initial correct bits as possible.

Moreover, the initial estimate gives at most 53 bits, so delegating its computation to Math.pow() or Math.cbrt() should be faster than computing it with 1 initial iteration of Newton's method in long arithmetic and next 6 iterations done in BigInteger's arithmetic.