-
Notifications
You must be signed in to change notification settings - Fork 54
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
sieve with bit-vec #12
Conversation
improve reference to Chia network's `inkfish` implementation of "Close to Uniform Prime Number GenerationWith Fewer Random Bits" by Pierre-Alain Fouque and Mehdi Tibouchi https://eprint.iacr.org/2011/481.pdf Closes poanetwork#11
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.
Looks good to me!
Do you have any numbers on whether it makes a difference? Even without proper benchmarks, just using time
, does it speed things up?
Just fyi my benchmarks look like:
|
Just running |
There was a BitVec in std way back https://doc.rust-lang.org/1.2.0/std/collections/struct.BitVec.html but never got stabilized due to limits on indexing https://github.com/Gankro/rfcs/blob/collections2/text/0000-collections-reform-part-2.md |
Well, my benchmarks don't look like yours. 😄
|
Oddly this may even cause a regression: w3f@961caee |
I see, cargo bench in the vdf directory does not work, but so far the bench.sh script seemingly works. Thanks! |
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.
Looks good!
Cool. I suppose this allocation must be being optimized away, which supports the regression in that other branch w3f@961caee for which I did not send a pull request. Anyways.. |
I believe this improves sieve cache performance with the bit-vec crate, although I've some hiccup with cargo bench for testing this. This also fixes the confusing reference in #11