JDK 9 RFR of 8026236: Add PrimeTest for BigInteger [TEST-ONLY]
brian.burkhalter at oracle.com
Tue May 6 16:38:10 UTC 2014
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:
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:
>> I believe that it addresses all the points of concern aside from testing Mersenne primes.
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:
> 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) : DEFAULT_UPPER_BOUND;
> 56 int certainty = args.length > 2 ? Integer.valueOf(args) : DEFAULT_CERTAINTY;
> 57 boolean parallel = args.length > 3 ? Boolean.valueOf(args) : 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.
More information about the core-libs-dev