[9] RFR of 8032027: Add BigInteger square root methods
joe darcy
joe.darcy at oracle.com
Mon Nov 16 18:00:55 UTC 2015
Returning to this review...
On 10/7/2015 7:02 PM, Brian Burkhalter wrote:
> Joe / Andrew / Louis,
>
> Following up on the thread regarding https://bugs.openjdk.java.net/browse/JDK-8032027.
>
> I revised the algorithm similarly to what Louis suggested for the initial value of the iteration. I also tightened up the memory usage. The updated version is here:
>
> http://cr.openjdk.java.net/~bpb/8032027/webrev.01/
>
> I did not at this time change the algorithm to compute the reciprocal of the square root instead of the root directly as Andrew suggested. Apparently this would improve the rate of convergence from quadratic to cubic, but I am inclined to file this and the Karatsuba square root as two separate issues for future enhancement.
>
> Likewise I did not yet include the special case which Joe mentioned for tuning performance for n < 2^1023 wherein the spread of unity bits is sufficiently small. If this does not make it into the current round of revisions a separate enhancement issue may likewise be filed.
>
> Thanks,
>
> Brian
A few comments and suggestions:
2452 public BigInteger[] sqrtAndRemainder() {
2453 BigInteger s = sqrt();
2454 BigInteger r = this.subtract(s.square());
2455 return new BigInteger[] {s, r};
2456 }
It that "remainder >= 0" would be a good assert to add before line 2455.
As a stylistic matter, in MutableBigInteger I would prefer a direct
check of bitLength rather than relying on the exception being thrown
from longValueExact. Something like
if (bitLength < 63)
// use double initial approx
else
// use other technique
In the tests, use of lambda could allow the core testing logic to be shared.
Thanks,
-Joe
More information about the core-libs-dev
mailing list