Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Low hanging performance fixes and some comment cleanup #4

wants to merge 4 commits into from

Conversation

evrhines
Copy link
Contributor

@evrhines evrhines commented Dec 7, 2021

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.

Old performance per 2048 bit prime
real 1.442000

New performance per 2048 bit prime
real 0.814000

Evan Hines added 4 commits December 6, 2021 22:07
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;
Copy link
Contributor Author

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);
Copy link
Contributor Author

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()));
Copy link
Contributor Author

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 {
Copy link
Contributor Author

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
Copy link
Contributor Author

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();
Copy link
Contributor Author

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was unreachable code.

}

}
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant