[前][次][番号順一覧][スレッド一覧]

ruby-changes:72241

From: Josh <ko1@a...>
Date: Mon, 20 Jun 2022 02:34:54 +0900 (JST)
Subject: [ruby-changes:72241] aeab405878 (master): [rubygems/rubygems] Improve performance of Bundler::SpecSet#for by using hash lookup of handled deps

https://git.ruby-lang.org/ruby.git/commit/?id=aeab405878

From aeab4058784c86df47a455ffdb08714b337d0209 Mon Sep 17 00:00:00 2001
From: Josh Nichols <josh.nichols@g...>
Date: Fri, 13 May 2022 17:22:54 -0400
Subject: [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
---
 lib/bundler/spec_set.rb | 8 +++++---
 1 file 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 https://github.com/ruby/ruby/blob/trunk/lib/bundler/spec_set.rb#L12
     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?
-- 
cgit v1.2.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]