[rubygems/rubygems] Implement pub_grub strategy interface

My application spends more than 30% of time during `bundle update`
comparing versions due to versions being sorted inside
next_package_to_try. This has been addressed in pub_grub by defining a
strategy interface (a `#next_package_and_version` method) which allows
consumers to have finer control over the heuristic to select the next
package to try.

This commit implements the new strategy interface to remove extraneous
version sorting (previously in next_package_to_try) since only the final
count of versions is used.

Combined with a previous change to pub_grub (already applied to
Bundler), this commit results in `bundle update` taking only half the
time it did on 2.6.5.

https://github.com/rubygems/rubygems/commit/62f69e27f0

lib/bundler/resolver.rb

@@ -12,6 +12,7 @@ module Bundler
     require_relative "resolver/candidate"
     require_relative "resolver/incompatibility"
     require_relative "resolver/root"
+    require_relative "resolver/strategy"
 
     include GemHelpers
 
@@ -78,7 +79,7 @@ module Bundler
     end
 
     def solve_versions(root:, logger:)
-      solver = PubGrub::VersionSolver.new(source: self, root: root, logger: logger)
+      solver = PubGrub::VersionSolver.new(source: self, root: root, strategy: Strategy.new(self), logger: logger)
       result = solver.solve
       resolved_specs = result.flat_map {|package, version| version.to_specs(package, @most_specific_locked_platform) }
       SpecSet.new(resolved_specs).specs_with_additional_variants_from(@base.locked_specs)
@@ -167,15 +168,7 @@ module Bundler
     end
 
     def versions_for(package, range=VersionRange.any)
-      versions = select_sorted_versions(package, range)
+      range.select_versions(@sorted_versions[package])
-
-      # Conditional avoids (among other things) calling
-      # sort_versions_by_preferred with the root package
-      if versions.size > 1
-        sort_versions_by_preferred(package, versions)
-      else
-        versions
-      end
     end
 
     def no_versions_incompatibility_for(package, unsatisfied_term)
@@ -355,6 +348,10 @@ module Bundler
       raise GemNotFound, message
     end
 
+    def sort_versions_by_preferred(package, versions)
+      @gem_version_promoter.sort_versions(package, versions)
+    end
+
     private
 
     def filtered_versions_for(package)
@@ -414,10 +411,6 @@ module Bundler
       requirement.satisfied_by?(spec.version) || spec.source.is_a?(Source::Gemspec)
     end
 
-    def sort_versions_by_preferred(package, versions)
-      @gem_version_promoter.sort_versions(package, versions)
-    end
-
     def repository_for(package)
       source_for(package.name)
     end
@@ -433,7 +426,7 @@ module Bundler
         next [dep_package, dep_constraint] if name == "bundler"
 
         dep_range = dep_constraint.range
-        versions = select_sorted_versions(dep_package, dep_range)
+        versions = versions_for(dep_package, dep_range)
         if versions.empty?
           if dep_package.ignores_prereleases? || dep_package.prefer_local?
             @all_versions.delete(dep_package)
@@ -441,7 +434,7 @@ module Bundler
           end
           dep_package.consider_prereleases! if dep_package.ignores_prereleases?
           dep_package.consider_remote_versions! if dep_package.prefer_local?
-          versions = select_sorted_versions(dep_package, dep_range)
+          versions = versions_for(dep_package, dep_range)
         end
 
         if versions.empty? && select_all_versions(dep_package, dep_range).any?
@@ -456,10 +449,6 @@ module Bundler
       end.to_h
     end
 
-    def select_sorted_versions(package, range)
-      range.select_versions(@sorted_versions[package])
-    end
-
     def select_all_versions(package, range)
       range.select_versions(@all_versions[package])
     end

lib/bundler/resolver/strategy.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module Bundler
+  class Resolver
+    class Strategy
+      def initialize(source)
+        @source = source
+      end
+
+      def next_package_and_version(unsatisfied)
+        package, range = next_term_to_try_from(unsatisfied)
+
+        [package, most_preferred_version_of(package, range).first]
+      end
+
+      private
+
+      def next_term_to_try_from(unsatisfied)
+        unsatisfied.min_by do |package, range|
+          matching_versions = @source.versions_for(package, range)
+          higher_versions = @source.versions_for(package, range.upper_invert)
+
+          [matching_versions.count <= 1 ? 0 : 1, higher_versions.count]
+        end
+      end
+
+      def most_preferred_version_of(package, range)
+        versions = @source.versions_for(package, range)
+
+        # Conditional avoids (among other things) calling
+        # sort_versions_by_preferred with the root package
+        if versions.size > 1
+          @source.sort_versions_by_preferred(package, versions)
+        else
+          versions
+        end
+      end
+    end
+  end
+end

lib/bundler/vendor/pub_grub/lib/pub_grub/basic_package_source.rb

@@ -79,29 +79,17 @@ module Bundler::PubGrub
       dependencies_for(@root_package, @root_version)
     end
 
-    # Override me (maybe)
-    #
-    # If not overridden, the order returned by all_versions_for will be used
-    #
-    # Returns: Array of versions in preferred order
-    def sort_versions_by_preferred(package, sorted_versions)
-      indexes = @version_indexes[package]
-      sorted_versions.sort_by { |version| indexes[version] }
-    end
-
     def initialize
       @root_package = Package.root
       @root_version = Package.root_version
 
-      @cached_versions = Hash.new do |h,k|
+      @sorted_versions = Hash.new do |h,k|
         if k == @root_package
           h[k] = [@root_version]
         else
-          h[k] = all_versions_for(k)
+          h[k] = all_versions_for(k).sort
         end
       end
-      @sorted_versions = Hash.new { |h,k| h[k] = @cached_versions[k].sort }
-      @version_indexes = Hash.new { |h,k| h[k] = @cached_versions[k].each.with_index.to_h }
 
       @cached_dependencies = Hash.new do |packages, package|
         if package == @root_package
@@ -117,15 +105,7 @@ module Bundler::PubGrub
     end
 
     def versions_for(package, range=VersionRange.any)
-      versions = range.select_versions(@sorted_versions[package])
+      range.select_versions(@sorted_versions[package])
-
-      # Conditional avoids (among other things) calling
-      # sort_versions_by_preferred with the root package
-      if versions.size > 1
-        sort_versions_by_preferred(package, versions)
-      else
-        versions
-      end
     end
 
     def no_versions_incompatibility_for(_package, unsatisfied_term)

lib/bundler/vendor/pub_grub/lib/pub_grub/strategy.rb

@@ -0,0 +1,42 @@
+module Bundler::PubGrub
+  class Strategy
+    def initialize(source)
+      @source = source
+
+      @root_package = Package.root
+      @root_version = Package.root_version
+
+      @version_indexes = Hash.new do |h,k|
+        if k == @root_package
+          h[k] = { @root_version => 0 }
+        else
+          h[k] = @source.all_versions_for(k).each.with_index.to_h
+        end
+      end
+    end
+
+    def next_package_and_version(unsatisfied)
+      package, range = next_term_to_try_from(unsatisfied)
+
+      [package, most_preferred_version_of(package, range)]
+    end
+
+    private
+
+    def most_preferred_version_of(package, range)
+      versions = @source.versions_for(package, range)
+
+      indexes = @version_indexes[package]
+      versions.min_by { |version| indexes[version] }
+    end
+
+    def next_term_to_try_from(unsatisfied)
+      unsatisfied.min_by do |package, range|
+        matching_versions = @source.versions_for(package, range)
+        higher_versions = @source.versions_for(package, range.upper_invert)
+
+        [matching_versions.count <= 1 ? 0 : 1, higher_versions.count]
+      end
+    end
+  end
+end

lib/bundler/vendor/pub_grub/lib/pub_grub/version_solver.rb

@@ -2,17 +2,20 @@ require_relative 'partial_solution'
 require_relative 'term'
 require_relative 'incompatibility'
 require_relative 'solve_failure'
+require_relative 'strategy'
 
 module Bundler::PubGrub
   class VersionSolver
     attr_reader :logger
     attr_reader :source
     attr_reader :solution
+    attr_reader :strategy
 
-    def initialize(source:, root: Package.root, logger: Bundler::PubGrub.logger)
+    def initialize(source:, root: Package.root, strategy: Strategy.new(source), logger: Bundler::PubGrub.logger)
       @logger = logger
 
       @source = source
+      @strategy = strategy
 
       # { package => [incompatibility, ...]}
       @incompatibilities = Hash.new do |h, k|
@@ -104,25 +107,15 @@ module Bundler::PubGrub
       unsatisfied.package
     end
 
-    def next_term_to_try_from(unsatisfied_terms)
-      unsatisfied_terms.min_by do |term|
-        package = term.package
-        range = term.constraint.range
-        matching_versions = source.versions_for(package, range)
-        higher_versions = source.versions_for(package, range.upper_invert)
-
-        [matching_versions.count <= 1 ? 0 : 1, higher_versions.count]
-      end
-    end
-
     def choose_package_version_from(unsatisfied_terms)
-      unsatisfied_term = next_term_to_try_from(unsatisfied_terms)
+      remaining = unsatisfied_terms.map { |t| [t.package, t.constraint.range] }.to_h
-      package = unsatisfied_term.package
+
+      package, version = strategy.next_package_and_version(remaining)
 
-      version = source.versions_for(package, unsatisfied_term.constraint.range).first
       logger.debug { "attempting #{package} #{version}" }
 
       if version.nil?
+        unsatisfied_term = unsatisfied_terms.find { |t| t.package == package }
         add_incompatibility source.no_versions_incompatibility_for(package, unsatisfied_term)
         return package
       end

tool/bundler/vendor_gems.rb

@@ -8,7 +8,7 @@ gem "net-http", "0.6.0"
 gem "net-http-persistent", "4.0.4"
 gem "net-protocol", "0.2.2"
 gem "optparse", "0.6.0"
-gem "pub_grub", github: "jhawthorn/pub_grub", ref: "57d4f344366c8b86f7fe506e9bfa08f3c731e397"
+gem "pub_grub", github: "jhawthorn/pub_grub", ref: "df6add45d1b4d122daff2f959c9bd1ca93d14261"
 gem "resolv", "0.6.0"
 gem "securerandom", "0.4.1"
 gem "timeout", "0.4.3"

tool/bundler/vendor_gems.rb.lock

@@ -6,8 +6,8 @@ GIT
 
 GIT
   remote: https://github.com/jhawthorn/pub_grub.git
-  revision: 57d4f344366c8b86f7fe506e9bfa08f3c731e397
+  revision: df6add45d1b4d122daff2f959c9bd1ca93d14261
-  ref: 57d4f344366c8b86f7fe506e9bfa08f3c731e397
+  ref: df6add45d1b4d122daff2f959c9bd1ca93d14261
   specs:
     pub_grub (0.5.0)