Fixes #9: Better wheel implementation provides 4x speedup#11
Open
EMBradley wants to merge 4 commits intowackywendell:masterfrom
Open
Fixes #9: Better wheel implementation provides 4x speedup#11EMBradley wants to merge 4 commits intowackywendell:masterfrom
EMBradley wants to merge 4 commits intowackywendell:masterfrom
Conversation
By allowing the Wheel type to store a factor, we are able to store Wheels in our BinaryHeap to drastically reduce the number of times we have the rearrange the heap to keep it as a valid heap. This results in a 4-5x increase in throughput for the find() method. This commit also replaces Wheel30 with a more general Wheel that is not tied to a specific choice of intiial primes. By keeping everything relating to the choice of initial primes in a few constants near the top of the file, we are able to easily swap out different sized Wheels. E.g., if we wanted to also filter out all multiples of 7, all we would need to do is 1) rename WHEEL_235 to WHEEL_2357, 2) replace WHEEL_2357 with an array consisting of all the numbers less than 2 * 3 * 5 * 7 that are coprime with 2, 3, 5, and 7, and 3) update WHEEL_LENGTH to be the length of the resulting array. At the cost of a small amount of memory, this would result in, theoretically, a 14% speedup, since 1/7 of all composites are multiples of 7.
This commit moves the constants used to define a Wheel into the impl Wheel block itself. This ties these constants more directly to the Wheel type, making it more clear that their purpose is to make the Wheel behave properly, not to be used on their own.
EMBradley
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
[Fixes #9]
This PR includes the changes to the
Wheeltype that I recommended in issue #9.The graphs below show the performance of the
SieveandTrialDivisionprime generators. No changes have been made toTrialDivision, so the green line in all of these graphs is the same.First, here is the performance of the prime generators without the changes I'm recommending. Notice that the line for
Sieveis well above the line forTrialDivision(lower is better). In fact,TrialDivisionis usually twice as fast asSieve.Just by implementing the
peek_mut()optimization I suggested in issue #9, we double the speed ofSieve. It's now slightly faster thanTrialDivisionno matter the input size.Then, using the improved
Wheelimplementation, we get another doubling. TheSieveis now much faster thanTrialDivisionfor all input sizes.