Optimize dependency matching#586
Merged
Merged
Conversation
Member
|
I like this change overall. Can you use Array#bsearch if it is available (Ruby 2.0.0+)? Can you move your binary search implementation to a utility method? #search_for_dependency is already a very long method and that we're performing a binary search in the middle of it is not obvious. Why freeze the result of latest_specs? |
Member
|
What's with the calls to dup and freeze? Those don't seem related to the search change. |
Contributor
Author
|
Sorry guys, I messed up the commit for this pull request. Please delete this pull request and review the new one: #595 |
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
3 participants
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.
When searching for a specific name, we can just do a binary search to find the gems by their name, instead of iterating thru all the gems (which takes a long time, since there are over 320,000 gem versions).
To do this, we need to sort the tuples by name, which is just a little overhead.
Motivation:
Gem update took 32 seconds on my machine, when there was nothing to update, only to check if there is a newer version.
With the optimized version, it dropped down to 2 seconds.
What do you guys think?