Skip to content

Commit

Permalink
Remove extra ast pass for Pattern#names
Browse files Browse the repository at this point in the history
Before this commit #ast and #names were each doing a separate pass over
the journey ast. Since both of these methods are called for every route
definition, we can do that work up front in #initialize, consolidating
into a single pass through the ast.

I think we can also get rid of the `spec` alias, but I am planning to do
that in a separate PR to limit the scope of this one.

This is similar to the work done in rails#38901 and rails#39903, and the benchmark
results are similar:

```
Warming up --------------------------------------
              before     7.791k i/100ms
               after    15.843k i/100ms
Calculating -------------------------------------
              before     78.081k (± 1.4%) i/s -    397.341k in   5.089797s
               after    159.093k (± 1.8%) i/s -    807.993k in   5.080367s

Comparison:
               after:   159093.1 i/s
              before:    78081.2 i/s - 2.04x  (± 0.00) slower

MEMORY
Calculating -------------------------------------
              before   240.000  memsize (    40.000  retained)
                         4.000  objects (     1.000  retained)
                         0.000  strings (     0.000  retained)
               after   120.000  memsize (    40.000  retained)
                         2.000  objects (     1.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
               after:        120 allocated
              before:        240 allocated - 2.00x more
```
  • Loading branch information
composerinteralia committed Jul 24, 2020
1 parent 7556f7c commit ea5c857
Showing 1 changed file with 12 additions and 18 deletions.
30 changes: 12 additions & 18 deletions actionpack/lib/action_dispatch/journey/path/pattern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ module ActionDispatch
module Journey # :nodoc:
module Path # :nodoc:
class Pattern # :nodoc:
attr_reader :spec, :requirements, :anchored
attr_reader :names, :spec, :requirements, :anchored
alias :ast :spec

def self.from_string(string)
build(string, {}, "/.?", true)
Expand All @@ -22,40 +23,33 @@ def initialize(ast, requirements, separators, anchored)
@separators = separators
@anchored = anchored

@names = nil
@optional_names = nil
@required_names = nil
@re = nil
@offsets = nil
end

def build_formatter
Visitors::FormatBuilder.new.accept(spec)
end

def eager_load!
required_names
offsets
to_regexp
nil
end

def ast
@names = []
@spec.each do |node|
if node.symbol?
@names << node.name
re = @requirements[node.to_sym]
node.regexp = re if re
elsif node.star?
node = node.left
node.regexp = @requirements[node.to_sym] || /(.+)/
end
end
end

@spec
def build_formatter
Visitors::FormatBuilder.new.accept(spec)
end

def names
@names ||= spec.find_all(&:symbol?).map(&:name)
def eager_load!
required_names
offsets
to_regexp
nil
end

def required_names
Expand Down

0 comments on commit ea5c857

Please sign in to comment.