| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
| |
We added a comparison to "id", and call to `self.class.primary_key` a
*lot*. We also have performance hits from `&block` all over the place.
We skip the check in a new method, in order to avoid breaking the
behavior of `read_attribute`
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch uniformizes warning messages. I used the most common style
already present in the code base:
* Capitalize the first word.
* End the message with a full stop.
* "Rails 5" instead of "Rails 5.0".
* Backticks for method names and inline code.
Also, converted a few long strings into the new heredoc convention.
|
|
|
|
|
|
|
|
|
|
|
|
| |
The current style for warning messages without newlines uses
concatenation of string literals with manual trailing spaces
where needed.
Heredocs have better readability, and with `squish` we can still
produce a single line.
This is a similar use case to the one that motivated defining
`strip_heredoc`, heredocs are super clean.
|
|
|
|
| |
This will avoid naming clash with user defined methods
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Inspired by @tenderlove's work in
c363fff29f060e6a2effe1e4bb2c4dd4cd805d6e, this reduces the number of
strings allocated when running callbacks for ActiveRecord instances. I
measured that using this script:
```
require 'objspace'
require 'active_record'
require 'allocation_tracer'
ActiveRecord::Base.establish_connection adapter: "sqlite3",
database: ":memory:"
ActiveRecord::Base.connection.instance_eval do
create_table(:articles) { |t| t.string :name }
end
class Article < ActiveRecord::Base; end
a = Article.create name: "foo"
a = Article.find a.id
N = 10
result = ObjectSpace::AllocationTracer.trace do
N.times { Article.find a.id }
end
result.sort.each do |k,v|
p k => v
end
puts "total: #{result.values.map(&:first).inject(:+)}"
```
When I run this against master and this branch I get this output:
```
pete@balloon:~/projects/rails/activerecord$ git checkout master
M Gemfile
Switched to branch 'master'
pete@balloon:~/projects/rails/activerecord$ bundle exec ruby benchmark_allocation_with_callback_send.rb > allocations_before
pete@balloon:~/projects/rails/activerecord$ git checkout remove-dynamic-send-on-built-in-callbacks
M Gemfile
Switched to branch 'remove-dynamic-send-on-built-in-callbacks'
pete@balloon:~/projects/rails/activerecord$ bundle exec ruby benchmark_allocation_with_callback_send.rb > allocations_after
pete@balloon:~/projects/rails/activerecord$ diff allocations_before allocations_after
39d38
<
{["/home/pete/projects/rails/activesupport/lib/active_support/callbacks.rb",
81]=>[40, 0, 0, 0, 0, 0]}
42c41
< total: 630
---
> total: 590
```
In addition to this, there are two micro-optimizations present:
* Using `block.call if block` vs `yield if block_given?` when the block was being captured already.
```
pete@balloon:~/projects$ cat benchmark_block_call_vs_yield.rb
require 'benchmark/ips'
def block_capture_with_yield &block
yield if block_given?
end
def block_capture_with_call &block
block.call if block
end
def no_block_capture
yield if block_given?
end
Benchmark.ips do |b|
b.report("block_capture_with_yield") { block_capture_with_yield }
b.report("block_capture_with_call") { block_capture_with_call }
b.report("no_block_capture") { no_block_capture }
end
pete@balloon:~/projects$ ruby benchmark_block_call_vs_yield.rb
Calculating -------------------------------------
block_capture_with_yield
124979 i/100ms
block_capture_with_call
138340 i/100ms
no_block_capture 136827 i/100ms
-------------------------------------------------
block_capture_with_yield
5703108.9 (±2.4%) i/s - 28495212 in 4.999368s
block_capture_with_call
6840730.5 (±3.6%) i/s - 34169980 in 5.002649s
no_block_capture 5821141.4 (±2.8%) i/s - 29144151 in 5.010580s
```
* Defining and calling methods instead of using send.
```
pete@balloon:~/projects$ cat benchmark_method_call_vs_send.rb
require 'benchmark/ips'
class Foo
def tacos
nil
end
end
my_foo = Foo.new
Benchmark.ips do |b|
b.report('send') { my_foo.send('tacos') }
b.report('call') { my_foo.tacos }
end
pete@balloon:~/projects$ ruby benchmark_method_call_vs_send.rb
Calculating -------------------------------------
send 97736 i/100ms
call 151142 i/100ms
-------------------------------------------------
send 2683730.3 (±2.8%) i/s - 13487568 in 5.029763s
call 8005963.9 (±2.7%) i/s - 40052630 in 5.006604s
```
The result of this is making typical ActiveRecord operations slightly faster:
https://gist.github.com/phiggins/e46e51dcc7edb45b5f98
|
|
|
|
|
|
|
|
|
|
| |
Using heredoc would enforce line wrapping to whatever column width we decided to
use in the code, making it difficult for the users to read on some consoles.
This does make the source code read slightly worse and a bit more error-prone,
but this seems like a fair price to pay since the primary purpose for these
messages are for the users to read and the code will not stick around for too
long.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Reliant on https://github.com/rails/rails/pull/15747 but pulled to a
separate PR to reduce noise. `has_many :through` associations have the
undocumented behavior of automatically detecting counter caches.
However, the way in which it does so is inconsistent with counter caches
everywhere else, and doesn't actually work consistently.
As with normal `has_many` associations, the user should specify the
counter cache on the `belongs_to`, if they'd like it updated.
|
|\
| |
| | |
Always update counter caches in memory when adding records
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Before, calling `size` would only work if it skipped the cache, and
would return a different result from the cache, but only if:
- The association was previously loaded
- Or you called size previously
- But only if the size was 0 when you called it
This ensures that the counter is appropriately updated in memory.
|
|\ \
| |/
|/|
| | |
Don't include inheritance column in the through_scope_attributes
|
|/ |
|
|\
| |
| | |
Open extension point for defining options in build_through_record
|
| |
| |
| |
| | |
This fixes #15496
|
|/
|
|
|
|
|
|
|
|
|
|
|
| |
member = Member.new(club: Club.new)
member.save!
Before:
member.current_membership.club_id # => nil
After:
member.current_membership.club_id # => club's id
|
|
|
|
|
|
|
| |
Reflection has an available method that is used to check if the
reflection is a collection. Any :has_many macro is considered a
collection and `collection?` should be used instead of
`macro == :has_many`.
|
|
|
|
|
|
| |
Instead of checking for `macro == :has_one` throughout the
codebase we can create a `has_one?` method to match the `belongs_to?`,
`polymorphic?` and other methods.
|
|
|
|
|
|
|
| |
Integration tests are inside protected_attributes test suite.
Fixes #15496
Fixes rails/protected_attributes#35
|
|
|
|
|
|
|
|
|
|
| |
Reflection has a `belongs_to?` method. Instead of checking for
`macro == :belongs_to` throughout the source reuse existing
method.
I also bumped `foreign_key_present?` method onto on line because
the `belongs_to?` makes it shorter than other longer lines in
the same class.
|
|
|
|
|
|
|
| |
Rename delete_all_records because this name better describes
what the method is doing. We can then remove :all from the
hm:t version and pull out the unoptimized call to load_target
in delete_records and pass it directly.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Refactor by creating two methods delete_all_records and delete_records
to be called by delete_all and delete (or destroy) respectively.
This reduces the number of conditionals required to handle _how_
records get deleted.
The new delete_count method handles how scope is applied to which
delete action.
A delete_all_records method also has to be called in has_many_through
association because of how the methods are chained. This will be
refactored later on.
|
|
|
|
|
| |
HABTM should fall back to using the normal CollectionAssociation's size calculation if the collection is not cached or loaded.
This addresses both #14913 and #14914 for master.
|
|
|
|
|
|
|
|
| |
Rewrite to avoid 'we'/'you', add missing period, and keep lines at 80 chars. Cheers :)
Improve readability with help from @senny
[ci skip]
|
| |
|
| |
|
| |
|
|\
| |
| |
| |
| |
| |
| | |
Fix insertion of records for hmt association with scope
Conflicts:
activerecord/CHANGELOG.md
|
| | |
|
|/
|
|
|
|
|
|
|
|
| |
`before_add` callbacks are fired before the record is saved on
`has_and_belongs_to_many` assocations *and* on `has_many :through`
associations. Before this change, `before_add` callbacks would be fired
before the record was saved on `has_and_belongs_to_many` associations, but
*not* on `has_many :through` associations.
Fixes #14144
|
|
|
|
|
| |
when destroying a record on a has_many :through association.
:destroy method has own counter_cache callbacks.
|
| |
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
This commit fixes reported issue #7630 in which counter
caches were not being updated properly when replacing
has_many_through relationships
|
|\
| |
| | |
Integrate strong_parameters in Rails 4
|
| | |
|
|/
|
|
|
|
| |
If assigning to a has_many :through collection against an unsaved
object using the collection=[<array_of_items>] syntax, the join models
were not properly created, previously.
|
| |
|
| |
|
|
|
|
|
|
| |
It has been moved to active_record_deprecated_finders.
Use #to_a instead.
|
| |
|
|
|
|
| |
Bug #6289
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
| |
If a record is removed from a has_many :through, all of the join records
relating to that record should also be removed from the through
association's target.
(Previously the records were removed in the database, but only one was
removed from the in-memory target array.)
|
|
|
|
| |
Fixes #3425.
|