Merge pull request #15806 from tgxworld/partition_routes_during_setup

Partition routes during setup.

4 files changed, 39 insertions, 8 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 0134bf7cab..8c4ba2a154 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Partitioning of routes is now done when the routes are being drawn. This
+    helps to decrease the time spent filtering the routes during the first request.
+
+    *Guo Xiang Tan*
+
 *   Fix regression in functional tests. Responses should have default headers
     assigned.
 
diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb
index 2b036796ab..cc4bd6105d 100644
--- a/actionpack/lib/action_dispatch/journey/router.rb
+++ b/actionpack/lib/action_dispatch/journey/router.rb
@@ -88,7 +88,7 @@ module ActionDispatch
         end
 
         def custom_routes
-          partitioned_routes.last
+          routes.custom_routes
         end
 
         def filter_routes(path)
diff --git a/actionpack/lib/action_dispatch/journey/routes.rb b/actionpack/lib/action_dispatch/journey/routes.rb
index 80e3818ccd..a6d1980db2 100644
--- a/actionpack/lib/action_dispatch/journey/routes.rb
+++ b/actionpack/lib/action_dispatch/journey/routes.rb
@@ -5,13 +5,14 @@ module ActionDispatch
     class Routes # :nodoc:
       include Enumerable
 
-      attr_reader :routes, :named_routes
+      attr_reader :routes, :named_routes, :custom_routes, :anchored_routes
 
       def initialize
         @routes             = []
         @named_routes       = {}
         @ast                = nil
-        @partitioned_routes = nil
+        @anchored_routes    = []
+        @custom_routes      = []
         @simulator          = nil
       end
 
@@ -30,18 +31,22 @@ module ActionDispatch
 
       def clear
         routes.clear
+        anchored_routes.clear
+        custom_routes.clear
         named_routes.clear
       end
 
-      def partitioned_routes
-        @partitioned_routes ||= routes.partition do |r|
-          r.path.anchored && r.ast.grep(Nodes::Symbol).all?(&:default_regexp?)
+      def partition_route(route)
+        if route.path.anchored && route.ast.grep(Nodes::Symbol).all?(&:default_regexp?)
+          anchored_routes << route
+        else
+          custom_routes << route
         end
       end
 
       def ast
         @ast ||= begin
-          asts = partitioned_routes.first.map(&:ast)
+          asts = anchored_routes.map(&:ast)
           Nodes::Or.new(asts) unless asts.empty?
         end
       end
@@ -60,6 +65,7 @@ module ActionDispatch
         route.precedence = routes.length
         routes << route
         named_routes[name] = route if name && !named_routes[name]
+        partition_route(route)
         clear_cache!
         route
       end
@@ -68,7 +74,6 @@ module ActionDispatch
 
         def clear_cache!
           @ast                = nil
-          @partitioned_routes = nil
           @simulator          = nil
         end
     end
diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb
index 0159f75a18..b54d961f66 100644
--- a/actionpack/test/journey/routes_test.rb
+++ b/actionpack/test/journey/routes_test.rb
@@ -3,6 +3,10 @@ require 'abstract_unit'
 module ActionDispatch
   module Journey
     class TestRoutes < ActiveSupport::TestCase
+      setup do
+        @routes = Routes.new
+      end
+
       def test_clear
         routes = Routes.new
         exp    = Router::Strexp.build '/foo(/:id)', {}, ['/.?']
@@ -36,6 +40,23 @@ module ActionDispatch
         assert_not_equal sim, routes.simulator
       end
 
+      def test_partition_route
+        path   = Path::Pattern.from_string '/hello'
+
+        anchored_route = @routes.add_route nil, path, {}, {}, {}
+        assert_equal [anchored_route], @routes.anchored_routes
+        assert_equal [], @routes.custom_routes
+
+        strexp = Router::Strexp.build(
+          "/hello/:who", { who: /\d/ }, ['/', '.', '?']
+        )
+        path  = Path::Pattern.new strexp
+
+        custom_route = @routes.add_route nil, path, {}, {}, {}
+        assert_equal [custom_route], @routes.custom_routes
+        assert_equal [anchored_route], @routes.anchored_routes
+      end
+
       def test_first_name_wins
         routes = Routes.new