From f1525dac115cea40ec41b4f9e267e011be8da22e Mon Sep 17 00:00:00 2001
From: Ben Hughes <ben@pixelmachine.org>
Date: Wed, 28 Dec 2016 11:35:43 -0800
Subject: Optimize Journey::Route#score

Scoring routes based on constraints repeated many type conversions that
could be performed in the outer loop.  Determinations of score and
fitness also used Array operations that required allocations.  Against
my benchmark with a large routeset, this reduced object allocations by
over 30x and wall time by over 3x.
---
 actionpack/lib/action_dispatch/journey/formatter.rb |  6 +++++-
 actionpack/lib/action_dispatch/journey/route.rb     | 13 +++++++++----
 actionpack/test/journey/route_test.rb               |  2 +-
 3 files changed, 15 insertions(+), 6 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb
index 20ff4441a0..1d239addf8 100644
--- a/actionpack/lib/action_dispatch/journey/formatter.rb
+++ b/actionpack/lib/action_dispatch/journey/formatter.rb
@@ -92,7 +92,11 @@ module ActionDispatch
           else
             routes = non_recursive(cache, options)
 
-            hash = routes.group_by { |_, r| r.score(options) }
+            supplied_keys = options.each_with_object({}) do |(k, v), h|
+              h[k.to_s] = true if v
+            end
+
+            hash = routes.group_by { |_, r| r.score(supplied_keys) }
 
             hash.keys.sort.reverse_each do |score|
               break if score < 0
diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb
index 0cc8d83ac8..f2ac4818d8 100644
--- a/actionpack/lib/action_dispatch/journey/route.rb
+++ b/actionpack/lib/action_dispatch/journey/route.rb
@@ -96,13 +96,18 @@ module ActionDispatch
         required_parts + required_defaults.keys
       end
 
-      def score(constraints)
+      def score(supplied_keys)
         required_keys = path.required_names
-        supplied_keys = constraints.map { |k, v| v && k.to_s }.compact
 
-        return -1 unless (required_keys - supplied_keys).empty?
+        required_keys.each do |k|
+          return -1 unless supplied_keys.include?(k)
+        end
+
+        score = 0
+        path.names.each do |k|
+          score += 1 if supplied_keys.include?(k)
+        end
 
-        score = (supplied_keys & path.names).length
         score + (required_defaults.length * 2)
       end
 
diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb
index d2a8163ffb..8fd73970b8 100644
--- a/actionpack/test/journey/route_test.rb
+++ b/actionpack/test/journey/route_test.rb
@@ -96,7 +96,7 @@ module ActionDispatch
         path = Path::Pattern.from_string "/:controller(/:action(/:id))(.:format)"
         generic = Route.build "name", nil, path, constraints, [], {}
 
-        knowledge = { id: 20, controller: "pages", action: "show" }
+        knowledge = { "id" => true, "controller" => true, "action" => true }
 
         routes = [specific, generic]
 
-- 
cgit v1.2.3