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

BigInts should use gmp implementation of gcd and lcm #3039

Merged
merged 1 commit into from
Jul 25, 2016

Conversation

endSly
Copy link
Contributor

@endSly endSly commented Jul 24, 2016

Using gmp methods to calculate gcd and lcm of bigints performance is improved by 4x to 16x.

Simple benchmark.

# # Number Theoretic Functions

fun gcd = __gmpz_gcd(rop : MPZ*, op1 : MPZ*, op2 : MPZ*)
fun gcd_ui = __gmpz_gcd_ui(rop : MPZ*, op1 : MPZ*, op2 : MPZ*) : Int
Copy link
Member

Choose a reason for hiding this comment

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

@asterite
Copy link
Member

@endSly Thank you, this is excellent!

I made a few comments, mostly for the overloads that accept an Int.

@endSly endSly force-pushed the master branch 2 times, most recently from 2b0f786 to e465360 Compare July 25, 2016 09:59
@endSly
Copy link
Contributor Author

endSly commented Jul 25, 2016

Fixed, now operations with ints uses *_ui methods. Also added tests for negative numbers.

@asterite asterite added this to the 0.19.0 milestone Jul 25, 2016
@asterite
Copy link
Member

Thank you again! ❤️

@asterite asterite merged commit ca55409 into crystal-lang:master Jul 25, 2016
end

def lcm(other : Int) : BigInt
BigInt.new { |mpz| LibGMP.lcm_ui(mpz, self, other.abs.to_u64) }
Copy link
Contributor

Choose a reason for hiding this comment

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

What about #3039 (comment) ?

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

Successfully merging this pull request may close these issues.

None yet

4 participants