-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Low hanging performance fixes and some comment cleanup #4
base: master
Are you sure you want to change the base?
Conversation
Clean up some old code comments that were distracting Use a new factorization algorithm so that the tests actually finish. This will make working much less tedious.
…dulus the prime and return maybe prime although we can just as easily check if it actually that prime right here. This was causing small primes to go through the more expensive primality tests although we didn't need to. Add a test for factoring two prime numbers
@@ -19,7 +21,7 @@ use log::info; | |||
|
|||
// Settings | |||
// NIST recomends 5 rounds for miller rabin. This implementation does 8. Apple uses 16. Three iterations has a probability of 2^80 of failing | |||
const MILLER_RABIN_ROUNDS: usize = 8usize; | |||
const MILLER_RABIN_ROUNDS: usize = 64usize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I bumped this to 64 to match openssl but we can set it back to 8. Just noticed the comment so I will update it later this evening.
//candidate.set_bit(0, true); | ||
//candidate.set_bit((n-1) as u32, true); | ||
let mut candidate: BigUint = rng.gen_biguint(n); | ||
candidate.set_bit((n-1) as u64, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Composite numbers should be able to be even(?) so I didn't set the 0th bit. Let me know if we only want to generate odd composites.
} | ||
|
||
if &d == &org { | ||
return Some(brute_force_prime_factor(d.try_into().unwrap())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we fail to find a factor, fall back to brute force search up to the sqrt.
for mut i in 3..n_sqrt { | ||
while n.divides(&BigUint::from(i)) { | ||
n = n / BigUint::from(i); | ||
for mut i in 3..n_sqrt + 1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to check the n_sqrt as well in this case since it is possible that n = p^2
enum SmallPrimeResult { | ||
Prime, | ||
NotPrime, | ||
MaybePrime | ||
} | ||
|
||
// if true, then is not prime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove this comments tonight.
|
||
let (d,s) = rewrite(&candidate); | ||
let step = s.sub(&one).to_usize().unwrap(); | ||
let one_usize = one.to_usize().unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just moved these outside of the loop since they are constants.
@@ -450,7 +485,6 @@ fn is_prime(candidate: &BigUint) -> bool { | |||
else { | |||
return true | |||
} | |||
return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was unreachable code.
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a newline tonight :D
Bump dependencies and move allocations outside of the hotpath.
Clean up some old code comments that were distracting
Use a new factorization algorithm so that the tests actually finish. This will make working much less tedious. It may be buggy still but its a very simple algorithm and buggy is better than an infinite run time.
Set bits properly now that set_bit is available. This way the integers will be guaranteed to be large enough to use in cryptography.