Skip to content

Commit 796cab4

Browse files
committed
Reduce allocations when running AR callbacks.
Inspired by @tenderlove's work in c363fff, 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
1 parent 3a9b3ba commit 796cab4

File tree

8 files changed

+31
-22
lines changed

8 files changed

+31
-22
lines changed

activemodel/lib/active_model/validations.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ def invalid?(context = nil)
390390
protected
391391

392392
def run_validations! #:nodoc:
393-
run_callbacks :validate
393+
run_validate_callbacks
394394
errors.empty?
395395
end
396396
end

activemodel/lib/active_model/validations/callbacks.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ def after_validation(*args, &block)
108108

109109
# Overwrite run validations to include callbacks.
110110
def run_validations! #:nodoc:
111-
run_callbacks(:validation) { super }
111+
run_validation_callbacks { super }
112112
end
113113
end
114114
end

activerecord/lib/active_record/associations/has_many_through_association.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ def delete_records(records, method)
159159
count = scope.destroy_all.length
160160
else
161161
scope.to_a.each do |record|
162-
record.run_callbacks :destroy
162+
record.run_destroy_callbacks
163163
end
164164

165165
arel = scope.arel

activerecord/lib/active_record/callbacks.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,25 +289,25 @@ module ClassMethods
289289
end
290290

291291
def destroy #:nodoc:
292-
run_callbacks(:destroy) { super }
292+
run_destroy_callbacks { super }
293293
end
294294

295295
def touch(*) #:nodoc:
296-
run_callbacks(:touch) { super }
296+
run_touch_callbacks { super }
297297
end
298298

299299
private
300300

301301
def create_or_update #:nodoc:
302-
run_callbacks(:save) { super }
302+
run_save_callbacks { super }
303303
end
304304

305305
def _create_record #:nodoc:
306-
run_callbacks(:create) { super }
306+
run_create_callbacks { super }
307307
end
308308

309309
def _update_record(*) #:nodoc:
310-
run_callbacks(:update) { super }
310+
run_update_callbacks { super }
311311
end
312312
end
313313
end

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ def checkin(conn)
360360
synchronize do
361361
owner = conn.owner
362362

363-
conn.run_callbacks :checkin do
363+
conn.run_checkin_callbacks do
364364
conn.expire
365365
end
366366

@@ -449,7 +449,7 @@ def checkout_new_connection
449449
end
450450

451451
def checkout_and_verify(c)
452-
c.run_callbacks :checkout do
452+
c.run_checkout_callbacks do
453453
c.verify!
454454
end
455455
c

activerecord/lib/active_record/core.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ def initialize(attributes = nil, options = {})
272272
init_attributes(attributes, options) if attributes
273273

274274
yield self if block_given?
275-
run_callbacks :initialize unless _initialize_callbacks.empty?
275+
run_initialize_callbacks
276276
end
277277

278278
# Initialize an empty model object from +coder+. +coder+ must contain
@@ -294,8 +294,8 @@ def init_with(coder)
294294

295295
self.class.define_attribute_methods
296296

297-
run_callbacks :find
298-
run_callbacks :initialize
297+
run_find_callbacks
298+
run_initialize_callbacks
299299

300300
self
301301
end
@@ -331,7 +331,7 @@ def initialize_dup(other) # :nodoc:
331331
@attributes = @attributes.dup
332332
@attributes.reset(self.class.primary_key)
333333

334-
run_callbacks(:initialize) unless _initialize_callbacks.empty?
334+
run_initialize_callbacks
335335

336336
@aggregation_cache = {}
337337
@association_cache = {}

activerecord/lib/active_record/transactions.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,15 +309,15 @@ def rollback_active_record_state!
309309
# Ensure that it is not called if the object was never persisted (failed create),
310310
# but call it after the commit of a destroyed object.
311311
def committed!(should_run_callbacks = true) #:nodoc:
312-
run_callbacks :commit if should_run_callbacks && destroyed? || persisted?
312+
run_commit_callbacks if should_run_callbacks && destroyed? || persisted?
313313
ensure
314314
force_clear_transaction_record_state
315315
end
316316

317317
# Call the +after_rollback+ callbacks. The +force_restore_state+ argument indicates if the record
318318
# state should be rolled back to the beginning or just to the last savepoint.
319319
def rolledback!(force_restore_state = false, should_run_callbacks = true) #:nodoc:
320-
run_callbacks :rollback if should_run_callbacks
320+
run_rollback_callbacks if should_run_callbacks
321321
ensure
322322
restore_transaction_record_state(force_restore_state)
323323
clear_transaction_record_state

activesupport/lib/active_support/callbacks.rb

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,21 @@ module Callbacks
7878
# save
7979
# end
8080
def run_callbacks(kind, &block)
81-
cbs = send("_#{kind}_callbacks")
82-
if cbs.empty?
83-
yield if block_given?
81+
send "run_#{kind}_callbacks", &block
82+
end
83+
84+
private
85+
86+
def _run_callbacks(callbacks, &block)
87+
if callbacks.empty?
88+
block.call if block
8489
else
85-
runner = cbs.compile
90+
runner = callbacks.compile
8691
e = Filters::Environment.new(self, false, nil, block)
8792
runner.call(e).value
8893
end
8994
end
9095

91-
private
92-
9396
# A hook invoked every time a before callback is halted.
9497
# This can be overridden in AS::Callback implementors in order
9598
# to provide better debugging/logging.
@@ -722,6 +725,12 @@ def define_callbacks(*names)
722725
names.each do |name|
723726
class_attribute "_#{name}_callbacks"
724727
set_callbacks name, CallbackChain.new(name, options)
728+
729+
module_eval <<-RUBY, __FILE__, __LINE__ + 1
730+
def run_#{name}_callbacks(&block)
731+
_run_callbacks(_#{name}_callbacks, &block)
732+
end
733+
RUBY
725734
end
726735
end
727736

0 commit comments

Comments
 (0)