Hi Brian,
I've filed a follow-up bug:
JDK-8042478: Include Mersenne primes in BigInteger primality
testing
Cheers,
-Joe
On 5/5/2014 2:17 PM, Brian Burkhalter wrote:
Joe / Paul,
Thanks for the comments. This an aggregate response to your two
messages. An updated version of the patch is here:
http://cr.openjdk.java.net/~bpb/8026236/webrev.03/
<http://cr.openjdk.java.net/%7Ebpb/8026236/webrev.03/>
I believe that it addresses all the points of concern aside from
testing Mersenne primes.
Please see more comments in line.
Thanks,
Brian
On May 3, 2014, at 9:55 AM, Joe Darcy <joe.da...@oracle.com
<mailto:joe.da...@oracle.com>> wrote:
Hi Brian,
I think the parsePrimes method would be better with a different name
since no parsing is occurring anymore.
Renamed methods to createPrimes() and getPrimes().
I think someone in this test the fact that Integer.MAX_VALUE is a
prime should be mentioned in taken advantage of :-)
Done. Simply appended this one value to the list. ;-)
How about adding another test method which tests some Mersenne primes
for primality? [1]
No done.
I'd prefer to see some comments on the primes method briefly
explaining it methodology.
Done via a link to Wikipedia.
Would the running time be unacceptable (or memory usage too large) if
the limit were set to Integer.MAX_VALUE?
Yes, I tried it. Still running after at least ten minutes when I
killed it.
Thanks,
-Joe
[1]http://en.wikipedia.org/wiki/Mersenne_prime
On May 5, 2014, at 2:11 AM, Paul Sandoz <paul.san...@oracle.com
<mailto:paul.san...@oracle.com>> wrote:
On May 3, 2014, at 6:55 PM, Joe Darcy <joe.da...@oracle.com
<mailto:joe.da...@oracle.com>> wrote:
Hi Brian,
I think the parsePrimes method would be better with a different name
since no parsing is occurring anymore.
Yes, and there is no need for the try block.
Removed.
(The use of BitSet.stream() also reminds me we can implement a
spliterator with size estimates based on the words in use, currently
the stream() is created from an iterator.)
Made no change based on this comment (which I interpreted as an aside).
I think someone in this test the fact that Integer.MAX_VALUE is a
prime should be mentioned in taken advantage of :-) How about
adding another test method which tests some Mersenne primes for
primality? [1]
I'd prefer to see some comments on the primes method briefly
explaining it methodology. Would the running time be unacceptable
(or memory usage too large) if the limit were set to Integer.MAX_VALUE?
+1 to both of those, although perhaps the former could be done as a
second iteration to get this code in?
This is what I did: +1 on the both but not the Mersennes.
Also, probably better to create the "NavigableSet<BigInteger> primes"
just once in "main", rather than creating it twice for both tests.
Done.
Still skeptical about the use of a PRNG in the tests, see my comments
in a previous email:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-April/026579.html
Changed to SplittableRandom but collected the intermediate stream into
a list which is inspected upon failure to find the value which exposed
the misbehavior.
