Issue #16897 has been updated by sam.saffron (Sam Saffron).
@Eregon: to summarize the one point of performance that I want to address h=
ere
### Memoizing a method that has both kwargs and args:
1. Using the fastest pattern available on Ruby 2.7.1 **REQUIRING** usage of=
`ruby2_keywords`
OldMethod args and kwargs: 944008.6 i/s
2. Using a standard pattern on 2.7.1, simply adding `**kwargs`
NewMethod args and kwargs: 766935.9 i/s
3. Using a heavily optimized and verbose approach:
OptimizedMethod args and kwargs: 771978.2 i/s
--- =
I tried this which is a hybrid of your and Samuels suggestion: =
```
module Memoizer
def self.KEY(*args, **kwargs)
[args, kwargs]
end
=
def memoize(method_name)
cache =3D "MEMOIZE2_#{method_name}"
uncached =3D "#{method_name}_without_cache"
alias_method uncached, method_name
class_eval <<~RUBY
#{cache} =3D {}
def #{method_name}(...)
found =3D true
args =3D Memoizer.KEY(...)
data =3D #{cache}.fetch(args) { found =3D false }
unless found
#{cache}[args] =3D data =3D #{uncached}(...)
end
data
end
RUBY
end
end
```
Sadly it appears to be slowest: 696435.9 i/s
I can not seem to dispatch `...` directly into fetch for arbitrary argument=
s:
```
class Foo
HASH =3D {}
def bar(...)
HASH.fetch(...) { rand }
end
end
foo =3D Foo.new
puts foo.bar(1)
/home/sam/Source/performance/memoize/memoize.rb:8: both block arg and actua=
l block given
```
Memoizer needs fetch cause you may be memoizing nil.
@jeremyevans0 would though argue that this is the only correct generic memo=
izer, but as implemented on 2.7.1 `...` is implemented in a buggy way:
```
class Foo
def key(*args, **kwargs)
{args: args, kwargs: kwargs}
end
def test(...)
key(...)
end
end
puts Foo.new.test({a: 1})
/home/sam/Source/performance/memoize/memoize.rb:11: warning: Using the last=
argument as keyword parameters is deprecated; maybe ** should be added to =
the call
/home/sam/Source/performance/memoize/memoize.rb:6: warning: The called meth=
od `key' is defined here
{:args=3D>[], :kwargs=3D>{:a=3D>1}}
puts Foo.new.test(a: 1)
{:args=3D>[], :kwargs=3D>{:a=3D>1}}
I am not following what the reticence here is for introducing an `Arguments=
` proper object, it solves this performance very cleanly. Plus it lets as d=
ispatch around a list of arguments cleanly maintaining args / kwargs separa=
tion without introducing a new object to communicate this concept. =
```
def foo(a =3D 1, b: 2)
puts "a: #{a} b: #{b}"
end
def delay(...x)
if @delayed
foo(...@delayed)
@delayed =3D x
else
@delayed =3D x
end
end
delay({b: 7})
# prints nothing
delay(9999)
"a: {b: 7} b: 2"
=
```
cc @ko1 , @mame
----------------------------------------
Feature #16897: Can a Ruby 3.0 compatible general purpose memoizer be writt=
en in such a way that it matches Ruby 2 performance?
https://bugs.ruby-lang.org/issues/16897#change-85971
* Author: sam.saffron (Sam Saffron)
* Status: Open
* Priority: Normal
----------------------------------------
```ruby
require 'benchmark/ips'
module Memoizer
def memoize_26(method_name)
cache =3D {}
uncached =3D "#{method_name}_without_cache"
alias_method uncached, method_name
define_method(method_name) do |*arguments|
found =3D true
data =3D cache.fetch(arguments) { found =3D false }
unless found
cache[arguments] =3D data =3D public_send(uncached, *arguments)
end
data
end
end
def memoize_27(method_name)
cache =3D {}
uncached =3D "#{method_name}_without_cache"
alias_method uncached, method_name
define_method(method_name) do |*args, **kwargs|
found =3D true
all_args =3D [args, kwargs]
data =3D cache.fetch(all_args) { found =3D false }
unless found
cache[all_args] =3D data =3D public_send(uncached, *args, **kwargs)
end
data
end
end
def memoize_27_v2(method_name)
uncached =3D "#{method_name}_without_cache"
alias_method uncached, method_name
cache =3D "MEMOIZE_#{method_name}"
params =3D instance_method(method_name).parameters
has_kwargs =3D params.any? {|t, name| "#{t}".start_with? "key"}
has_args =3D params.any? {|t, name| !"#{t}".start_with? "key"}
args =3D []
args << "args" if has_args
args << "kwargs" if has_kwargs
args_text =3D args.map do |n|
n =3D=3D "args" ? "*args" : "**kwargs"
end.join(",")
class_eval <<~RUBY
#{cache} =3D {}
def #{method_name}(#{args_text})
found =3D true
all_args =3D #{args.length =3D=3D=3D 2 ? "[args, kwargs]" : args[0]}
data =3D #{cache}.fetch(all_args) { found =3D false }
unless found
#{cache}[all_args] =3D data =3D public_send(:#{uncached} #{args.e=
mpty? ? "" : ", #{args_text}"})
end
data
end
RUBY
end
end
module Methods
def args_only(a, b)
sleep 0.1
"#{a} #{b}"
end
def kwargs_only(a:, b: nil)
sleep 0.1
"#{a} #{b}"
end
def args_and_kwargs(a, b:)
sleep 0.1
"#{a} #{b}"
end
end
class OldMethod
extend Memoizer
include Methods
memoize_26 :args_and_kwargs
memoize_26 :args_only
memoize_26 :kwargs_only
end
class NewMethod
extend Memoizer
include Methods
memoize_27 :args_and_kwargs
memoize_27 :args_only
memoize_27 :kwargs_only
end
class OptimizedMethod
extend Memoizer
include Methods
memoize_27_v2 :args_and_kwargs
memoize_27_v2 :args_only
memoize_27_v2 :kwargs_only
end
OptimizedMethod.new.args_only(1,2)
methods =3D [
OldMethod.new,
NewMethod.new,
OptimizedMethod.new
]
Benchmark.ips do |x|
x.warmup =3D 1
x.time =3D 2
methods.each do |m|
x.report("#{m.class} args only") do |times|
while times > 0
m.args_only(10, b: 10)
times -=3D 1
end
end
x.report("#{m.class} kwargs only") do |times|
while times > 0
m.kwargs_only(a: 10, b: 10)
times -=3D 1
end
end
x.report("#{m.class} args and kwargs") do |times|
while times > 0
m.args_and_kwargs(10, b: 10)
times -=3D 1
end
end
end
x.compare!
end
# # Ruby 2.6.5
# #
# OptimizedMethod args only: 974266.9 i/s
# OldMethod args only: 949344.9 i/s - 1.03x slower
# OldMethod args and kwargs: 945951.5 i/s - 1.03x slower
# OptimizedMethod kwargs only: 939160.2 i/s - 1.04x slower
# OldMethod kwargs only: 868229.3 i/s - 1.12x slower
# OptimizedMethod args and kwargs: 751797.0 i/s - 1.30x slower
# NewMethod args only: 730594.4 i/s - 1.33x slower
# NewMethod args and kwargs: 727300.5 i/s - 1.34x slower
# NewMethod kwargs only: 665003.8 i/s - 1.47x slower
#
# #
# # Ruby 2.7.1
#
# OptimizedMethod kwargs only: 1021707.6 i/s
# OptimizedMethod args only: 955694.6 i/s - 1.07x (=B1 0.00) slower
# OldMethod args and kwargs: 940911.3 i/s - 1.09x (=B1 0.00) slower
# OldMethod args only: 930446.1 i/s - 1.10x (=B1 0.00) slower
# OldMethod kwargs only: 858238.5 i/s - 1.19x (=B1 0.00) slower
# OptimizedMethod args and kwargs: 773773.5 i/s - 1.32x (=B1 0.00) slower
# NewMethod args and kwargs: 772653.3 i/s - 1.32x (=B1 0.00) slower
# NewMethod args only: 771253.2 i/s - 1.32x (=B1 0.00) slower
# NewMethod kwargs only: 700604.1 i/s - 1.46x (=B1 0.00) slower
```
The bottom line is that a generic delegator often needs to make use of all =
the arguments provided to a method.
```ruby
def count(*args, **kwargs)
counter[[args, kwargs]] +=3D 1
orig_count(*args, **kwargs)
end
```
The old pattern meant we could get away with one less array allocation per:
```ruby
def count(*args)
counter[args] +=3D 1
orig_count(*args, **kwargs)
end
```
I would like to propose some changes to Ruby 3 to allow to recover this per=
formance. =
Perhaps:
```ruby
def count(...)
args =3D ...
counter[args] +=3D 1
orig_count(...)
end
```
Or:
```ruby
def count(***args)
counter[args] +=3D 1
orig_count(***args)
end
```
Thoughts? =
-- =
https://bugs.ruby-lang.org/
Unsubscribe: <mailto:ruby-core-request / ruby-lang.org?subject=3Dunsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>