Skip to content

Commit

Permalink
Reduce memory used by ActiveSupport::Callbacks
Browse files Browse the repository at this point in the history 
Previously, callbacks which were shared with subclasses would not share
between them the procs generated in Filters::Before and Filters::After.
This was also lazily generated so would cause memory growth after an
application is booted (and not sharable between workers via CoW).

This was because we would rebuild the objects used to invoke the
callbacks via CallbackChain#compile, so any difference in the callback
chain would result in all of the callback procs being rebuilt.

This commit changes before and after callbacks (but not around!) to be
shared between all subclasses of where it was defined. This is done by
changing Filters::Before and Filters::After to plain classes which
respond to call instead of generating procs (which wasn't strictly
necessary but was easier to implement, and also results in simpler
objects which use less memory). These objects avoid referencing and tied
to a specific callback sequence and so can be memoized and reused.

This has the most impact on applications with many Controllers, and many
callbacks in the ApplicationController (or similar).

I also took this opportunity to merge together all the different forms
of procs generated (halting, halting_and_conditional, conditional,
simple) into one form with if statements. There isn't any significant
performance benefit from the specialization previously being done.
  • Loading branch information
@jhawthorn
jhawthorn committed Oct 21, 2023
1 parent 7c58911 commit 6c5a042
Showing 1 changed file with 63 additions and 108 deletions.
171 changes: 63 additions & 108 deletions activesupport/lib/active_support/callbacks.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def run_callbacks(kind, type = nil)
def halted_callback_hook(filter, name)
end

module Conditionals # :nodoc:
module Conditionals # :nodoc: all
class Value
def initialize(&block)
@block = block
Expand All @@ -159,128 +159,76 @@ def call(target, value); @block.call(value); end
end
end

module Filters
module Filters # :nodoc: all
Environment = Struct.new(:target, :halted, :value)

class Before
def self.build(callback_sequence, user_callback, user_conditions, chain_config, filter, name)
def initialize(user_callback, user_conditions, chain_config, filter, name)
halted_lambda = chain_config[:terminator]

if user_conditions.any?
halting_and_conditional(callback_sequence, user_callback, user_conditions, halted_lambda, filter, name)
else
halting(callback_sequence, user_callback, halted_lambda, filter, name)
end
@user_callback, @user_conditions, @halted_lambda, @filter, @name = user_callback, user_conditions, halted_lambda, filter, name
freeze
end
attr_reader :user_callback, :user_conditions, :halted_lambda, :filter, :name

def self.halting_and_conditional(callback_sequence, user_callback, user_conditions, halted_lambda, filter, name)
callback_sequence.before do |env|
target = env.target
value = env.value
halted = env.halted
def call(env)
target = env.target
value = env.value
halted = env.halted

if !halted && user_conditions.all? { |c| c.call(target, value) }
result_lambda = -> { user_callback.call target, value }
env.halted = halted_lambda.call(target, result_lambda)
if env.halted
target.send :halted_callback_hook, filter, name
end
if !halted && user_conditions.all? { |c| c.call(target, value) }
result_lambda = -> { user_callback.call target, value }
env.halted = halted_lambda.call(target, result_lambda)
if env.halted
target.send :halted_callback_hook, filter, name
end

env
end
end
private_class_method :halting_and_conditional

def self.halting(callback_sequence, user_callback, halted_lambda, filter, name)
callback_sequence.before do |env|
target = env.target
value = env.value
halted = env.halted

unless halted
result_lambda = -> { user_callback.call target, value }
env.halted = halted_lambda.call(target, result_lambda)
if env.halted
target.send :halted_callback_hook, filter, name
end
end
env
end

env
end
def apply(callback_sequence)
callback_sequence.before(self)
end
private_class_method :halting
end

class After
def self.build(callback_sequence, user_callback, user_conditions, chain_config)
if chain_config[:skip_after_callbacks_if_terminated]
if user_conditions.any?
halting_and_conditional(callback_sequence, user_callback, user_conditions)
else
halting(callback_sequence, user_callback)
end
else
if user_conditions.any?
conditional callback_sequence, user_callback, user_conditions
else
simple callback_sequence, user_callback
end
end
attr_reader :user_callback, :user_conditions, :halting
def initialize(user_callback, user_conditions, chain_config)
halting = chain_config[:skip_after_callbacks_if_terminated]
@user_callback, @user_conditions, @halting = user_callback, user_conditions, halting
freeze
end

def self.halting_and_conditional(callback_sequence, user_callback, user_conditions)
callback_sequence.after do |env|
target = env.target
value = env.value
halted = env.halted

if !halted && user_conditions.all? { |c| c.call(target, value) }
user_callback.call target, value
end
def call(env)
target = env.target
value = env.value
halted = env.halted

env
if (!halted || !@halting) && user_conditions.all? { |c| c.call(target, value) }
user_callback.call target, value
end
end
private_class_method :halting_and_conditional

def self.halting(callback_sequence, user_callback)
callback_sequence.after do |env|
unless env.halted
user_callback.call env.target, env.value
end

env
end
env
end
private_class_method :halting

def self.conditional(callback_sequence, user_callback, user_conditions)
callback_sequence.after do |env|
target = env.target
value = env.value

if user_conditions.all? { |c| c.call(target, value) }
user_callback.call target, value
end

env
end
def apply(callback_sequence)
callback_sequence.after(self)
end
private_class_method :conditional
end

def self.simple(callback_sequence, user_callback)
callback_sequence.after do |env|
user_callback.call env.target, env.value
class Around
def initialize(user_callback, user_conditions)
@user_callback, @user_conditions = user_callback, user_conditions
freeze
end

env
end
def apply(callback_sequence)
callback_sequence.around(@user_callback, @user_conditions)
end
private_class_method :simple
end
end

class Callback # :nodoc:#
class Callback # :nodoc:
def self.build(chain, filter, kind, options)
if filter.is_a?(String)
raise ArgumentError, <<-MSG.squish
Expand Down Expand Up @@ -329,19 +277,26 @@ def duplicates?(other)
end
end

def compiled
@compiled ||=
begin
user_conditions = conditions_lambdas
user_callback = CallTemplate.build(@filter, self)

case kind
when :before
Filters::Before.new(user_callback.make_lambda, user_conditions, chain_config, @filter, name)
when :after
Filters::After.new(user_callback.make_lambda, user_conditions, chain_config)
when :around
Filters::Around.new(user_callback, user_conditions)
end
end
end

# Wraps code with filter
def apply(callback_sequence)
user_conditions = conditions_lambdas
user_callback = CallTemplate.build(@filter, self)

case kind
when :before
Filters::Before.build(callback_sequence, user_callback.make_lambda, user_conditions, chain_config, @filter, name)
when :after
Filters::After.build(callback_sequence, user_callback.make_lambda, user_conditions, chain_config)
when :around
callback_sequence.around(user_callback, user_conditions)
end
compiled.apply(callback_sequence)
end

def current_scopes
Expand Down Expand Up @@ -375,7 +330,7 @@ def conditions_lambdas

# A future invocation of user-supplied code (either as a callback,
# or a condition filter).
module CallTemplate # :nodoc:
module CallTemplate # :nodoc: all
class MethodCall
def initialize(method)
@method_name = method
Expand Down Expand Up @@ -566,12 +521,12 @@ def initialize(nested = nil, call_template = nil, user_conditions = nil)
@after = []
end

def before(&before)
def before(before)
@before.unshift(before)
self
end

def after(&after)
def after(after)
@after.push(after)
self
end
Expand Down

0 comments on commit 6c5a042

Please sign in to comment.