| Commit message (Collapse) | Author | Age | Files | Lines |
|\
| |
| | |
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`.
|
|\ \
| | |
| | | |
Freeze string literals when not mutated.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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)
|
|\ \ \
| |/ /
|/| | |
Fix exception overwritten for parameters fetch method
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
When executing an `ActionController::Parameters#fetch` with a block
that raises a `KeyError` the raised `KeyError` will be rescued and
converted to an `ActionController::ParameterMissing` exception,
covering up the original exception.
[Jonas Schubert Erlandsson & Roque Pinel]
|
|/ / |
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This will silence deprecation warnings.
Most of the test can be changed from `render :text` to render `:plain`
or `render :body` right away. However, there are some tests that needed
to be fixed by hand as they actually assert the default Content-Type
returned from `render :body`.
|
|\ \
| | |
| | | |
Add deprecation warning for `render :text`
|
| | |
| | |
| | |
| | |
| | |
| | | |
We've started on discouraging the usage of `render :text` in #12374.
This is a follow-up commit to make sure that we print out the
deprecation warning.
|
| | |
| | |
| | |
| | |
| | | |
this way we don't need to call `to_unsafe_h` to get access to ask
questions about the underlying hash
|
| | |
| | |
| | |
| | | |
now `hash_filter` doesn't need to know about the `Parameters` class
|
| | |
| | |
| | |
| | |
| | | |
Since we proved that `element` is always of type `Parameter`, we know
that it will always respond to `permit`, so lets remove this conditional
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
`element` can never be a hash because:
1. `slice` returns a Parameters object and calls each on it: https://github.com/rails/rails/blob/cb3f25593b1137e344086364d4b1a52c08e8eb3b/actionpack/lib/action_controller/metal/strong_parameters.rb#L656
2. `each` which is implemented by `each_pair` will call `convert_hashes_to_parameters` on the value: https://github.com/rails/rails/blob/cb3f25593b1137e344086364d4b1a52c08e8eb3b/actionpack/lib/action_controller/metal/strong_parameters.rb#L192-197
3. `convert_hashes_to_parameters` will convert any hash objects in to parameters objects: https://github.com/rails/rails/blob/cb3f25593b1137e344086364d4b1a52c08e8eb3b/actionpack/lib/action_controller/metal/strong_parameters.rb#L550-566
|
| | |
| | |
| | |
| | |
| | |
| | | |
Now that the value is cached on the stack,
`array_of_permitted_scalars_filter` is exactly the same as
`array_of_permitted_scalars?`, so lets just have one
|
| | |
| | |
| | |
| | |
| | |
| | | |
this way the method doesn't have to know what the new params object is,
it just yields to a block. This change also caches the value of
`self[key]` on the stack
|
|/ /
| |
| |
| |
| |
| | |
We should disconnect `array_of_permitted_scalars_filter` from the
instance so that we can make hash filtering functional. For now, pull
the conditional up out of that method
|
| | |
|
| | |
|
| |
| |
| |
| |
| |
| | |
`ActionController::Parameters#to_h` returns a hash, so lets have
`ActionController::Parameters#to_unsafe_h` return a hash instead of
an `ActiveSupport::HashWithIndifferentAccess` for consistency.
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
| |
| |
| |
| | |
Rack [already implements `redirect?` on the response object](https://github.com/rack/rack/blob/1569a985e17d9caaf94d0e97d95ef642c4ab14ba/lib/rack/response.rb#L141) so we don't need to implement our own.
|
| |
| |
| |
| |
| | |
ActionController::TestResponse was removed in d9fe10c and caused a test
failure on Action View as its test case still refers to it.
|
| |
| |
| |
| |
| |
| |
| |
| | |
We shouldn't depend on specific methods imlemented in the TestResponse
subclass because the response could actually be a real response object.
In the future, we should either push the aliased predicate methods in
TestResponse up to the real response object, or remove them
|
| | |
|
| | |
|
|\ \
| | |
| | | |
[ci skip] docs: making clear that perform_caching has a limited impact
|
| | | |
|
|\ \ \
| | | |
| | | | |
Removed usage line docs [ci skip]
|
| | | | |
|
|\ \ \ \
| | | | |
| | | | | |
Concurrent load interlock (rm Rack::Lock)
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
We can't actually lean on Rack::Lock's implementation, so we'll just
copy it instead. It's simple enough that that's not too troubling.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
We don't need to fully disable concurrent requests: just ensure that
loads are performed in isolation.
|
| | | | | |
|
| | | | |
| | | | |
| | | | |
| | | | | |
PATH_INFO is already set, so this branch will never execute.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
we were already generating a path in the previous code (it was just not
returned), so lets just use the already computed path to popluate the
PATH_INFO header
|
| | | | | |
|
| | | | |
| | | | |
| | | | |
| | | | | |
Since we only work with new instances, these ivars will not be set.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
We should call the setter on `path_parameters` so that we know the hash
will only contain the values that we've set.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
I'd like to put all env mutations together so we can understand how to
change this code to call `call` on the controller
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Since parameters are converted to a query string, they will
automatically be turned in to strings by the query parser
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
non_path_parameters is used internally (it never escapes this method) so
we should be able to safely use a regular hash.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
since we are serializing parameters, we don't need to do all the dup
checks on each object.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
We should roundtrip the parameters through their respective encoders /
decoders so that the controller will get parameters similar to what they
actually get in a real world situation
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
pass in the instance variable to start decoupling the meat of the parser
from the instance of the middleware
|
| |/ / /
|/| | |
| | | |
| | | |
| | | | |
We will always make an assignment to the env hash and eliminate a
conditional
|
| | | |
| | | |
| | | |
| | | |
| | | | |
If we only deal with proc objects, then we can eliminate type checking
in the parameter parsing middleware
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
We should convert request parameters to a query string, then let the
request object parse that query string. This should give us results
that are more similar to the real-world
|
| | | |
| | | |
| | | |
| | | |
| | | | |
We should assign parameters to the request object rather than mutate the
hash that is returned by `query_parameters` or `request_parameters`
|