Let delegate define method with proper arity when delegating to a Class / Module
Method delegation with `...` argument is known to be slow because it allocates redundant Array and Hash objects each time when being called. see: https://bugs.ruby-lang.org/issues/19165 Current implementation of `delegate` defines all delegation methods in this slow form regardless of the original method's arity, but in case the delegation target is a Class or a Module, we can investigate the arity of the original method in the definition timing, then we can define the delegation in proper minimal arity. This makes 3.5 times faster zero arity method delegation as follows: Warming up -------------------------------------- old 811.534k i/100ms new 1.807M i/100ms Calculating ------------------------------------- old 9.809M (± 3.4%) i/s - 49.504M in 5.053355s new 34.360M (± 0.8%) i/s - 173.465M in 5.048692s Comparison: new: 34360408.4 i/s old: 9809157.4 i/s - 3.50x (± 0.00) slower
This commit is contained in:
Akira Matsuda
parent
4f28043066
commit
8b2f57dc6f
@ -1,3 +1,25 @@
|
||||
* `delegate` now defines method with proper arity when delegating to a Class.
|
||||
With this change, it defines faster method (3.5x faster with no argument).
|
||||
However, in order to gain this benefit, the delegation target method has to
|
||||
be defined before declaring the delegation.
|
||||
|
||||
```ruby
|
||||
# This defines 3.5 times faster method than before
|
||||
class C
|
||||
def self.x() end
|
||||
delegate :x, to: :class
|
||||
end
|
||||
|
||||
class C
|
||||
# This works but silently falls back to old behavior because
|
||||
# `delegate` cannot find the definition of `x`
|
||||
delegate :x, to: :class
|
||||
def self.x() end
|
||||
end
|
||||
```
|
||||
|
||||
*Akira Matsuda*
|
||||
|
||||
* `assert_difference` message now includes what changed.
|
||||
|
||||
This makes it easier to debug non-obvious failures.
|
||||
|
||||
@ -187,8 +187,8 @@ def delegate(*methods, to: nil, prefix: nil, allow_nil: nil, private: nil)
|
||||
location = caller_locations(1, 1).first
|
||||
file, line = location.path, location.lineno
|
||||
|
||||
to = to.to_s
|
||||
to = "self.#{to}" if DELEGATION_RESERVED_METHOD_NAMES.include?(to)
|
||||
receiver = to.to_s
|
||||
receiver = "self.#{receiver}" if DELEGATION_RESERVED_METHOD_NAMES.include?(receiver)
|
||||
|
||||
method_def = []
|
||||
method_names = []
|
||||
@ -199,7 +199,35 @@ def delegate(*methods, to: nil, prefix: nil, allow_nil: nil, private: nil)
|
||||
|
||||
# Attribute writer methods only accept one argument. Makes sure []=
|
||||
# methods still accept two arguments.
|
||||
definition = /[^\]]=\z/.match?(method) ? "arg" : "..."
|
||||
definition = \
|
||||
if /[^\]]=\z/.match?(method)
|
||||
"arg"
|
||||
else
|
||||
method_object =
|
||||
begin
|
||||
if to.is_a?(Module)
|
||||
to.method(method)
|
||||
elsif receiver == "self.class"
|
||||
method(method)
|
||||
end
|
||||
rescue NameError
|
||||
# Do nothing. Fall back to `"..."`
|
||||
end
|
||||
|
||||
if method_object
|
||||
parameters = method_object.parameters
|
||||
|
||||
if (parameters.map(&:first) & [:opt, :rest, :keyreq, :key, :keyrest]).any?
|
||||
"..."
|
||||
else
|
||||
defn = parameters.filter_map { |type, arg| arg if type == :req }
|
||||
defn << "&block" if parameters.last&.first == :block
|
||||
defn.join(", ")
|
||||
end
|
||||
else
|
||||
"..."
|
||||
end
|
||||
end
|
||||
|
||||
# The following generated method calls the target exactly once, storing
|
||||
# the returned value in a dummy variable.
|
||||
@ -213,7 +241,7 @@ def delegate(*methods, to: nil, prefix: nil, allow_nil: nil, private: nil)
|
||||
|
||||
method_def <<
|
||||
"def #{method_name}(#{definition})" <<
|
||||
" _ = #{to}" <<
|
||||
" _ = #{receiver}" <<
|
||||
" if !_.nil? || nil.respond_to?(:#{method})" <<
|
||||
" _.#{method}(#{definition})" <<
|
||||
" end" <<
|
||||
@ -224,11 +252,11 @@ def delegate(*methods, to: nil, prefix: nil, allow_nil: nil, private: nil)
|
||||
|
||||
method_def <<
|
||||
"def #{method_name}(#{definition})" <<
|
||||
" _ = #{to}" <<
|
||||
" _ = #{receiver}" <<
|
||||
" _.#{method}(#{definition})" <<
|
||||
"rescue NoMethodError => e" <<
|
||||
" if _.nil? && e.name == :#{method}" <<
|
||||
%( raise DelegationError, "#{self}##{method_name} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}") <<
|
||||
%( raise DelegationError, "#{self}##{method_name} delegated to #{receiver}.#{method}, but #{receiver} is nil: \#{self.inspect}") <<
|
||||
" else" <<
|
||||
" raise" <<
|
||||
" end" <<
|
||||
|
||||
@ -8,16 +8,16 @@
|
||||
end
|
||||
|
||||
Someone = Struct.new(:name, :place) do
|
||||
def self.table_name
|
||||
"some_table"
|
||||
end
|
||||
|
||||
delegate :street, :city, :to_f, to: :place
|
||||
delegate :name=, to: :place, prefix: true
|
||||
delegate :upcase, to: "place.city"
|
||||
delegate :table_name, to: :class
|
||||
delegate :table_name, to: :class, prefix: true
|
||||
|
||||
def self.table_name
|
||||
"some_table"
|
||||
end
|
||||
|
||||
self::FAILED_DELEGATE_LINE = __LINE__ + 1
|
||||
delegate :foo, to: :place
|
||||
|
||||
@ -171,6 +171,27 @@ def shift
|
||||
end
|
||||
|
||||
class ModuleTest < ActiveSupport::TestCase
|
||||
class ArityTester
|
||||
class << self
|
||||
def zero; end
|
||||
def zero_with_block(&bl); end
|
||||
def one(a) end
|
||||
def one_with_block(a) end
|
||||
def two(a, b) end
|
||||
def opt(a, b, c, d = nil) end
|
||||
def kwargs(a:, b:) end
|
||||
def kwargs_with_block(a:, b:, c:, &block) end
|
||||
def opt_kwargs(a:, b: 3) end
|
||||
def opt_kwargs_with_block(a:, b:, c:, d: "", &block) end
|
||||
end
|
||||
end
|
||||
|
||||
module ArityTesterModule
|
||||
class << self
|
||||
def zero; end
|
||||
end
|
||||
end
|
||||
|
||||
def setup
|
||||
@david = Someone.new("David", Somewhere.new("Paulina", "Chicago"))
|
||||
end
|
||||
@ -553,4 +574,50 @@ def initialize(place)
|
||||
assert_equal [:the_street, :the_city],
|
||||
location.delegate(:street, :city, to: :@place, prefix: :the, private: true)
|
||||
end
|
||||
|
||||
def test_delegation_arity
|
||||
c = Class.new do
|
||||
delegate :zero, :one, :two, to: ArityTester
|
||||
end
|
||||
assert_equal 0, c.instance_method(:zero).arity
|
||||
assert_equal 1, c.instance_method(:one).arity
|
||||
assert_equal 2, c.instance_method(:two).arity
|
||||
|
||||
d = Class.new(ArityTester) do
|
||||
delegate :zero, :zero_with_block, :one, :one_with_block, :two, :opt,
|
||||
:kwargs, :kwargs_with_block, :opt_kwargs, :opt_kwargs_with_block, to: :class
|
||||
end
|
||||
|
||||
assert_equal 0, d.instance_method(:zero).arity
|
||||
assert_equal 0, d.instance_method(:zero_with_block).arity
|
||||
assert_equal 1, d.instance_method(:one).arity
|
||||
assert_equal 1, d.instance_method(:one_with_block).arity
|
||||
assert_equal 2, d.instance_method(:two).arity
|
||||
assert_equal(-1, d.instance_method(:opt).arity)
|
||||
assert_equal(-1, d.instance_method(:kwargs).arity)
|
||||
assert_equal(-1, d.instance_method(:kwargs_with_block).arity)
|
||||
assert_equal(-1, d.instance_method(:opt_kwargs).arity)
|
||||
assert_equal(-1, d.instance_method(:opt_kwargs_with_block).arity)
|
||||
assert_nothing_raised do
|
||||
d.new.zero
|
||||
d.new.zero_with_block
|
||||
d.new.one(1)
|
||||
d.new.one_with_block(1)
|
||||
d.new.two(1, 2)
|
||||
d.new.opt(1, 2, 3)
|
||||
d.new.kwargs(a: 1, b: 2)
|
||||
d.new.kwargs_with_block(a: 1, b: 2, c: 3)
|
||||
d.new.opt_kwargs(a: 1)
|
||||
d.new.opt_kwargs_with_block(a: 1, b: 2, c: 3)
|
||||
end
|
||||
|
||||
e = Class.new do
|
||||
delegate :zero, to: ArityTesterModule
|
||||
end
|
||||
|
||||
assert_equal 0, e.instance_method(:zero).arity
|
||||
assert_nothing_raised do
|
||||
e.new.zero
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Loading…
Reference in New Issue
Block a user