Store list of generators with enabled uniqueness for faster clear #1246
Conversation
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
maschwenk
reviewed
Feb 5, 2019
| @@ -34,7 +41,8 @@ def clear | |||
| end | |||
There was a problem hiding this comment.
Is there a reason we can't just reassign
def clear
@previous_results = Hash.new { |hash, key| hash[key] = Set.new }
endThat passes all specs and seems to be fast. Is the fear that there would be a big pressure on the GC?
There was a problem hiding this comment.
Is this faster than @previous_resuls.clear?
There was a problem hiding this comment.
I'd need to do a proper benchmark, but I noticed this before I started using your new code here which seems plenty fast. Was purely curious what the reasoning was. Thanks so much for this PR btw! Can't wait till it's released because right now i can't use the old implementation
There was a problem hiding this comment.
No problem! I'm actually curious about this as well. If you do the benchmarks, please tell us about it.
davidmorton0
pushed a commit
to davidmorton0/faker
that referenced
this pull request
Jul 12, 2021
…ker-ruby#1246) * Store list of generators with enabled uniqueness for faster clear * Update CHANGELOG.md * Update CHANGELOG.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR addresses issue #1186. By keeping a list of generators which used the
uniquefeature, it is faster to clear all of them withFaker::UniqueGenerator.clear, since the latter only needs to go through the objects in the list.