| Commit message (Collapse) | Author | Age | Files | Lines |
|\
| |
| | |
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.
|
| |
|
| |
|
|\
| |
| | |
[ci skip] docs: making clear that perform_caching has a limited impact
|
| | |
|
|\ \
| | |
| | | |
Removed usage line docs [ci skip]
|
| | | |
|
| | | |
|
| | |
| | |
| | |
| | | |
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
|
| |
| |
| |
| |
| |
| | |
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`
|
| |
| |
| |
| | |
this prevents mutations from being available globally
|
| |
| |
| |
| |
| |
| | |
Instead of trying to manually clear out a request object, lets just
allocate a new one. The rack ENV is reused and cleaned (still), but the
request object is not.
|
| | |
|
| | |
|
| | |
|
| |
| |
| |
| |
| | |
There is no reason to "recycle" response objects when we can just
allocate a new one.
|
| |
| |
| |
| |
| | |
we should be pushing the cookies in via headers rather than maintaining
some object and "recycling" it.
|
|/ |
|
|\
| |
| | |
Allow default_render to take a block to customize behavior when there's no template
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In 0de4a23 the behavior when there is a missing template was changed to
not raise an error, but instead head :no_content. This is a breaking
change and some gems rely on this happening.
To allow gems and other code to work around this, allow
`default_render` to take a block which, if provided, will
execute the contents of that block instead of doing the `head :no_content`.
|
|\ \
| | |
| | |
| | |
| | |
| | | |
vngrs/strong_parameters_unpermitted_parameters_wrong_doc_fix
Fix the documentation about ActionController::UnpermittedParameters [ci skip]
|