JDK 9 RFR of 8026236: Add PrimeTest for BigInteger [TEST-ONLY]

[Brian Burkhalter]

Paul / Florian,

This is a combined response. I am making the corrections pointed out by Florian. Unless I hear otherwise, I am going to assume that Paul’s approval is still valid with these corrections included and will push the path on Wednesday. Please see the updated patch:

http://cr.openjdk.java.net/~bpb/8026236/webrev.04/

Thanks,

Brian

On May 6, 2014, at 1:25 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:

> On May 5, 2014, at 11:17 PM, Brian Burkhalter <brian.burkhalter at oracle.com> 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/
>> 
>> I believe that it addresses all the points of concern aside from testing Mersenne primes.
>> 
> 
> +1
> 
> Paul

On May 6, 2014, at 1:59 AM, Florian Weimer <fweimer at redhat.com> wrote:

> On 05/05/2014 11: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/
> 
> The command line parsing is off by one, I think, it should check for >= instead of >:
> 
>  54         // Prepare arguments
>  55         int upperBound = args.length > 1 ? Integer.valueOf(args[0]) : DEFAULT_UPPER_BOUND;
>  56         int certainty = args.length > 2 ? Integer.valueOf(args[1]) : DEFAULT_CERTAINTY;
>  57         boolean parallel = args.length > 3 ? Boolean.valueOf(args[2]) : true;

I think this happened when I replaced the primes file input with programmatic generation of primes.

> I think this line should go, it doesn't match the shifted indexing:
> 
>  98         //bs.set(0, 2, true);

I forgot to delete it when I changed to shifted indexing.

> There are some very long lines:
> 
> 120         NavigableSet<BigInteger> primes = bs.stream().mapToObj(p -> BigInteger.valueOf(p + 2)).collect(toCollection(TreeSet::new));
> 
> 180         List<BigInteger> bigInts = (new SplittableRandom()).ints(NUM_NON_PRIMES, 2, maxPrime).mapToObj(BigInteger::valueOf).filter(b -> !b.isProbablePrime(certainty)).collect(toList());

Lines split into several at worst not much longer than 80 characters.

> The arithmetic in checkPrime() is unnecessary because isProbablePrime() will never report a prime as a non-prime (the Monte-Carlo vs Las Vegas thing), so the count will always be exact because you feed it only primes.

Good point: check removed.

> It is impossible to check for the expected number of failures (non-primes reported as primes) in checkNonPrimes() because this would result in a test that fails sporadically.

There is no change as a result of this comment.