From 4b01bdedd357bc480e4f7cd3d07591c2f820fbeb Mon Sep 17 00:00:00 2001
From: Daniel Colson <danieljamescolson@gmail.com>
Date: Thu, 23 Jul 2020 11:51:08 -0400
Subject: [PATCH] Alter regexp on initialize to avoid extra ast pass
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Along the same lines as #38901, #39903, and #39914, this commit removes
one pass through the journey ast in an attempt to improve application
boot time.

Before this commit, `Mapper#path` would iterate through the ast to alter
the regex for custom routes. With this commit we move the regex
alterations to initialize, where we were already iterating through the
ast.

The Benchmark for this change is similar to what we have been seeing in
the other PRs.

```
Warming up --------------------------------------
               after    13.121k i/100ms
              before     7.416k i/100ms
Calculating -------------------------------------
               after    128.469k (± 3.1%) i/s -    642.929k in 5.009391s
              before     76.561k (± 1.8%) i/s -    385.632k in 5.038677s

Comparison:
               after:   128469.4 i/s
              before:    76560.8 i/s - 1.68x  (± 0.00) slower

Calculating -------------------------------------
               after   160.000  memsize (     0.000  retained)
                         3.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)
              before   360.000  memsize (     0.000  retained)
                         6.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
               after:        160 allocated
              before:        360 allocated - 2.25x more
```
---
 .../lib/action_dispatch/routing/mapper.rb     | 25 ++++++-------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 9a1e327ef4..b4c8b9fcd5 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -137,6 +137,8 @@ module ActionDispatch
               # Add a constraint for wildcard route to make it non-greedy and match the
               # optional format part of the route by default.
               wildcard_options[node.name.to_sym] ||= /.+?/
+            elsif node.cat?
+              alter_regex_for_custom_routes(node)
             end
           end
 
@@ -185,8 +187,10 @@ module ActionDispatch
           app(@blocks)
         end
 
+        JOINED_SEPARATORS = SEPARATORS.join # :nodoc:
+
         def path
-          build_path @ast, requirements, @anchor
+          Journey::Path::Pattern.new(@ast, requirements, JOINED_SEPARATORS, @anchor)
         end
 
         def conditions
@@ -207,16 +211,10 @@ module ActionDispatch
         end
         private :request_method
 
-        JOINED_SEPARATORS = SEPARATORS.join # :nodoc:
-
-        def build_path(ast, requirements, anchor)
-          pattern = Journey::Path::Pattern.new(ast, requirements, JOINED_SEPARATORS, anchor)
-
+        private
           # Find all the symbol nodes that are adjacent to literal nodes and alter
           # the regexp so that Journey will partition them into custom routes.
-          ast.find_all { |node|
-            next unless node.cat?
-
+          def alter_regex_for_custom_routes(node)
             if node.left.literal? && node.right.symbol?
               symbol = node.right
             elsif node.left.literal? && node.right.cat? && node.right.left.symbol?
@@ -225,20 +223,13 @@ module ActionDispatch
               symbol = node.left
             elsif node.left.symbol? && node.right.cat? && node.right.left.literal?
               symbol = node.left
-            else
-              next
             end
 
             if symbol
               symbol.regexp = /(?:#{Regexp.union(symbol.regexp, '-')})+/
             end
-          }
+          end
 
-          pattern
-        end
-        private :build_path
-
-        private
           def intern(object)
             object.is_a?(String) ? -object : object
           end