Skip to content

Conversation

gifnksm
Copy link
Contributor

@gifnksm gifnksm commented Jan 13, 2013

This commit reduces extra memory allocation in std::bigint.

In my simple benchmark test, time of computing 1000! becomes ~1000x faster.

There still may be some points to be improved in performance. For example, switching the algorithms depending on the length of the array.

@catamorphism
Copy link
Contributor

I would appreciate more comments in the vec_ops code that you're adding. Even though this is internal to the bigint library, somebody will have to maintain it.

Thanks!

@lomereiter
Copy link

Could you add figures for the GMP bindings available via cargo install gmp? Here's fibonacci and factorial using them: https://gist.github.com/4527144

@brson
Copy link
Contributor

brson commented Feb 4, 2013

I agree this is a lot of tricky, uncommented code. Most people (like me) are not intimately familiar with bignum math and will not immediately understand what is going on here. @gifnksm can you document this code a little more? I don't even know what documentation it needs because I don't understand it.

@catamorphism
Copy link
Contributor

I'm going to close this pull request and ask that @gifnksm submit a new one with comments if you're able to get around to it. Performance improvements are great, but they need to be documented.

@pnkfelix
Copy link
Contributor

pnkfelix commented Oct 1, 2013

cc me. (i looked briefly at the bigint code we have now and may want to resurrect this patch, or at least review it before embarking on trying to improve our current bigint code.)

RalfJung added a commit to RalfJung/rust that referenced this pull request Jul 20, 2025
Muscraft pushed a commit to Muscraft/rust that referenced this pull request Jul 21, 2025
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.

5 participants