-
Notifications
You must be signed in to change notification settings - Fork 222
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
Improve performance with GMP library. #44
Merged
Merged
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
6525de2
Use ^ and popcount calculate distance
bebound a0e8101
Update version number
bebound c2d3881
Clean code
bebound af576c6
Revert value calculation to previous method
bebound a1903a4
Rollback test
bebound ff5e043
Fix gmpy2 dependency
bebound d9efcbe
Fix travis test
bebound 7262f67
Update .travis.yml
bebound 96244a8
Update .travis.yml
bebound File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hi, thanks for the pull request.
I'm trying to understand the
popcount
here.Have you done some performance test to show that
popcount
is more efficient?Or maybe you have the source code of
popcount
?I'm trying to find it out, but not found anything about its implementation yet.
:-)
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.
popcount
is used to get the number of1
in binary.gmpy2
is usingGMP
libary'spopcount
function. There is also a cpu instruction namedPOPCNT
, I guessGMP
should be able to call it. I cpu instruction should be much faster than iterate the bit array to find the1
s.I've show the performance test in #43, it's 10x faster.
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.
Thanks!, that helps a lot and it makes sense now. Just curious why the 2 methods produced different results because the old method is to get the number of 1 in binary too. 🙂
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.
By the way, one more thing, in case cpu doesn't have that instruction popcount, can it fall back to less efficient ways to get the same result? 🙂
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.
The
distance
function produce same result19
.But the two version produce different value,
864692470817131398
and7048486158682030128
respectively. Because new method treat the first bit as most significant bit. For example, whenv
is001
, the original method produces4
, but the new method produces1
.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.
Don't worry,
GMP
library should always return the right result regardless of which cpu is using.