This repository has been archived by the owner on Apr 14, 2021. It is now read-only.
[Definition] Avoid re-resolving when a gemspec has dev deps #5354
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
|
@bundlerbot r+ |
|
📌 Commit 38ff427 has been approved by |
bundlerbot
added a commit
that referenced
this pull request
Jan 24, 2017
…rect [Definition] Avoid re-resolving when a gemspec has dev deps Inspired by #5349. Since dev deps are added with `type: :development`, they are `!=` to the deps retrieved from the lockfile, which have no type. This compares the deps ignoring type completely
|
☀️ Test successful - status-travis |
bundlerbot
added a commit
that referenced
this pull request
Mar 30, 2017
Change `definition#converge_dependencies` comparison to an iterator for performance What this addresses --- Follow up for #5539 Previously, a proc was used to extract name and requirements from dependencies. This was done twice, `2*O(n)`, and compared as `Set` objects. This was identified as slow:  ** Again, note that tracepoint slows the results in the chart down. Without `TracePoint`, we run at about 13-15ms. How this fixes it --- Now that we use a hash for `@locked_deps`, we can iterate one of the arrays (`@dependencies`) and compare against entries in `@locked_deps` with `O(1)` access. This results in `O(n)` access. The result is that `converge_dependencies`, which used to take 15ms, now takes about 2-3ms.  ** Again, note that tracepoint slows the results in the chart down. Considerations --- Equality between 2 dependencies, as defined [here for =~](https://docs.ruby-lang.org/en/2.4.0/Gem/Dependency.html), shows us that equality means: > This dependency will match if the name matches the other's name, and other has only an equal version requirement that satisfies this dependency. To make sure we do not regress, we need to check the name and the requirements. Looking at the source code, we can actually just compare the deps as that doesnt take into account `type`. Running the added test from #5354, it passed. So these assumptions seem to hold. cc @segiddins as you wrote #5354 and reviewed #5539
bundlerbot
added a commit
that referenced
this pull request
Mar 31, 2017
Change `definition#converge_dependencies` comparison to an iterator for performance What this addresses --- Follow up for #5539 Previously, a proc was used to extract name and requirements from dependencies. This was done twice, `2*O(n)`, and compared as `Set` objects. This was identified as slow:  ** Again, note that tracepoint slows the results in the chart down. Without `TracePoint`, we run at about 13-15ms. How this fixes it --- Now that we use a hash for `@locked_deps`, we can iterate one of the arrays (`@dependencies`) and compare against entries in `@locked_deps` with `O(1)` access. This results in `O(n)` access. The result is that `converge_dependencies`, which used to take 15ms, now takes about 2-3ms.  ** Again, note that tracepoint slows the results in the chart down. Considerations --- Equality between 2 dependencies, as defined [here for =~](https://docs.ruby-lang.org/en/2.4.0/Gem/Dependency.html), shows us that equality means: > This dependency will match if the name matches the other's name, and other has only an equal version requirement that satisfies this dependency. To make sure we do not regress, we need to check the name and the requirements. Looking at the source code, we can actually just compare the deps as that doesnt take into account `type`. Running the added test from #5354, it passed. So these assumptions seem to hold. cc @segiddins as you wrote #5354 and reviewed #5539
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Inspired by #5349.
Since dev deps are added with
type: :development, they are!=to the deps retrieved from the lockfile, which have no type. This compares the deps ignoring type completely