diff options
author | Josh Nichols <josh.nichols@gusto.com> | 2022-05-13 17:22:54 -0400 |
---|---|---|
committer | git <svn-admin@ruby-lang.org> | 2022-06-20 02:34:41 +0900 |
commit | aeab4058784c86df47a455ffdb08714b337d0209 (patch) | |
tree | 491beeb9357260424a7cec6e1487d289854dba82 /lib/bundler/spec_set.rb | |
parent | da362fee59b2bff30eb8d63ee15724f5c3aac957 (diff) |
[rubygems/rubygems] Improve performance of Bundler::SpecSet#for by using hash lookup of handled deps
I was looking at (yet another) flamegraph in speedscope, and used the
'left hand heavy' and was shocked to realize that 0.5s of the 1.7s
is spent in DepProxy#name. This method _only_ delegates the name to an
underlying spec, so it's not complex at all.
It seems to be of how often this line ends up calling it:
next if handled.any?{|d| d.name == dep.name && (match_current_platform || d.__platform == dep.__platform) } || dep.name == "bundler"
The `handled` array is built up as dependencies are handled, so this get
slower as more dependencies are installed.
This change changes how `handled` is track. Instead of just an array, I've
tried using a Hash, with the key being a dep's name, and the value being
a list of deps with that name. This means it's constant time to find
the dependencies with the same name.
I saw a drop from 1.7s to 1.0s against master, and from 0.95s to 0.24s
when used with https://github.com/rubygems/rubygems/pull/5533
https://github.com/rubygems/rubygems/commit/844dac30d4
Diffstat (limited to 'lib/bundler/spec_set.rb')
-rw-r--r-- | lib/bundler/spec_set.rb | 8 |
1 files changed, 5 insertions, 3 deletions
diff --git a/lib/bundler/spec_set.rb b/lib/bundler/spec_set.rb index 0dfaed9807..85a9d1537b 100644 --- a/lib/bundler/spec_set.rb +++ b/lib/bundler/spec_set.rb @@ -12,15 +12,17 @@ module Bundler end def for(dependencies, check = false, match_current_platform = false) - handled = [] + # dep.name => [list, of, deps] + handled = Hash.new {|h, k| h[k] = [] } deps = dependencies.dup specs = [] loop do break unless dep = deps.shift - next if handled.any? {|d| d.name == dep.name && (match_current_platform || d.__platform == dep.__platform) } || dep.name == "bundler" + next if handled[dep.name].any? {|d| match_current_platform || d.__platform == dep.__platform } || dep.name == "bundler" - handled << dep + # use a hash here to ensure constant lookup time in the `any?` call above + handled[dep.name] << dep specs_for_dep = spec_for_dependency(dep, match_current_platform) if specs_for_dep.any? |