Speed up tags lookup #44
Open
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
the latest tag. The `from_path` method only cares about the most recent tag in the given path, so it's inefficient to check to see if all tags are `contained_in_current_branch?`, when we're only going to use the first one anyway.
mrdziuban
force-pushed
the
speed-up-tags-lookup
branch
from
7513206
to
f693217
Compare
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.
For repositories with a large number of tags, the call to
all_from_pathcan take a significant amount of time, because it gets all tags, then checks to see if all of them arecontained_in_current_branch?.Here's an example of a repository with 234 tags that all match the version format regex. Before these changes, the checks to
contained_in_current_branch?took ~10 seconds:After these changes, the lookup took 0.05 seconds, because it only had to call
contained_in_current_branch?once:I don't have enough knowledge of p4 to implement a similar change in
P4Version, so I just have it callingall_from_path(path).firstfor now.