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

utils: Change gcd function from recursive to interative #138

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

honggyukim
Copy link

It'd be faster to use interative implementation than recursive one.

It'd be faster to use interative implementation than recursive one.
@BenWiederhake
Copy link
Contributor

I agree that the iterative version could be faster, but apparently it's not relevant.

So let's test this. The worst-case clearly is large numbers, and occurs when the ratio between a and b is close to the golden ratio. (Intuition: if the ration is very large, then the first step reduces a and b very quickly because the modulo factor is so large. If the ratio is very small, then the modulo remainder cannot be big, so the first step again reduces a nad b very quickly.)
So let's use the numbers a = 18446744073709551615 = 2^64-1 and b = 11400714819323197569 ~= a/phi.

One can just compile this with the same options as tgl will be compiled, and run some experiments.

graph of microseconds_per_call

As you can see, it doesn't really make a difference, and actually looks like the recursive version is a bit faster, if at all. Also note that the stack will reach at most a depth of 93 = log_phi (2^64-1), so there really isn't much to optimize.

The long wait on startup is more likely due to bn_factorize at the bottom of the same file, which implements the proof-of-work part of the login process. I'd love to see an even better implementation of that.

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.

2 participants