From: sam.saffron@... Date: 2020-06-23T05:00:09+00:00 Subject: [ruby-core:98919] [Ruby master Feature#16897] General purpose memoizer in Ruby 3 with Ruby 2 performance Issue #16897 has been updated by sam.saffron (Sam Saffron). > In terms of usability, matz seems to like handling *args, **kwargs because it is explicit and not so complex. To me the design we arrived at is very very non-intuitive sadly, @matz ``` def bar(a: 1) end def foo(*x) puts x bar(*x) end ``` `*x` captures both kwargs and args, yet is is not allowed to pass kwargs along. `foo(a: 1)` will print `{a: 1}` and then error out. This is very bad usability in the language. If there is strong insistence on separation we should do so properly. `foo(a: 1)` should throw an exception cause it only captures args an not kwargs. At least that would guide people at the right direction. My preference remains : 1. Best... fix *x so it is able to delegate kwargs properly 100% of the time, `foo(a: 1)` should work, `foo({a: 1})` should exception. This means we codify and make official `{}.kwargs?` or something like that. 2. Second best is introducing ***x and Arguments But the current status quo is a huge trap we are leaving for future Ruby generations. ---------------------------------------- Feature #16897: General purpose memoizer in Ruby 3 with Ruby 2 performance https://bugs.ruby-lang.org/issues/16897#change-86298 * Author: sam.saffron (Sam Saffron) * Status: Open * Priority: Normal ---------------------------------------- ```ruby require 'benchmark/ips' module Memoizer def memoize_26(method_name) cache = {} uncached = "#{method_name}_without_cache" alias_method uncached, method_name define_method(method_name) do |*arguments| found = true data = cache.fetch(arguments) { found = false } unless found cache[arguments] = data = public_send(uncached, *arguments) end data end end def memoize_27(method_name) cache = {} uncached = "#{method_name}_without_cache" alias_method uncached, method_name define_method(method_name) do |*args, **kwargs| found = true all_args = [args, kwargs] data = cache.fetch(all_args) { found = false } unless found cache[all_args] = data = public_send(uncached, *args, **kwargs) end data end end def memoize_27_v2(method_name) uncached = "#{method_name}_without_cache" alias_method uncached, method_name cache = "MEMOIZE_#{method_name}" params = instance_method(method_name).parameters has_kwargs = params.any? {|t, name| "#{t}".start_with? "key"} has_args = params.any? {|t, name| !"#{t}".start_with? "key"} args = [] args << "args" if has_args args << "kwargs" if has_kwargs args_text = args.map do |n| n == "args" ? "*args" : "**kwargs" end.join(",") class_eval <<~RUBY #{cache} = {} def #{method_name}(#{args_text}) found = true all_args = #{args.length === 2 ? "[args, kwargs]" : args[0]} data = #{cache}.fetch(all_args) { found = false } unless found #{cache}[all_args] = data = public_send(:#{uncached} #{args.empty? ? "" : ", #{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 = [ OldMethod.new, NewMethod.new, OptimizedMethod.new ] Benchmark.ips do |x| x.warmup = 1 x.time = 2 methods.each do |m| x.report("#{m.class} args only") do |times| while times > 0 m.args_only(10, b: 10) times -= 1 end end x.report("#{m.class} kwargs only") do |times| while times > 0 m.kwargs_only(a: 10, b: 10) times -= 1 end end x.report("#{m.class} args and kwargs") do |times| while times > 0 m.args_and_kwargs(10, b: 10) times -= 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 (� 0.00) slower # OldMethod args and kwargs: 940911.3 i/s - 1.09x (� 0.00) slower # OldMethod args only: 930446.1 i/s - 1.10x (� 0.00) slower # OldMethod kwargs only: 858238.5 i/s - 1.19x (� 0.00) slower # OptimizedMethod args and kwargs: 773773.5 i/s - 1.32x (� 0.00) slower # NewMethod args and kwargs: 772653.3 i/s - 1.32x (� 0.00) slower # NewMethod args only: 771253.2 i/s - 1.32x (� 0.00) slower # NewMethod kwargs only: 700604.1 i/s - 1.46x (� 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]] += 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] += 1 orig_count(*args, **kwargs) end ``` I would like to propose some changes to Ruby 3 to allow to recover this performance. Perhaps: ```ruby def count(...) args = ... counter[args] += 1 orig_count(...) end ``` Or: ```ruby def count(***args) counter[args] += 1 orig_count(***args) end ``` Thoughts? -- https://bugs.ruby-lang.org/ Unsubscribe: