summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThe Bundler Bot <bot@bundler.io>2017-03-31 00:37:26 +0000
committerThe Bundler Bot <bot@bundler.io>2017-03-31 00:37:26 +0000
commitfa61ff501492b74cb11cff1fef82aa9038784511 (patch)
tree34a32e5d01f316f762bdf4a4e37e460fe2a54b0f
parentca5710e41dacefce8951f4992378d95b470882dc (diff)
parent6e7ed38d503de61a9b3721e06ff7d17bd6e07c11 (diff)
downloadbundler-fa61ff501492b74cb11cff1fef82aa9038784511.tar.gz
Auto merge of #5546 - jules2689:master, r=segiddins
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: ![image](https://cloud.githubusercontent.com/assets/3074765/24470432/e87f5324-148c-11e7-81c6-3bb4216c4e83.png) ** 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. ![image](https://cloud.githubusercontent.com/assets/3074765/24470454/f7db5638-148c-11e7-8005-1388c4bacea2.png) ** 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
-rw-r--r--lib/bundler/definition.rb13
1 files changed, 11 insertions, 2 deletions
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 5bff85332f..a4aebe95ed 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -695,8 +695,17 @@ module Bundler
dep.platforms.concat(@platforms.map {|p| Dependency::REVERSE_PLATFORM_MAP[p] }.flatten(1)).uniq!
end
end
- dependency_without_type = proc {|d| Gem::Dependency.new(d.name, *d.requirement.as_list) }
- Set.new(@dependencies.map(&dependency_without_type)) != Set.new(@locked_deps.values.map(&dependency_without_type))
+
+ # We want to know if all match, but don't want to check all entries
+ # This means we need to return false if any dependency doesn't match
+ # the lock or doesn't exist in the lock.
+ @dependencies.any? do |dependency|
+ locked_dep = @locked_deps[dependency.name]
+ next true if locked_dep.nil?
+ # We already know the name matches from the hash lookup
+ # so we only need to check the requirement now
+ dependency.requirement != locked_dep.requirement
+ end
end
# Remove elements from the locked specs that are expired. This will most