From: sam.saffron@... Date: 2020-06-25T08:09:03+00:00 Subject: [ruby-core:98936] [Ruby master Feature#16897] General purpose memoizer in Ruby 3 with Ruby 2 performance Issue #16897 has been updated by sam.saffron (Sam Saffron). Understood Jeremy, there are always compromises. ``` def bar(a:); end def foo(*args); bar(*args); end; ``` There was a deliberate decision made to break `foo(a: 1)` here (by default) which has both its upsides and downsides. If we keep `foo(a: 1)` working a developer may get confused (despite everything operating 100% correctly): > Why does `foo(a: 1)` work and `bar(*[{a: 1}])` not work? I still think that wider compatibility is better, teaching the developer about this trick for the huge edge case imo is better than the alternative of dropping support for kwarg tracking in `*args`: ``` h = {a: 1} h.kwargs! bar(*[h]) ``` Advantage of long term kwarg tracking is wider compatibility and we can have 1 entity for all the arguments (a single Array). Disadvantage is that there are "nooks" inside Hash that we need to carry long term and `bar(**h)` works anyway. I stand by it being a trap, it is hard to explain why you can capture one thing but can not delegate it without hacks and traps. I also admit this is not the biggest issue in the world, but it is strange. I guess the problem is that the solution I am proposing (and that has been proposed before) is also strange, so it we are replacing strange with strange. ---------------------------------------- Feature #16897: General purpose memoizer in Ruby 3 with Ruby 2 performance https://bugs.ruby-lang.org/issues/16897#change-86311 * 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: