| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| |
| |
| |
| | |
Saves 888 string objects per request.
|
| |
| |
| |
| |
| |
| |
| |
| | |
When generating a url with `url_for` the hash of arguments passed in, is dup-d and merged a TON. I wish I could clean this up better, and might be able to do it in the future. This change removes one dup, since it's literally right after we just dup-d the hash to pass into this constructor.
This may be a breaking, change but the tests pass...so :shipit: we can revert if it causes problems
This change buys us 205,933 bytes of memory and 887 fewer objects per request.
|
|/
|
|
|
|
|
|
| |
In handle_positional_args `Array#-=` is used which allocates a new array. Instead we can iterate through and delete elements, modifying the array in place.
Also `Array#take` allocates a new array. We can build the same by iterating over the other element.
This change buys us 106,470 bytes of memory and 2,663 fewer objects per request.
|
|\
| |
| | |
Remove unnecessary `dup` from Mapper `add_route`
|
| |
| |
| |
| |
| |
| |
| | |
The `dup` was introduced by c4106d0c08954b0761726e0015ec601b7bc7ea4b
to work around a frozen key. Nowadays, the string is already being
duplicated by the `tr` in `options[:action] ||= action.tr('-', '_')`
and later joined into a new string in `name_for_action`.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
I wrote a utility that helps find areas where you could optimize your program using a frozen string instead of a string literal, it's called [let_it_go](https://github.com/schneems/let_it_go). After going through the output and adding `.freeze` I was able to eliminate the creation of 1,114 string objects on EVERY request to [codetriage](codetriage.com). How does this impact execution?
To look at memory:
```ruby
require 'get_process_mem'
mem = GetProcessMem.new
GC.start
GC.disable
1_114.times { " " }
before = mem.mb
after = mem.mb
GC.enable
puts "Diff: #{after - before} mb"
```
Creating 1,114 string objects results in `Diff: 0.03125 mb` of RAM allocated on every request. Or 1mb every 32 requests.
To look at raw speed:
```ruby
require 'benchmark/ips'
number_of_objects_reduced = 1_114
Benchmark.ips do |x|
x.report("freeze") { number_of_objects_reduced.times { " ".freeze } }
x.report("no-freeze") { number_of_objects_reduced.times { " " } }
end
```
We get the results
```
Calculating -------------------------------------
freeze 1.428k i/100ms
no-freeze 609.000 i/100ms
-------------------------------------------------
freeze 14.363k (± 8.5%) i/s - 71.400k
no-freeze 6.084k (± 8.1%) i/s - 30.450k
```
Now we can do some maths:
```ruby
ips = 6_226k # iterations / 1 second
call_time_before = 1.0 / ips # seconds per iteration
ips = 15_254 # iterations / 1 second
call_time_after = 1.0 / ips # seconds per iteration
diff = call_time_before - call_time_after
number_of_objects_reduced * diff * 100
# => 0.4530373333993266 miliseconds saved per request
```
So we're shaving off 1 second of execution time for every 220 requests.
Is this going to be an insane speed boost to any Rails app: nope. Should we merge it: yep.
p.s. If you know of a method call that doesn't modify a string input such as [String#gsub](https://github.com/schneems/let_it_go/blob/b0e2da69f0cca87ab581022baa43291cdf48638c/lib/let_it_go/core_ext/string.rb#L37) please [give me a pull request to the appropriate file](https://github.com/schneems/let_it_go/blob/b0e2da69f0cca87ab581022baa43291cdf48638c/lib/let_it_go/core_ext/string.rb#L37), or open an issue in LetItGo so we can track and freeze more strings.
Keep those strings Frozen

|
|/
|
|
|
|
|
|
| |
This is another take at #14384 as we decided to wait until `master` is
targeting Rails 5.0. This commit is implementation-complete, as it
guarantees that all the public methods on the hash-inherited Parameters
are still working (based on test case). We can decide to follow-up later
if we want to remove some methods out from Parameters.
|
|
|
|
|
|
|
|
|
| |
Clarify the `url_for` usage in mailers.
Re-add the documentation about `url_for` and Route's path parameters,
first introduced by 5c4f1859970d06228a0b67cad6d4486c1526ef2a.
This was reported on #15097 and until it is decided to deprecate it
or not, I believe the documentation should exist.
|
|
|
|
|
|
|
|
| |
This reverts commit 0b3397872582f2cf1bc6960960a6393f477c55e6, reversing
changes made to 56d52e3749180e6c1dcf7166adbad967470aa78b.
As pointed out on the PR, this will hide development mistakes too, which
is not ideal.
|
|
|
|
|
| |
Handle URI::InvalidURIError errors on the redirect route method, so it
wont raise a 500 if a bad path is given.
|
|
|
|
|
| |
See the following commit to have context about this change:
https://github.com/rails/rails/commit/757a2bc3e3e52a5d9418656928db993db42b741b
|
| |
|
| |
|
| |
|
|
|
|
|
| |
matches? should only deal with methods on the request object, so lets
just filter out anything that the request object doesn't respond to
|
|
|
|
|
| |
this way we can remove the strange "respond_to?" conditional in the
`matches?` loop
|
|
|
|
|
|
| |
match method without setting `:via` option has been deprecated
fix minor typo
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
We need to ignore the `assets_prefix` when running a command like `rake
routes`. However we cannot reference asserts_prefix from action_pack as
that is a sprockets-rails concern.
See this is now implemented on sprockets-rails
https://github.com/rails/sprockets-rails/commit/85b89c44ad40af3056899808475e6e4bf65c1f5a
|
|\
| |
| | |
A shorter and more concise version of select..size
|
| | |
|
| |
| |
| |
| |
| |
| | |
it is avoid sort errot within different and mixed keys.
used `sort_by` + `block` to list parameter by keys.
keep minimum changes
|
| |
| |
| |
| |
| | |
This way we can get the relative_url_root from the application without
setting another global value
|
| |
| |
| |
| |
| | |
If you would like to use a custom request class, please subclass and implemet
the `request_class` method.
|
|\ \
| | |
| | |
| | | |
Fallback to RAILS_RELATIVE_URL_ROOT in `url_for`
|
| | |
| | |
| | |
| | |
| | |
| | | |
Fixed an issue where the `RAILS_RELATIVE_URL_ROOT` environment
variable is not prepended to the path when `url_for` is called.
If `SCRIPT_NAME` (used by Rack) is set, it takes precedence.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
this patch makes errors slightly more expensive when someone is missing
a route key, but in exchange it drops 4 allocations per `url_for` call.
Since missing a route key is an error, optimizing for the non-error path
seems like a good trade off
|
| | |
| | |
| | |
| | |
| | | |
we already know the length of the args, so we can use that length for
parallel iteration and cut down on allocations for `url_for` calls.
|
| | |
| | |
| | |
| | |
| | |
| | | |
this centralizes the logic for determining the script name key and drops
object allocations when calling `engine_script_name` (which is called on
each `url_for`).
|
| | | |
|
| | | |
|
| | |
| | |
| | |
| | |
| | | |
_generate_paths_by_default wasn't used in AD::Routing::UrlFor, so we
should be able to move it where it is used in AV::Routing
|
| | | |
|
|/ /
| |
| |
| |
| |
| |
| |
| | |
Change filter on /rails/info/routes to use an actual path regexp from rails
and not approximate javascript version. Oniguruma supports much more
extensive list of features than javascript regexp engine.
Fixes #18402.
|
|\ \
| | |
| | | |
Don't use shorthand match on routes with inappropriate symbols
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Shorthand route match is when controller and action are taken literally from path.
E.g.
get '/foo/bar' # => will use 'foo#bar' as endpoint
get '/foo/bar/baz' # => will use 'foo/bar#baz' as endpoint
Not any path with level two or more of nesting can be used as shortcut.
If path contains any characters outside of /[\w-]/ then it can't be
used as such.
This commit ensures that invalid shortcuts aren't used.
':controller/:action/postfix' - is an example of invalid shortcut
that was previosly matched and led to exception:
"ArgumentError - ':controller/:action' is not a supported controller name"
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This has 2 effects:
1. RoutesProxy is CRAZY faster because it's no longer creating a new
Module each time method_missing is hit.
2. It bypasses an existing bug in ruby that makes `class << obj` unsafe
to be used in threading contexts.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
We shouldn't cache if it's not absolutely necessary. Removes
route caching and instead skips using the `url_helpers` is the
integration test session doesn't require it. Benchmark ips on
integration and controller index method tests below.
Without any caching or changes to `#url_helpers`:
```
Calculating -------------------------------------
INDEX: Integration Test
71.000 i/100ms
INDEX: Functional Test
99.000 i/100ms
-------------------------------------------------
INDEX: Integration Test
728.878 (± 8.0%) i/s - 3.692k
INDEX: Functional Test
1.015k (± 6.7%) i/s - 5.148k
Comparison:
INDEX: Functional Test: 1015.4 i/s
INDEX: Integration Test: 728.9 i/s - 1.39x slower
```
With caching on `#url_helpers`:
```
Calculating -------------------------------------
INDEX: Integration Test
74.000 i/100ms
INDEX: Functional Test
99.000 i/100ms
-------------------------------------------------
INDEX: Integration Test
752.377 (± 6.9%) i/s - 3.774k
INDEX: Functional Test
1.021k (± 6.7%) i/s - 5.148k
Comparison:
INDEX: Functional Test: 1021.1 i/s
INDEX: Integration Test: 752.4 i/s - 1.36x slower
```
Afer removing the caching and bypassing the `url_helpers` when not
necessary in the session:
```
Calculating -------------------------------------
INDEX: Integration Test
87.000 i/100ms
INDEX: Functional Test
97.000 i/100ms
-------------------------------------------------
INDEX: Integration Test
828.433 (± 6.4%) i/s - 4.176k
INDEX: Functional Test
926.763 (± 7.2%) i/s - 4.656k
Comparison:
INDEX: Functional Test: 926.8 i/s
INDEX: Integration Test: 828.4 i/s - 1.12x slower
```
|
| | |
| | |
| | |
| | |
| | | |
The use of `# :startdoc:` inside of the class was overriding the
outer-most `# :nodoc:`, causing it to be listed in the documented API.
|
| | |
| | |
| | |
| | |
| | | |
The commit 3b63780 re-introduced url helper caching but we need to
cache a separate module for Action Mailer without paths.
|
| | |
| | |
| | |
| | |
| | |
| | | |
`url_helpers` used to be memoized. This was lost in a refactoring and
this PR adds it back. We noticed this while investigating why
integration tests are slower than controller tests.
|
| | |
| | |
| | |
| | |
| | | |
Fixes an issue that would cause default_url_options to be lost when generating
URLs with fewer positional arguments than parameters in the route definition.
|
|/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
I found delegate to be a bottleneck during integration tests. Here is
the test case:
```ruby
require 'test_helper'
class DocumentsIntegrationTest < ActionDispatch::IntegrationTest
test "index" do
get '/documents'
assert_equal 200, response.status
end
end
Minitest.run_one_method(DocumentsIntegrationTest, 'test_index')
StackProf.run(mode: :wall, out: 'stackprof.dump') do
3000.times do
Minitest.run_one_method(DocumentsIntegrationTest, 'test_index')
end
end
```
Top of the stack:
```
[aaron@TC integration_performance_test (master)]$ stackprof stackprof.dump
==================================
Mode: wall(1000)
Samples: 23694 (7.26% miss rate)
GC: 1584 (6.69%)
==================================
TOTAL (pct) SAMPLES (pct) FRAME
7058 (29.8%) 6178 (26.1%) block in Module#delegate
680 (2.9%) 680 (2.9%) ActiveSupport::PerThreadRegistry#instance
405 (1.7%) 405 (1.7%) ThreadSafe::NonConcurrentCacheBackend#[]
383 (1.6%) 383 (1.6%) Set#include?
317 (1.3%) 317 (1.3%) ActiveRecord::Base.logger
281 (1.2%) 281 (1.2%) Rack::Utils::HeaderHash#[]=
269 (1.1%) 269 (1.1%) ActiveSupport::Notifications::Fanout::Subscribers::Evented#subscribed_to?
262 (1.1%) 262 (1.1%) block (4 levels) in Class#class_attribute
384 (1.6%) 246 (1.0%) block (2 levels) in Class#class_attribute
```
According to @eileencodes's tests, this speeds up integration tests so
that they are only 1.4x slower than functional tests:
Before:
INDEX: Integration Test: 153.2 i/s - 2.43x slower
After:
INDEX: Integration Test: 275.1 i/s - 1.41x slower
|
| | |
|
| |
| |
| |
| | |
These requires were added only to change deprecation message
|
| | |
|
| | |
|
| | |
|