aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Refactor remove duplication.Santosh Wadghule2016-08-311-2/+6
|
* Merge pull request #25880 from ↵Andrew White2016-08-301-1/+1
|\ | | | | | | | | ryandv/fix_performance_regression_in_timewithzone_to_time Fix performance regression in `TimeWithZone#to_time`
| * Memoize coerced TimeWithZone value in TimeWithZone#localtime.Ryan De Villa2016-08-231-7/+1
| |
| * Fix performance regression in `TimeWithZone#to_time`Ryan De Villa2016-08-231-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A performance regression was introduced by commit b79adc4323ff289aed3f5787fdfbb9542aa4f89f from Rails 4.0.0.beta1, in which `TimeWithZone#to_time` no longer returns a cached instance attribute but instead coerces the value to `Time`. This coerced value is not cached, and recomputation degrades the performance of comparisons between TimeWithZone objects. See https://github.com/rails/rails/commit/b79adc4323ff289aed3f5787fdfbb9542aa4f89f#diff-3497a506c921a3a3e40fd517e92e4fe3R322 for the change in question. The following benchmark, which reverts the change linked above, demonstrates the performance regression: require 'active_support/time' require 'benchmark/ips' utc = Time.utc(2000, 1, 1, 0) time_zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)'] twz = ActiveSupport::TimeWithZone.new(utc, time_zone) twz2 = ActiveSupport::TimeWithZone.new(Time.utc(1999, 12, 31, 23, 59, 59), ActiveSupport::TimeZone['UTC']) patchedTimeWithZone = Class.new(ActiveSupport::TimeWithZone) do def to_time utc end end patched_twz = patchedTimeWithZone.new(utc, time_zone) patched_twz2 = patchedTimeWithZone.new(Time.utc(1999, 12, 31, 23, 59, 59), ActiveSupport::TimeZone['UTC']) Benchmark.ips do |x| x.report("comparison out of the box") { twz <=> twz2 } x.report("comparison reverting to_time") { patched_twz <=> patched_twz2 } x.compare! end The results, when run in rails-dev-box, are as follows: Warming up -------------------------------------- comparison out of the box 24.765k i/100ms comparison reverting to_time 57.237k i/100ms Calculating ------------------------------------- comparison out of the box 517.245k (± 4.7%) i/s - 2.600M in 5.038700s comparison reverting to_time 2.624M (± 5.0%) i/s - 13.050M in 4.985808s Comparison: comparison reverting to_time: 2624266.1 i/s comparison out of the box: 517244.6 i/s - 5.07x slower The change made to run the benchmark, however, is not possible, as it would undo the intent to standardize the return value of `to_time` to `Time` in the system timezone. Our proposed solution is to restore the caching behaviour of `to_time` as it existed prior to the change linked above. Benchmark of our solution: require 'active_support/time' require 'benchmark/ips' patchedTimeWithZone = Class.new(ActiveSupport::TimeWithZone) do def to_time @to_time ||= super end end utc = Time.utc(2000, 1, 1, 0) time_zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)'] twz = ActiveSupport::TimeWithZone.new(utc, time_zone) twz2 = ActiveSupport::TimeWithZone.new(Time.utc(1999, 12, 31, 23, 59, 59), ActiveSupport::TimeZone['UTC']) patched_twz = patchedTimeWithZone.new(utc, time_zone) patched_twz2 = patchedTimeWithZone.new(Time.utc(1999, 12, 31, 23, 59, 59), ActiveSupport::TimeZone['UTC']) Benchmark.ips do |x| x.report("TimeWithZone comparison - existing implementation") { twz <=> twz2 } x.report("TimeWithZone comparison - caching implementation") { patched_twz <=> patched_twz2 } x.compare! end Results in rails-dev-box: Warming up -------------------------------------- TimeWithZone comparison - existing implementation 26.629k i/100ms TimeWithZone comparison - caching implementation 59.144k i/100ms Calculating ------------------------------------- TimeWithZone comparison - existing implementation 489.757k (± 4.2%) i/s - 2.450M in 5.011639s TimeWithZone comparison - caching implementation 2.802M (± 5.3%) i/s - 13.958M in 4.996116s Comparison: TimeWithZone comparison - caching implementation: 2801519.1 i/s TimeWithZone comparison - existing implementation: 489756.7 i/s - 5.72x slower
* | Merge pull request #26317 from maclover7/jm-fix-26298Aaron Patterson2016-08-293-8/+30
|\ \ | | | | | | Allow `send_file` to declare a charset
| * | Remove default argument, and extract internal convenience methodJon Moss2016-08-291-6/+12
| | |
| * | Add regression testsJon Moss2016-08-291-0/+14
| | |
| * | Allow `send_file` to declare a charsetJon Moss2016-08-292-4/+6
| | | | | | | | | | | | | | | | | | Removed my patch in favor of @tenderlove's less invasive approach. [Aaron Patterson & Jon Moss]
* | | Merge pull request #26318 from alexcameron89/contributing-doc-updateAndrew White2016-08-291-1/+1
|\ \ \ | | | | | | | | [ci skip] Update branch version in Contributing Guide
| * | | [ci skip] Update branch version in Contributing GuideAlex Kitchens2016-08-291-1/+1
|/ / / | | | | | | | | | | | | Since only Rails 4 and above are currently supported, this updates a 3-2-stable example to be consistent with the support standard.
* / / Fix nested multiple rootsRyo Hashimoto2016-08-293-2/+61
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | The PR #20940 enabled the use of multiple roots with different constraints at the top level but unfortunately didn't work when those roots were inside a namespace and also broke the use of root inside a namespace after a top level root was defined because the check for the existence of the named route used the global :root name and not the namespaced name. This is fixed by using the name_for_action method to expand the :root name to the full namespaced name. We can pass nil for the second argument as we're not dealing with resource definitions so don't need to handle the cases for edit and new routes. Fixes #26148.
* | Remove unneeded FIXME noteRafael Mendonça França2016-08-291-2/+0
| | | | | | | | | | This is the intended behavior. You should not do more than one request in a controller test.
* | Merge pull request #26311 from y-yagi/use_inspect_for_show_valueKasper Timm Hansen2016-08-292-1/+10
|\ \ | | | | | | use `inspect` for show `from` value
| * | use `inspect` for show `from` valueyuuji.yaginuma2016-08-292-1/+10
| | | | | | | | | | | | If `from` is nil, in order to avoid the blank is showed.
* | | Merge pull request #26310 from kjellberg/guidesJon Moss2016-08-281-0/+1
|\ \ \ | |/ / |/| | Add .gitignore to "files & purpose" list - getting started docs
| * | Added gitignore to "files & purpose" list [ci skip]Rasmus Kjellberg2016-08-291-0/+1
|/ /
* | Merge pull request #26305 from kamipo/follow-up-to-26301Kasper Timm Hansen2016-08-281-3/+1
|\ \ | | | | | | Switch back to `Hash.dup`
| * | Switch back to `Hash.dup`Ryuta Kamizono2016-08-281-3/+1
| | | | | | | | | | | | Follow up to #26301.
* | | Merge pull request #26306 from kamipo/remove_under_ruby-1.9Kasper Timm Hansen2016-08-281-9/+9
|\ \ \ | |/ / |/| | Remove "Under Ruby 1.9" [ci skip]
| * | Remove "Under Ruby 1.9" [ci skip]Ryuta Kamizono2016-08-281-9/+9
|/ / | | | | | | Rails dropped Ruby 1.9 support, but this comment still true.
* | validate_each in NumericalityValidator is never called in this case.Guillermo Iguaran2016-08-281-2/+0
| | | | | | | | | | NumericalityValidator#validate_each is never called when allow_nil is true and the value is nil because it is already skipped in EachValidator#validate.
* | Add test for allow_blank in numericality validationNicolai Reuschling2016-08-281-0/+7
| | | | | | | | Signed-off-by: Guillermo Iguaran <guilleiguaran@gmail.com>
* | Merge pull request #26301 from maclover7/jm-fix-commentGuillermo Iguaran2016-08-271-4/+2
|\ \ | | | | | | Switch back to `Hash.dup`
| * | Switch back to `Hash.dup`Jon Moss2016-08-271-4/+2
|/ / | | | | | | | | | | | | | | | | | | | | | | | | The performance difference between `Hash[]` and `Hash.dup` looks to have been narrowed by @tenderlove via this commit --> https://github.com/ruby/ruby/commit/b3803cc49ad382e23291d75ce57ffb2b74bb9577#diff-eff9999082c8ce7d8ba1fc1d79f439cf. Since this commit first appeared in Ruby 2.0.0, and since Rails now requires a minimum Ruby version of 2.2.2, this performance boost should be available for all users. Relevant links: - This behavior was originally added via https://github.com/rails/rails/commit/02174a3efc6fa8f2e5e6f114e4cf0d8a06305b6a - The conversation on the Ruby issue tracker lives here --> https://bugs.ruby-lang.org/issues/7166
* | Merge pull request #26299 from davydovanton/fix-typoVipul A M2016-08-271-1/+1
|\ \ | | | | | | Fix typo in Delegation#delegate_missing_to doc [ci skip]
| * | Fix typo in Delegation#delegate_missing_to doc [skip ci]Anton Davydov2016-08-271-1/+1
|/ /
* | Merge pull request #26297 from ledermann/patch-1Vijay Dev2016-08-271-1/+1
|\ \ | | | | | | Errors#add_on_blank: Fix typo in deprecation message
| * | Fix typo in deprecation messageGeorg Ledermann2016-08-271-1/+1
|/ / | | | | This fixes a copy-and-paste-issue slipped in by #18996
* | Merge pull request #26293 from mechanicles/move-custom-assertionGuillermo Iguaran2016-08-272-11/+11
|\ \ | | | | | | Move Rails' custom assertion method `assert_nothing_raised` to its proper place.
| * | Move custom assertion to its proper placeSantosh Wadghule2016-08-272-11/+11
|/ / | | | | | | | | | | | | | | ActiveSupport::Testing::Assertions. We have a separate module in which have defined Rails' own custom assertions. So it would be good to keep all custom Rails' assertions in one place i.e. in this module.
* | Missing key should throw KeyErroreileencodes2016-08-262-1/+7
| | | | | | | | | | | | It should not throw a NameError, but should throw a KeyError. Fixes #26278
* | Merge pull request #26288 from maclover7/jm-revert-21995Guillermo Iguaran2016-08-261-7/+11
|\ \ | | | | | | Update `Rails.configuration` documentation
| * | Clarify two ways to set Rails configuration optionsJon Moss2016-08-261-5/+8
| | | | | | | | | | | | [ci skip]
| * | Add back in `config_for` exampleJon Moss2016-08-261-0/+28
| | | | | | | | | | | | [ci skip]
| * | Revert "Merge pull request #21995 from tak1n/master"Jon Moss2016-08-261-35/+8
| | | | | | | | | | | | | | | This reverts commit 4973704bf56dbb0d8beba977e1053d57e346ebd0, reversing changes made to 78edeb33346e13ab33a62d2a6b553aabf5b3186a.
* | | Merge pull request #26287 from mechanicles/rails-assertionJon Moss2016-08-261-1/+1
|\ \ \ | |/ / |/| | Move `assert_nothing_raised` method to Rails Specific Assertions section [ci skip]
| * | Move `assert_nothing_raised` method to Rails Specific AssertionsSantosh Wadghule2016-08-261-1/+1
|/ / | | | | | | | | | | | | | | section [ci skip] - Method `assert_nothing_raised` is Rails' own custom assertion method and not a part of Minitest. So move it from Minitest assertions section to Rails Specific Assertions section.
* | Merge pull request #26284 from kamipo/fix_warnigGuillermo Iguaran2016-08-251-1/+1
|\ \ | | | | | | Fix "warning: assigned but unused variable - task"
| * | Fix "warning: assigned but unused variable - task"Ryuta Kamizono2016-08-261-1/+1
|/ /
* | Fix typo in the hook nameRafael Mendonça França2016-08-251-1/+1
| |
* | Add load hooks to all tests classesRafael Mendonça França2016-08-254-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Usually users extends tests classes doing something like: ActionView::TestCase.include MyCustomTestHelpers This is bad because it will load the ActionView::TestCase right aways and this will load ActionController::Base making its on_load hooks to execute early than it should. One way to fix this is using the on_load hooks of the components like: ActiveSupport.on_load(:action_view) do ActionView::TestCase.include MyCustomTestHelpers end The problem with this approach is that the test extension will be only load when ActionView::Base is loaded and this may happen too late in the test. To fix this we are adding hooks to people extend the test classes that will be loaded exactly when the test classes are needed.
* | kick different instrumentation methodAaron Patterson2016-08-241-8/+12
| | | | | | | | | | | | | | We can eliminate a conditional by calling a different instrumentation method depending on the situation. In this case, we'll call the special case "!render_template" instrumentation method and eliminate the case / when clause from the `instrument` method.
* | remove useless freezeAaron Patterson2016-08-241-1/+1
| | | | | | | | Ruby already does this freeze for us.
* | Simplify cache hit loggingAaron Patterson2016-08-242-5/+5
| | | | | | | | | | | | | | CacheHelper is mixed in to Helpers, Helpers is mixed in to AV::Base. This means we can count on instances of AV::Base to have the "cache hit" method on them, and we can stop setting an ivar for cache logging and just ask the view if it was a cache hit.
* | Merge pull request #26271 from jcoleman/fix-unnecessary-query-cache-busting-testGuillermo Iguaran2016-08-241-0/+20
|\ \ | | | | | | Test that AR query cache isn't busted when types are not same object
| * | Test that AR query cache isn't busted when types are not same objectJames Coleman2016-08-241-0/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is fixed in 5.0 as an ancillary part of 574f255629a45cd67babcfb9bb8e163e091a53b8 but here I also add a test for the condition. I'd previously backported the fix (and added a test) in the below commit; this brings the fix back up to master. (cherry picked from commit fce3dbf30241f2a65c777e192a7171b0eea81453)
* | | Merge pull request #26235 from ↵Sean Griffin2016-08-242-1/+23
|\ \ \ | |/ / |/| | | | | | | | samphippen/allow-early-setting-of-integration-session Allow the `integration_sesion` to be set early on ActionDispatch::Integration::Runner.
| * | Move setting of integration session to constructor.Sam Phippen2016-08-211-1/+5
| | | | | | | | | | | | This allows us to not `||=` in `before_setup`.
| * | Allow the `integration_sesion` to be set early on ↵Sam Phippen2016-08-202-1/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ActionDispatch::Integration::Runner. In commit fa63448420d3385dbd043aca22dba973b45b8bb2, @tenderlove changed the behaviour of the way `integration_session` is set up in this object. It used to be the case that the first time it was accessed, it was memoized with nil, however, this means that if it had already been set it was not replaced. After that commit, it is now always set to `nil` in the execution of `before_setup`. In RSpec, users are able to invoke `host!` in `before(:all)` blocks, which execute well before `before_setup` is ever invoked (which happens in what is equivalent to a `before(:each)` block, for each test. `host!` causes the integration session to be set up to correctly change the host, but after fa63448420d3385dbd043aca22dba973b45b8bb2 the `integration_session` gets overwritten, meaning that users lose their `host!` configuration (see https://github.com/rspec/rspec-rails/issues/1662). This commit changes the behaviour back to memoizing with `nil`, as opposed to directly overwriting with `nil`. This causes the correct behaviour to occur in RSpec, and unless I'm mistaken will also ensure that users who want to modify their integration sessions early in rails will also be able to do so.
* | | Merge pull request #26267 from riseshia/patch-1Rafael França2016-08-231-2/+2
|\ \ \ | | | | | | | | Rename the title of comand_line.md in index