| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
| |
After @kamipo CR feedback we realized `Route#build` wasn't used. As it
is also private API being able to create Routes both with `#new` and
`#build` was redundant.
|
| |
|
|
|
|
|
|
|
|
| |
This commit changes from constructor's argument list to keyword
arguments in order to remove the dependency of remember parameters'
positions.
The constructor already provided a default value for `internal`, this
commits takes the chance to also add default values for `precedence` and
`scope_options`.
|
| |
|
|
|
|
|
|
|
| |
When a route was defined within an optional scope, if that route didn't
take parameters the scope was lost when using path helpers. This patch
ensures scope is kept both when the route takes parameters or when it
doesn't.
Fixes #33219
|
| | |
|
| |
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
`AD::Journey::GTG::Simulator` is lazily built the first time
`Journey::Router#find_routes` is invoked, which happens when
the first request is served.
On large applications with many routes, building the simulator
can take several hundred milliseconds (~700ms for us).
Triggering this initialization during the boot process reduces
the impact of deploys on the application response time.
|
| |
|
|
|
|
|
|
| |
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.
|
| | |
|
| |
|
|
|
|
| |
There were never public API only there by mistake.
[ci skip]
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is meant to provide a way for Action Cable, Sprockets, and possibly
other Rack applications to mark themselves as internal, and to exclude
themselves from the routing inspector, and thus `rails routes` / `rake
routes`.
I think this is the only way to have mounted Rack apps be marked as
internal, within AD/Journey. Another option would be to create an array
of regexes for internal apps, and then to iterate over that everytime a
request comes through. Also, I only had the first `add_route` method set
`internal`'s default to false, to avoid littering it all over the
codebase.
|
| |
|
|
|
|
|
|
|
| |
By [this commit](https://github.com/rails/rails/commit/0b476de445faf330c58255e2ec3eea0f3a7c1bfc)
`Journey::Route#verb` need not to return verb as regexp.
The returned value is used by inspector, so change it to be a string.
Add inspect_with_multiple_verbs test case to keep the behavior of
inspector correctly.
|
| |
|
|
| |
This way we can make the Route object a read-only data structure.
|
| |
|
|
| |
verb_matcher never returns nil.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rather than building a regexp for every route, lets use the strategy
pattern to select among objects that can match HTTP verbs. This commit
introduces strategy objects for each verb that has a predicate method on
the request object like `get?`, `post?`, etc.
When we build the route object, look up the strategy for the verbs the
user specified. If we can't find it, fall back on string matching.
Using a strategy / null object pattern (the `All` VerbMatcher is our
"null" object in this case) we can:
1) Remove conditionals
2) Drop boot time allocations
2) Drop run time allocations
3) Improve runtime performance
Here is our boot time allocation benchmark:
```ruby
require 'action_pack'
require 'action_dispatch'
route_set = ActionDispatch::Routing::RouteSet.new
routes = ActionDispatch::Routing::Mapper.new route_set
result = ObjectSpace::AllocationTracer.trace do
500.times do
routes.resources :foo
end
end
sorted = ObjectSpace::AllocationTracer.allocated_count_table.sort_by(&:last)
sorted.each do |k,v|
next if v == 0
p k => v
end
__END__
Before:
$ be ruby -rallocation_tracer route_test.rb
{:T_SYMBOL=>11}
{:T_REGEXP=>4017}
{:T_STRUCT=>6500}
{:T_MATCH=>12004}
{:T_DATA=>84092}
{:T_OBJECT=>99009}
{:T_HASH=>122015}
{:T_STRING=>216652}
{:T_IMEMO=>355137}
{:T_ARRAY=>441057}
After:
$ be ruby -rallocation_tracer route_test.rb
{:T_SYMBOL=>11}
{:T_REGEXP=>17}
{:T_STRUCT=>6500}
{:T_MATCH=>12004}
{:T_DATA=>84092}
{:T_OBJECT=>99009}
{:T_HASH=>122015}
{:T_STRING=>172647}
{:T_IMEMO=>355136}
{:T_ARRAY=>433056}
```
This benchmark adds 500 resources. Each resource has 8 routes, so it
adds 4000 routes. You can see from the results that this patch
eliminates 4000 Regexp allocations, ~44000 String allocations, and ~8000
Array allocations. With that, we can figure out that the previous code
would allocate 1 regexp, 11 strings, and 2 arrays per route *more* than
this patch in order to handle verb matching.
Next lets look at runtime allocations:
```ruby
require 'action_pack'
require 'action_dispatch'
require 'benchmark/ips'
route_set = ActionDispatch::Routing::RouteSet.new
routes = ActionDispatch::Routing::Mapper.new route_set
routes.resources :foo
route = route_set.routes.first
request = ActionDispatch::Request.new("REQUEST_METHOD" => "GET")
result = ObjectSpace::AllocationTracer.trace do
500.times do
route.matches? request
end
end
sorted = ObjectSpace::AllocationTracer.allocated_count_table.sort_by(&:last)
sorted.each do |k,v|
next if v == 0
p k => v
end
__END__
Before:
$ be ruby -rallocation_tracer route_test.rb
{:T_MATCH=>500}
{:T_STRING=>501}
{:T_IMEMO=>1501}
After:
$ be ruby -rallocation_tracer route_test.rb
{:T_IMEMO=>1001}
```
This benchmark runs 500 calls against the `matches?` method on the route
object. We check this method in the case that there are two methods
that match the same path, but they are differentiated by the verb (or
other conditionals). For example `POST /users` vs `GET /users`, same
path, different action.
Previously, we were using regexps to match against the verb. You can
see that doing the regexp match would allocate 1 match object and 1
string object each time it was called. This patch eliminates those
allocations.
Next lets look at runtime performance.
```ruby
require 'action_pack'
require 'action_dispatch'
require 'benchmark/ips'
route_set = ActionDispatch::Routing::RouteSet.new
routes = ActionDispatch::Routing::Mapper.new route_set
routes.resources :foo
route = route_set.routes.first
match = ActionDispatch::Request.new("REQUEST_METHOD" => "GET")
no_match = ActionDispatch::Request.new("REQUEST_METHOD" => "POST")
Benchmark.ips do |x|
x.report("match") do
route.matches? match
end
x.report("no match") do
route.matches? no_match
end
end
__END__
Before:
$ be ruby -rallocation_tracer runtime.rb
Calculating -------------------------------------
match 17.145k i/100ms
no match 24.244k i/100ms
-------------------------------------------------
match 259.708k (± 4.3%) i/s - 1.303M
no match 453.376k (± 5.9%) i/s - 2.279M
After:
$ be ruby -rallocation_tracer runtime.rb
Calculating -------------------------------------
match 23.958k i/100ms
no match 29.402k i/100ms
-------------------------------------------------
match 465.063k (± 3.8%) i/s - 2.324M
no match 691.956k (± 4.5%) i/s - 3.469M
```
This tests tries to see how many times it can match a request per
second. Switching to method calls and string comparison makes the
successful match case about 79% faster, and the unsuccessful case about
52% faster.
That was fun!
|
| |
|
|
|
|
| |
verb matching is very common (all routes besides rack app endpoints
require one). We will extract verb matching for now, and use a more
efficient method of matching (then regexp) later
|
| |
|
|
|
| |
I want to change the real constructor to take a particular parameter for
matching the request method
|
| |
|
|
|
| |
Routes are always constructed with a list of required_defaults, so
there's no need to check whether or not it's nil
|
| |
|
|
|
| |
we may want to change the name of the class at some point, so it's
better to use a predicate
|
| | |
|
| |
|
|
|
| |
this way we can remove the strange "respond_to?" conditional in the
`matches?` loop
|
| | |
|
| |
|
|
|
| |
This method was copied from journey at https://github.com/rails/rails/commit/56fee39c392788314c44a575b3fd66e16a50c8b5#diff-2cfaf53c860732fea8689d6f2002594bR78.
`grep -nr 'optional_parts' .`
|
| | |
|
| | |
|
| |
|
|
|
|
|
| |
Unwrap Constraints objects. I don't actually think it's possible
to pass a Constraints object to this constructor, but there were
multiple places that kept testing children of this object. I
*think* they were just being defensive, but I have no idea.
|
| |
|
|
|
| |
The optimized and non-optimized path share more code now without
significant performance degretation
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
1. Escape '%' characters in URLs - only unescaped data
should be passed to URL helpers
2. Add an `escape_segment` helper to `Router::Utils`
that escapes '/' characters
3. Use `escape_segment` rather than `escape_fragment`
in optimized URL generation
4. Use `escape_segment` rather than `escape_path`
in URL generation
For point 4 there are two exceptions. Firstly, when a route uses wildcard
segments (e.g. *foo) then we use `escape_path` as the value may contain '/'
characters. This means that wildcard routes can't be optimized. Secondly,
if a `:controller` segment is used in the path then this uses `escape_path`
as the controller may be namespaced.
Fixes #14629, #14636 and #14070.
|
| |
|
|
|
|
|
|
|
|
| |
When generating an unnamed url (i.e. using `url_for` with an options
hash) we should skip anything other than standard Rails routes otherwise
it will match the first mounted application or redirect and generate a
url with query parameters rather than raising an error if the options
hash doesn't match any defined routes.
Fixes #8018
|
| |
|
|
|
|
| |
leading .)
Adding a boolean route constraint checks for presence/absence of request property
|
| |
|
|
|
|
|
|
| |
Rather than trying to use gsub to remove the optional route segments,
which will fail with nested optional segments, use a custom visitor
class that returns a empty string for group nodes.
Closes #9524
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This commit changes route defaults so that explicit defaults are no
longer required where the key is not part of the path. For example:
resources :posts, bucket_type: 'posts'
will be required whenever constructing the url from a hash such as a
functional test or using url_for directly. However using the explicit
form alters the behavior so it's not required:
resources :projects, defaults: { bucket_type: 'projects' }
This changes existing behavior slightly in that any routes which
only differ in their defaults will match the first route rather
than the closest match.
Closes #8814
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This now allows the use of arrays like this:
get '/foo/:action', to: 'foo', constraints: { subdomain: %w[www admin] }
or constraints where the request method returns an Fixnum like this:
get '/foo', to: 'foo#index', constraints: { port: 8080 }
Note that this only applies to constraints on the request - path
constraints still need to be specified as Regexps as the various
constraints are compiled into a single Regexp.
|
| | |
|
| | |
|
| | |
|
| | |
|
