| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since Rails 6.0 will support Ruby 2.4.1 or higher
`# frozen_string_literal: true` magic comment is enough to make string object frozen.
This magic comment is enabled by `Style/FrozenStringLiteralComment` cop.
* Exclude these files not to auto correct false positive `Regexp#freeze`
- 'actionpack/lib/action_dispatch/journey/router/utils.rb'
- 'activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb'
It has been fixed by https://github.com/rubocop-hq/rubocop/pull/6333
Once the newer version of RuboCop released and available at Code Climate these exclude entries should be removed.
* Replace `String#freeze` with `String#-@` manually if explicit frozen string objects are required
- 'actionpack/test/controller/test_case_test.rb'
- 'activemodel/test/cases/type/string_test.rb'
- 'activesupport/lib/active_support/core_ext/string/strip.rb'
- 'activesupport/test/core_ext/string_ext_test.rb'
- 'railties/test/generators/actions_test.rb'
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In Ruby 2.3 or later, `String#+@` is available and `+@` is faster than `dup`.
```ruby
# frozen_string_literal: true
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
gem "benchmark-ips"
end
Benchmark.ips do |x|
x.report('+@') { +"" }
x.report('dup') { "".dup }
x.compare!
end
```
```
$ ruby -v benchmark.rb
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux]
Warming up --------------------------------------
+@ 282.289k i/100ms
dup 187.638k i/100ms
Calculating -------------------------------------
+@ 6.775M (± 3.6%) i/s - 33.875M in 5.006253s
dup 3.320M (± 2.2%) i/s - 16.700M in 5.032125s
Comparison:
+@: 6775299.3 i/s
dup: 3320400.7 i/s - 2.04x slower
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
### Summary
This PR changes .rubocop.yml.
Regarding the code using `if ... else ... end`, I think the coding style
that Rails expects is as follows.
```ruby
var = if cond
a
else
b
end
```
However, the current .rubocop.yml setting does not offense for the
following code.
```ruby
var = if cond
a
else
b
end
```
I think that the above code expects offense to be warned.
Moreover, the layout by autocorrect is unnatural.
```ruby
var = if cond
a
else
b
end
```
This PR adds a setting to .rubocop.yml to make an offense warning and
autocorrect as expected by the coding style.
And this change also fixes `case ... when ... end` together.
Also this PR itself is an example that arranges the layout using
`rubocop -a`.
### Other Information
Autocorrect of `Lint/EndAlignment` cop is `false` by default.
https://github.com/bbatsov/rubocop/blob/v0.51.0/config/default.yml#L1443
This PR changes this value to `true`.
Also this PR has changed it together as it is necessary to enable
`Layout/ElseAlignment` cop to make this behavior.
|
| |
|
| |
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| |
|
| |
|
| |
|
| |
|
|
|
|
| |
Use ActionDispatch::Request instead of Request because ActionDispatch::Request no longer inherits from Rack::Request.
|
| |
|
|
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It used to behave like this:
url_for(controller: 'x', action: 'y', q: {})
# -> "/x/y?"
We previously avoided empty query strings in most cases by removing
nil values, then checking whether params was empty. But as you can
see above, even non-empty params can yield an empty query string. So
I changed the code to just directly check whether the query string
ended up empty.
(To make everything more consistent, the "removing nil values"
functionality should probably move to ActionPack's Hash#to_query, the
place where empty hashes and arrays get removed. However, this would
change a lot more behavior.)
|
|
|
|
|
|
| |
Just include the modules necessary in the Request object to implement
the things we need. This should make it easier to build delegate
request objects because the API is smaller
|
| |
|
|\
| |
| | |
Fix broken IPv6 addresses handling
|
| | |
|
| | |
|
| | |
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
![](https://www.dropbox.com/s/z4dj9fdsv213r4v/let-it-go.gif?dl=1)
|
|
|
|
|
|
| |
Previously, an empty X_FORWARDED_HOST header would cause
Actiondispatch::Http:URL.raw_host_with_port to return nil, causing
Actiondispatch::Http:URL.host to raise a NoMethodError.
|
|
|
|
|
|
|
|
|
|
|
|
| |
```ruby
article = Article.new.tap(&:save!)
view.url_for article
result = ObjectSpace::AllocationTracer.trace do
3000.times { view.url_for article }
end
p ObjectSpace::AllocationTracer.allocated_count_table[:T_STRING] / 3000
```
|
|
|
|
|
|
|
|
|
|
| |
Add docs for `extract_domain`, `extract_subdomains`, `extract_subdomain`.
Add doc examples for `url`, `protocol`, `raw_host_with_port`, `host`,
`host_with_port`, `port`, `standard_port`, `standard_port?`, `optional_port`,
`port_string`.
[ci skip]
|
|
|
|
| |
Fixes https://github.com/rails/rails/issues/17714.
|
| |
|
|
|
|
|
| |
These methods mutate the path variable/argument so there is no need
to reassign it every time.
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
since we know that the route should be a path or fully qualified, we can
pass a strategy object that handles generation. This allows us to
eliminate an "if only_path" branch when generating urls.
|
| |
|
| |
|
| |
|
|
|
|
| |
Then we only need to extract host once.
|
| |
|
| |
|
|
|
|
|
| |
if the subdomain wasn't specified, it's the same as if specifying
:subdomain as `true`, so we can default the value to `true` safely.
|
| |
|
|
|
|
|
| |
`normalize_host` already calls `named_host?`, so there is no reason to
test `named_host?` again in the `extract_domain` method.
|
|
|
|
|
|
|
|
|
|
| |
Before:
/Users/Juan/dev/rails/actionpack/lib/action_dispatch/http/url.rb:95: warning: shadowing outer local variable - port
After:
No warning
|
|
|
|
|
| |
extract_subdomain always returns a string, and to_param calls to_s on a
string
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
| |
irb(main):004:0> /foo/ !~ nil
=> true
irb(main):005:0> /foo/ !~ 'bar'
=> true
irb(main):006:0> /foo/ !~ 'foo'
=> false
|