diff options
author | Pete Higgins <pete@peterhiggins.org> | 2014-09-28 14:42:26 -0700 |
---|---|---|
committer | Pete Higgins <pete@peterhiggins.org> | 2014-09-28 15:38:32 -0700 |
commit | 796cab45561fce268aa74e6587cdb9cae3bb243e (patch) | |
tree | bb1bafe2911318c3ff061650a062ded660dccc89 /activerecord | |
parent | 3a9b3ba082d50498751f6c144ef31dbefd7f0357 (diff) | |
download | rails-796cab45561fce268aa74e6587cdb9cae3bb243e.tar.gz rails-796cab45561fce268aa74e6587cdb9cae3bb243e.tar.bz2 rails-796cab45561fce268aa74e6587cdb9cae3bb243e.zip |
Reduce allocations when running AR callbacks.
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
Diffstat (limited to 'activerecord')
5 files changed, 14 insertions, 14 deletions
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 0968b0068e..455a540bdb 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -159,7 +159,7 @@ module ActiveRecord count = scope.destroy_all.length else scope.to_a.each do |record| - record.run_callbacks :destroy + record.run_destroy_callbacks end arel = scope.arel diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 5955673b42..1aa760157a 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -289,25 +289,25 @@ module ActiveRecord end def destroy #:nodoc: - run_callbacks(:destroy) { super } + run_destroy_callbacks { super } end def touch(*) #:nodoc: - run_callbacks(:touch) { super } + run_touch_callbacks { super } end private def create_or_update #:nodoc: - run_callbacks(:save) { super } + run_save_callbacks { super } end def _create_record #:nodoc: - run_callbacks(:create) { super } + run_create_callbacks { super } end def _update_record(*) #:nodoc: - run_callbacks(:update) { super } + run_update_callbacks { super } end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 9760729da3..da43e5bb10 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -360,7 +360,7 @@ module ActiveRecord synchronize do owner = conn.owner - conn.run_callbacks :checkin do + conn.run_checkin_callbacks do conn.expire end @@ -449,7 +449,7 @@ module ActiveRecord end def checkout_and_verify(c) - c.run_callbacks :checkout do + c.run_checkout_callbacks do c.verify! end c diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 069aa977bf..5571a2d297 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -272,7 +272,7 @@ module ActiveRecord init_attributes(attributes, options) if attributes yield self if block_given? - run_callbacks :initialize unless _initialize_callbacks.empty? + run_initialize_callbacks end # Initialize an empty model object from +coder+. +coder+ must contain @@ -294,8 +294,8 @@ module ActiveRecord self.class.define_attribute_methods - run_callbacks :find - run_callbacks :initialize + run_find_callbacks + run_initialize_callbacks self end @@ -331,7 +331,7 @@ module ActiveRecord @attributes = @attributes.dup @attributes.reset(self.class.primary_key) - run_callbacks(:initialize) unless _initialize_callbacks.empty? + run_initialize_callbacks @aggregation_cache = {} @association_cache = {} diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 45bc10b9b0..e53297d0ab 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -309,7 +309,7 @@ module ActiveRecord # Ensure that it is not called if the object was never persisted (failed create), # but call it after the commit of a destroyed object. def committed!(should_run_callbacks = true) #:nodoc: - run_callbacks :commit if should_run_callbacks && destroyed? || persisted? + run_commit_callbacks if should_run_callbacks && destroyed? || persisted? ensure force_clear_transaction_record_state end @@ -317,7 +317,7 @@ module ActiveRecord # Call the +after_rollback+ callbacks. The +force_restore_state+ argument indicates if the record # state should be rolled back to the beginning or just to the last savepoint. def rolledback!(force_restore_state = false, should_run_callbacks = true) #:nodoc: - run_callbacks :rollback if should_run_callbacks + run_rollback_callbacks if should_run_callbacks ensure restore_transaction_record_state(force_restore_state) clear_transaction_record_state |