Merge pull request #32 from panorama-ed/silencing-args-warning

Separate *args from **kwargs

README.md

@@ -27,13 +27,15 @@ TODO: Write usage instructions here
 Memoized value retrieval time using Ruby 2.7.1 and
 [`benchmark-ips`](https://github.com/evanphx/benchmark-ips) 2.8.2:
 
-|Gem|Method with no arguments|Method with positional arguments|Method with keyword arguments|
-|---|------------------------|--------------------------------|-----------------------------|
-|**`memo_wise` (0.1.0)**|**baseline**|**baseline**|**baseline**|
-|`memery` (1.3.0)|11.80x (± 0.00) slower|2.00x (± 0.00) slower|1.93x (± 0.00) slower|
-|`memoist` (0.16.2)|2.55x (± 0.00) slower|2.20x (± 0.00) slower|2.01x (± 0.00) slower|
-|`memoized` (1.0.2)|1.19x  (± 0.00) slower|1.81x (± 0.00) slower|1.81x (± 0.00) slower|
-|`memoizer` (1.0.3)|2.93x (± 0.00) slower|1.97x (± 0.00) slower|1.90x (± 0.00) slower|
+|Method arguments|**`memo_wise` (0.1.0)**|`memery` (1.3.0)|`memoist` (0.16.2)|`memoized` (1.0.2)|`memoizer` (1.0.3)|
+|--|--|--|--|--|--|
+|`()` (none)|**baseline**|12.11x slower|2.49x slower|1.22x slower|3.21x slower|
+|`(a, b)`|**baseline**|2.00x slower|2.28x slower|1.84x slower|1.99x slower|
+|`(a:, b:)`|**baseline**|2.19x slower|2.36x slower|2.07x slower|2.17x slower|
+|`(a, b:)`|**baseline**|1.56x slower|1.70x slower|1.46x slower|1.54x slower|
+|`(a, *args)`|**baseline**|2.01x slower|2.34x slower|1.98x slower|2.01x slower|
+|`(a:, **kwargs)`|**baseline**|1.94x slower|2.11x slower|1.90x slower|1.92x slower|
+|`(a, *args, b:, **kwargs)`|**baseline**|1.94x slower|2.15x slower|1.92x slower|1.92x slower|
 
 You can run benchmarks yourself with:
 

benchmarks/benchmarks.rb

@@ -70,6 +70,26 @@ def keyword_args(a:, b:)
         100
       end
       #{benchmark_gem.memoization_method} :keyword_args
+
+      def positional_and_keyword_args(a, b:)
+        100
+      end
+      #{benchmark_gem.memoization_method} :positional_and_keyword_args
+
+      def positional_and_splat_args(a, *args)
+        100
+      end
+      #{benchmark_gem.memoization_method} :positional_and_splat_args
+
+      def keyword_and_double_splat_args(a:, **kwargs)
+        100
+      end
+      #{benchmark_gem.memoization_method} :keyword_and_double_splat_args
+
+      def positional_splat_keyword_and_double_splat_args(a, *args, b:, **kwargs)
+        100
+      end
+      #{benchmark_gem.memoization_method} :positional_splat_keyword_and_double_splat_args
     end
   CLASS
 end
@@ -130,3 +150,80 @@ def keyword_args(a:, b:)
 
   x.compare!
 end
+
+Benchmark.ips do |x|
+  x.config(suite: suite)
+  BENCHMARK_GEMS.each do |benchmark_gem|
+    instance = Object.const_get("#{benchmark_gem.klass}Example").new
+
+    # Run once with each set of arguments to memoize the result values, so our
+    # benchmark only tests memoized retrieval time.
+    ARGUMENTS.each { |a, b| instance.positional_and_keyword_args(a, b: b) }
+
+    x.report("#{benchmark_gem.benchmark_name}: positional_and_keyword_args") do
+      ARGUMENTS.each { |a, b| instance.positional_and_keyword_args(a, b: b) }
+    end
+  end
+
+  x.compare!
+end
+
+Benchmark.ips do |x|
+  x.config(suite: suite)
+  BENCHMARK_GEMS.each do |benchmark_gem|
+    instance = Object.const_get("#{benchmark_gem.klass}Example").new
+
+    # Run once with each set of arguments to memoize the result values, so our
+    # benchmark only tests memoized retrieval time.
+    ARGUMENTS.each { |a, b| instance.positional_and_splat_args(a, b) }
+
+    x.report("#{benchmark_gem.benchmark_name}: positional_and_splat_args") do
+      ARGUMENTS.each { |a, b| instance.positional_and_splat_args(a, b) }
+    end
+  end
+
+  x.compare!
+end
+
+Benchmark.ips do |x|
+  x.config(suite: suite)
+  BENCHMARK_GEMS.each do |benchmark_gem|
+    instance = Object.const_get("#{benchmark_gem.klass}Example").new
+
+    # Run once with each set of arguments to memoize the result values, so our
+    # benchmark only tests memoized retrieval time.
+    ARGUMENTS.each { |a, b| instance.keyword_and_double_splat_args(a: a, b: b) }
+
+    x.report(
+      "#{benchmark_gem.benchmark_name}: keyword_and_double_splat_args"
+    ) do
+      ARGUMENTS.each { |a, b| instance.positional_and_splat_args(a: a, b: b) }
+    end
+  end
+
+  x.compare!
+end
+
+Benchmark.ips do |x|
+  x.config(suite: suite)
+  BENCHMARK_GEMS.each do |benchmark_gem|
+    instance = Object.const_get("#{benchmark_gem.klass}Example").new
+
+    # Run once with each set of arguments to memoize the result values, so our
+    # benchmark only tests memoized retrieval time.
+    ARGUMENTS.each do |a, b|
+      instance.positional_splat_keyword_and_double_splat_args(a, b, a: a, b: b)
+    end
+
+    x.report(
+      "#{benchmark_gem.benchmark_name}: "\
+        "positional_splat_keyword_and_double_splat_args"
+    ) do
+      ARGUMENTS.each do |a, b|
+        instance.positional_and_splat_args(a, b, a: a, b: b)
+      end
+    end
+  end
+
+  x.compare!
+end

lib/memo_wise.rb

@@ -8,13 +8,25 @@ def initialize(*values)
     super
   end
 
-  def self.prepended(target)
+  def self.has_arg?(method) # rubocop:disable Naming/PredicateName
+    method.parameters.any? do |(param, _)|
+      param == :req || param == :opt || param == :rest # rubocop:disable Style/MultipleComparison
+    end
+  end
+
+  def self.has_kwarg?(method) # rubocop:disable Naming/PredicateName
+    method.parameters.any? do |(param, _)|
+      param == :keyreq || param == :key || param == :keyrest # rubocop:disable Style/MultipleComparison
+    end
+  end
+
+  def self.prepended(target) # rubocop:disable Metrics/PerceivedComplexity
     class << target
       # Implements memoization for the given method name.
       #
       # @param method_name [Symbol]
       #   Name of method for which to implement memoization.
-      def memo_wise(method_name)
+      def memo_wise(method_name) # rubocop:disable Metrics/PerceivedComplexity
         unless method_name.is_a?(Symbol)
           raise ArgumentError,
                 "#{method_name.inspect} must be a Symbol"
@@ -32,7 +44,11 @@ def memo_wise(method_name)
         alias_method not_memoized_name, method_name
         private not_memoized_name
 
-        if instance_method(method_name).arity.zero?
+        method = instance_method(method_name)
+
+        # Zero-arg methods can use simpler/more performant logic because the
+        # hash key is just the method name.
+        if method.arity.zero?
           module_eval <<-END_OF_METHOD, __FILE__, __LINE__ + 1
             def #{method_name}
               @_memo_wise.fetch(:#{method_name}) do
@@ -41,13 +57,31 @@ def #{method_name}
             end
           END_OF_METHOD
         else
+          # If our method has arguments, we need to separate out our handling of
+          # normal args vs. keyword args due to the changes in Ruby 3.
+          # See: <link>
+          # By only including logic for *args or **kwargs when they are used in
+          # the method, we can avoid allocating unnecessary arrays and hashes.
+          has_arg = MemoWise.has_arg?(method)
+
+          if has_arg && MemoWise.has_kwarg?(method)
+            args_str = "(*args, **kwargs)"
+            fetch_key = "[args, kwargs].freeze"
+          elsif has_arg
+            args_str = "(*args)"
+            fetch_key = "args"
+          else
+            args_str = "(**kwargs)"
+            fetch_key = "kwargs"
+          end
+
           # Note that we don't need to freeze args before using it as a hash key
           # because Ruby always copies argument arrays when splatted.
           module_eval <<-END_OF_METHOD, __FILE__, __LINE__ + 1
-            def #{method_name}(*args)
+            def #{method_name}#{args_str}
               hash = @_memo_wise[:#{method_name}]
-              hash.fetch(args) do
-                hash[args] = #{not_memoized_name}(*args)
+              hash.fetch(#{fetch_key}) do
+                hash[#{fetch_key}] = #{not_memoized_name}#{args_str}
               end
             end
           END_OF_METHOD
@@ -60,7 +94,7 @@ def #{method_name}(*args)
     end
   end
 
-  def reset_memo_wise(method_name, *args)
+  def reset_memo_wise(method_name, *args, **kwargs)
     unless method_name.is_a?(Symbol)
       raise ArgumentError, "#{method_name.inspect} must be a Symbol"
     end
@@ -69,10 +103,20 @@ def reset_memo_wise(method_name, *args)
       raise ArgumentError, "#{method_name} is not a defined method"
     end
 
-    if args.empty?
+    if args.empty? && kwargs.empty?
       @_memo_wise.delete(method_name)
     else
-      @_memo_wise[method_name].delete(args)
+      method = self.class.instance_method(method_name)
+
+      has_arg = MemoWise.has_arg?(method)
+
+      if has_arg && MemoWise.has_kwarg?(method)
+        @_memo_wise[method_name].delete([args, kwargs])
+      elsif has_arg
+        @_memo_wise[method_name].delete(args)
+      else
+        @_memo_wise[method_name].delete(kwargs)
+      end
     end
   end