Commit Graph

52662 Commits

Author SHA1 Message Date
CoralineAda
80da14b580 Adds a code of conduct
An easy way to begin addressing the problem of inclusivity is to be overt in our
openness, welcoming all people to contribute, and pledging in return to
value them as human beings and to foster an atmosphere of kindness,
cooperation, and understanding.

A code of conduct is  one way to express these values. It lets us pledge
our respect and appreciation for contributors and participants to the
project.
2015-08-18 11:39:17 -05:00
Yves Senn
2a2473fd71 Merge pull request #21283 from ravindrakumawat/add_docs_for_pending_migration
Add Docs for ActiveRecord #check_pending [ci skip]
2015-08-18 08:50:22 +02:00
Yves Senn
57498e6f57 Merge pull request #21284 from prakashlaxkar/argument_error_tests
Correct error message in Standard American english and add a test cas…
2015-08-18 08:48:22 +02:00
prakash
820726704d Correct error message in Standard American english and add a test case for the same. 2015-08-18 11:39:52 +05:30
ravindra kumar kumawat
8bd064ec2b Add Docs for ActiveRecord #check_pending [ci skip] 2015-08-18 11:13:15 +05:30
Rafael Mendonça França
88a1800526 Remove unreached default value
verb_matcher never returns nil.
2015-08-17 23:22:59 -03:00
Rafael Mendonça França
352da06503 Merge pull request #21278 from byroot/try-arity-check
Use == 0 instead of .zero? in #try
2015-08-17 23:02:47 -03:00
Aaron Patterson
0b476de445 use the strategy pattern to match request verbs
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!
2015-08-17 18:17:55 -07:00
Aaron Patterson
c989e2c56f switch Route constructors and pass in the regexp
We don't need to add and delete from the conditions hash anymore, just
pass the regexp directly to the constructor.
2015-08-17 16:43:40 -07:00
Aaron Patterson
bb10030802 split the verb regex from the constraints hash
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
2015-08-17 16:39:26 -07:00
Aaron Patterson
23cfdd4b71 test the verb method on the route, specifically 2015-08-17 16:36:31 -07:00
Aaron Patterson
c42db41f54 routes are always constructed with a hash for the conditions 2015-08-17 16:30:53 -07:00
Aaron Patterson
1ce74b009f introduce an alternate constructor for Route objects
I want to change the real constructor to take a particular parameter for
matching the request method
2015-08-17 16:08:39 -07:00
Aaron Patterson
559e7f9450 drop object allocation during routes setup
This commit introduces a functional Path AST visitor and implements
`each` on the AST in terms of the functional visitor.  The functional
visitor doesn't maintain state, so we only need to allocate one of them.

Given this benchmark route file:

```ruby
require 'action_pack'
require 'action_dispatch'

route_set = ActionDispatch::Routing::RouteSet.new
routes = ActionDispatch::Routing::Mapper.new route_set

ObjectSpace::AllocationTracer.setup(%i{path line type})

result = ObjectSpace::AllocationTracer.trace do
  500.times{|i|
    routes.resource :omglol
  }
end

result.find_all { |k,v| k.first =~ /git\/rails/ }.sort_by { |k,v|
  v.first
}.each { |k,v|
  p k => v
}
```

node.rb line 17 was in our top 3 allocation spot:

```
{["/Users/aaron/git/rails/actionpack/lib/action_dispatch/journey/nodes/node.rb", 17, :T_OBJECT]=>[31526, 0, 28329, 0, 2, 1123160]}
{["/Users/aaron/git/rails/actionpack/lib/action_dispatch/routing/mapper.rb", 2080, :T_IMEMO]=>[34002, 0, 30563, 0, 2, 1211480]}
{["/Users/aaron/git/rails/actionpack/lib/action_dispatch/routing/mapper.rb", 2071, :T_IMEMO]=>[121934, 1, 109608, 0, 7, 4344400]}
```

This commit eliminates allocations at that place.
2015-08-17 15:57:06 -07:00
Jean Boussier
a94c2e1f23 Use == 0 instead of .zero? in #try
The perf gain is relatively minor but consistent:

```
Calculating -------------------------------------
             0.zero?   137.091k i/100ms
             1.zero?   137.350k i/100ms
              0 == 0   142.207k i/100ms
              1 == 0   144.724k i/100ms
-------------------------------------------------
             0.zero?      8.893M (± 6.5%) i/s -     44.280M
             1.zero?      8.751M (± 6.4%) i/s -     43.677M
              0 == 0     10.033M (± 7.0%) i/s -     49.915M
              1 == 0      9.814M (± 8.0%) i/s -     48.772M
```

And try! is quite a big hotspot for us so every little gain is appreciable.
2015-08-17 18:44:51 -04:00
Aaron Patterson
8d7b883f33 avoid is_a? checks
add another predicate method so we can avoid is_a checks
2015-08-17 15:28:23 -07:00
Aaron Patterson
56f734a361 pull RegexpOffsets in to a method
we don't really need this visitor
2015-08-17 15:11:20 -07:00
Aaron Patterson
15bc6b630f required_defaults is always passed in, remove conditional
Routes are always constructed with a list of required_defaults, so
there's no need to check whether or not it's nil
2015-08-17 15:11:20 -07:00
Rafael Mendonça França
0d4834583f Merge pull request #21276 from rodzyn/fix_build
Fix master build
2015-08-17 18:41:10 -03:00
Marcin Olichwirowicz
48a240bcbe Fix master build 2015-08-17 23:30:32 +02:00
Robin Dupret
b79ab88245 Merge pull request #21273 from piton4eg/patch-6
Small fixes [ci skip]
2015-08-17 23:12:07 +02:00
Aaron Patterson
d12ff4fa50 use predicate methods to avoid is_a? checks
we may want to change the name of the class at some point, so it's
better to use a predicate
2015-08-17 13:51:39 -07:00
Aaron Patterson
e9777ef62e default pattern to use a joined string
The string we create is almost always the same, so rather than joining
all the time, lets join once, then reuse that string everywhere.
2015-08-17 13:51:38 -07:00
Alexey Markov
76c2f01fcb Small fixes [ci skip] 2015-08-17 23:09:31 +03:00
Kasper Timm Hansen
65d3bf9153 Merge pull request #21270 from jonatack/update-debugging-guide-byebug-info
Update the Debugging Rails Guide [skip ci]
2015-08-17 22:01:24 +02:00
Kasper Timm Hansen
f7e625b222 Merge pull request #21244 from ronakjangir47/method_call_assertions_fix
Replacing lambda with proc getting argument error because of it.
2015-08-17 21:53:33 +02:00
Arun Agrawal
2778293ec5 Merge pull request #21272 from amitsuroliya/fix_docs
fix Docs [ci skip]
2015-08-17 21:30:16 +02:00
amitkumarsuroliya
bb289b9409 fix Docs [ci skip] 2015-08-18 00:41:17 +05:30
Richard Schneeman
54042c45e3 Merge pull request #21271 from amitsuroliya/typo_fix
typo fix [ci skip]
2015-08-17 13:54:33 -05:00
amitkumarsuroliya
f2c04d77ba typo fix [ci skip] 2015-08-18 00:02:16 +05:30
Jon Atack
60a3122171 Update the Debugging Rails Guide
[skip ci].

- Update to the current output when running `byebug help`.

- Remove the alias `exit` because it does not work and seems to have
been removed from Byebug, as confirmed by the source code here:
https://github.com/deivid-rodriguez/byebug/blob/master/lib/byebug/comman
ds/quit.rb

- Added the useful `q!` instead to avoid the "Really quit? (y/n)"
prompt.
2015-08-17 20:23:26 +02:00
Rafael Mendonça França
15fd2586a8 Merge pull request #21252 from rodzyn/improve_params_parser
Improve params parser
2015-08-17 13:20:09 -03:00
Ronak Jangir
e49c2cd743 Replacing lambda with proc getting argument error because of it. 2015-08-17 20:28:00 +05:30
Zachary Scott
7985908bea reorganize testing guide. [Zachary Scott & Yves Senn]
[ci skip]

Better reading flow for the information presented in this guide.
The first part is written in a similar fashion as the "Getting Started
Guide" and can be read from start to finish. The second section
introduces the different testing components that Rails provides and
explains how and when to use them.

The guide is still work in progress.
2015-08-17 16:23:59 +02:00
Rafael Mendonça França
b0cbfeb63e Merge pull request #21264 from jonatack/fix-action-controller-strong-params-typo
[skip ci] Fix minor typo
2015-08-17 11:07:47 -03:00
Jon Atack
51b237c955 [skip ci] Fix minor typo 2015-08-17 16:02:56 +02:00
Rafael Mendonça França
9356d9674f Merge pull request #21263 from printercu/patch-1
Fixed syslog example in production config template
2015-08-17 10:50:24 -03:00
printercu
0b7a37aa02 Fixed syslog example in production config template 2015-08-17 16:40:48 +03:00
Sean Griffin
249a06d3be Merge pull request #21135 from DropsOfSerenity/master
make disable_with default in submit_tag
2015-08-17 06:49:00 -06:00
Marcin Olichwirowicz
9366d622be Cleanup ActionDispatch:ParamsParser 2015-08-17 11:38:37 +02:00
Guillermo Iguaran
6c014a4900 Merge pull request #21258 from unfunco/plugin-semver-update
Plugins are generated with the version 0.1.0
2015-08-16 17:48:55 -05:00
Daniel Morris
0a888f0c0d Updated tests for the generated version number change 2015-08-16 23:45:55 +01:00
Rafael Mendonça França
3f866cbdab Fix typo on method name
[Robin Dupret]
2015-08-16 19:28:32 -03:00
Daniel Morris
c3a9e97f84 Plugins are generated with the version 0.1.0
The semantic versioning specification uses MAJOR.MINOR.PATCH – it would
make more sense to set the version to 0.1.0 for initial development
since a patch release cannot be created before a minor feature release.
2015-08-16 23:22:15 +01:00
Claudio B.
0e5ffa740c Merge pull request #21253 from vincefrancesi/date-helper-use-hidden-documentation
Documentation for ActionView::Helpers::DateHelper :use_hidden option
2015-08-16 08:41:20 -07:00
Robin Dupret
cbca29a959 Add a changelog entry for #21124 [ci skip]
[Kir Shatrov & Robin Dupret]
2015-08-16 14:42:09 +02:00
Robin Dupret
b803798b9d Tiny documentation fixes [ci skip]
* Add missing `def` and remove useless `do` keywords.
* Move `:nodoc:` in front of the methods' definition so that methods
  under these ones are correctly visible on the API.
2015-08-16 14:16:20 +02:00
Aaron Patterson
fe19d07138 move route allocation to a factory method on the mapping object
I would like to change the signature of the Route constructor.  Since
the mapping object has all the data required to construct a Route
object, move the allocation to a factory method.
2015-08-15 14:34:37 -07:00
Aaron Patterson
703275ba70 use the mapper to build the routing table
We should build the routes using the user facing API which is `Mapper`.
This frees up the library internals to change as we see fit. IOW we
shouldn't be testing internals.
2015-08-15 14:32:04 -07:00
Aaron Patterson
05eea6a2c3 only process via once
we can directly turn it in to a regular expression here, so we don't
need to test its value twice
2015-08-15 14:32:03 -07:00