aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack/lib
Commit message (Collapse)AuthorAgeFilesLines
* Merge pull request #20946 from schneems/schneems/let-it-goSean Griffin2015-07-1910-18/+18
|\ | | | | Freeze string literals when not mutated.
| * Freeze string literals when not mutated.schneems2015-07-1910-18/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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)
* | Merge pull request #20936 from repinel/fix-params-fetch-exception-overwrittenSean Griffin2015-07-191-3/+7
|\ \ | |/ |/| Fix exception overwritten for parameters fetch method
| * Fix exception overwritten for parameters fetch methodRoque Pinel2015-07-181-3/+7
| | | | | | | | | | | | | | | | | | 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]
* | Fix formatting of force_ssl options documentation [ci skip]Eliot Sykes2015-07-191-6/+6
|/
* Stop using deprecated `render :text` in testPrem Sichanugrist2015-07-174-7/+7
| | | | | | | | | 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`.
* Merge pull request #20917 from sikachu/ps-deprecate-render-textJeremy Daer (Kemper)2015-07-171-0/+14
|\ | | | | Add deprecation warning for `render :text`
| * Add deprecation warning for `render :text`Prem Sichanugrist2015-07-171-0/+14
| | | | | | | | | | | | 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.
* | push fields_for_style? in to a protected methodAaron Patterson2015-07-171-5/+5
| | | | | | | | | | this way we don't need to call `to_unsafe_h` to get access to ask questions about the underlying hash
* | push is_a checks up the stackAaron Patterson2015-07-171-13/+13
| | | | | | | | now `hash_filter` doesn't need to know about the `Parameters` class
* | remove useless conditionalAaron Patterson2015-07-171-1/+0
| | | | | | | | | | Since we proved that `element` is always of type `Parameter`, we know that it will always respond to `permit`, so lets remove this conditional
* | remove useless conditionalsAaron Patterson2015-07-171-2/+2
| | | | | | | | | | | | | | | | `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
* | remove useless functionAaron Patterson2015-07-171-8/+2
| | | | | | | | | | | | 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
* | stop passing `params` to `array_of_permitted_scalars_filter`Aaron Patterson2015-07-171-4/+6
| | | | | | | | | | | | 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
* | push key checking upAaron Patterson2015-07-171-1/+2
|/ | | | | | 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
* Oops! :bomb:Akira Matsuda2015-07-171-1/+1
|
* "warning: instance variable @routes not initialized"Akira Matsuda2015-07-171-1/+1
|
* Standardize `ActionController::Parameters#to_unsafe_h` return valueZoltan Kiss2015-07-151-1/+1
| | | | | | `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.
* Update documentation on `AC::Parameters`Prem Sichanugrist2015-07-151-6/+32
|
* Make AC::Parameters not inherited from HashPrem Sichanugrist2015-07-152-28/+103
| | | | | | | | 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 implements `redirect?` so we don't need itAaron Patterson2015-07-142-6/+0
| | | | 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.
* Change AC::TestResponse to AD::TestResponsePrem Sichanugrist2015-07-141-1/+1
| | | | | ActionController::TestResponse was removed in d9fe10c and caused a test failure on Action View as its test case still refers to it.
* use a lookup table for `assert_response`Aaron Patterson2015-07-131-3/+8
| | | | | | | | 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
* only have one TestResponse classAaron Patterson2015-07-131-4/+1
|
* move buffer caching on to the bufferAaron Patterson2015-07-133-7/+15
|
* Merge pull request #20736 from antoine-lizee/docsRobin Dupret2015-07-131-1/+1
|\ | | | | [ci skip] docs: making clear that perform_caching has a limited impact
| * [ci skip] doc: making clear that perform_caching has a limited impactantoine.lizee2015-07-131-1/+1
| |
* | Merge pull request #20842 from TheBlasfem/removed_usage_lines_docsClaudio B.2015-07-112-2/+2
|\ \ | | | | | | Removed usage line docs [ci skip]
| * | added description instead of remove usage [ci skip]Julio Lopez2015-07-112-2/+2
| | |
* | | Merge pull request #17102 from matthewd/load-interlockAaron Patterson2015-07-102-0/+22
|\ \ \ | | | | | | | | Concurrent load interlock (rm Rack::Lock)
| * | | Fix the Interlock middlewareMatthew Draper2015-07-091-5/+14
| | | | | | | | | | | | | | | | | | | | 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.
| * | | Soften the lock requirements when eager_load is disabledMatthew Draper2015-07-092-0/+13
| | | | | | | | | | | | | | | | | | | | We don't need to fully disable concurrent requests: just ensure that loads are performed in isolation.
* | | | we don't really need an extra method to set the script nameAaron Patterson2015-07-101-5/+1
| | | |
* | | | Remove useless conditionalAaron Patterson2015-07-101-12/+0
| | | | | | | | | | | | | | | | PATH_INFO is already set, so this branch will never execute.
* | | | default `PATH_INFO` to the generated pathAaron Patterson2015-07-101-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | 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
* | | | always default the SCRIPT_NAME to whatever is on the controllerAaron Patterson2015-07-101-1/+1
| | | |
* | | | remove useless ivar clearingAaron Patterson2015-07-101-6/+0
| | | | | | | | | | | | | | | | Since we only work with new instances, these ivars will not be set.
* | | | call the `path_parameters=` setter rather than rely on mutationsAaron Patterson2015-07-101-1/+3
| | | | | | | | | | | | | | | | | | | | We should call the setter on `path_parameters` so that we know the hash will only contain the values that we've set.
* | | | start collecting `env` mutationsAaron Patterson2015-07-101-5/+3
| | | | | | | | | | | | | | | | | | | | I'd like to put all env mutations together so we can understand how to change this code to call `call` on the controller
* | | | Parameters are converted to a query stringAaron Patterson2015-07-101-17/+0
| | | | | | | | | | | | | | | | | | | | Since parameters are converted to a query string, they will automatically be turned in to strings by the query parser
* | | | no more HWIAAaron Patterson2015-07-101-1/+1
| | | | | | | | | | | | | | | | | | | | non_path_parameters is used internally (it never escapes this method) so we should be able to safely use a regular hash.
* | | | remove param dup'ing logicAaron Patterson2015-07-101-8/+0
| | | | | | | | | | | | | | | | | | | | since we are serializing parameters, we don't need to do all the dup checks on each object.
* | | | encode / decode parameters before assigning them to the requestAaron Patterson2015-07-101-5/+58
| | | | | | | | | | | | | | | | | | | | | | | | 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
* | | | start disconnecting the parameter parser from the instanceAaron Patterson2015-07-101-3/+3
| | | | | | | | | | | | | | | | | | | | pass in the instance variable to start decoupling the meat of the parser from the instance of the middleware
* | | | drop a conditional by always assigningAaron Patterson2015-07-101-6/+5
| |/ / |/| | | | | | | | | | | We will always make an assignment to the env hash and eliminate a conditional
* | | drop runtime conditionals in parameter parsingAaron Patterson2015-07-091-13/+9
| | | | | | | | | | | | | | | If we only deal with proc objects, then we can eliminate type checking in the parameter parsing middleware
* | | set parameters as a query stringAaron Patterson2015-07-091-6/+9
| | | | | | | | | | | | | | | | | | 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
* | | build and assign parameters rather than rely on mutationsAaron Patterson2015-07-081-1/+15
| | | | | | | | | | | | | | | We should assign parameters to the request object rather than mutate the hash that is returned by `query_parameters` or `request_parameters`
* | | assign the cookie hash on request allocationAaron Patterson2015-07-082-2/+2
| | | | | | | | | | | | this prevents mutations from being available globally
* | | request objects are no longer recycledAaron Patterson2015-07-081-10/+8
| | | | | | | | | | | | | | | | | | 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.