Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate *args from **kwargs #32

Merged
merged 1 commit into from
Sep 23, 2020
Merged

Separate *args from **kwargs #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
97 changes: 97 additions & 0 deletions benchmarks/benchmarks.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an issue to fill these in?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would we fill these in with? I don't think the result would impact the benchmark since we're only benchmarking cache value returns, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yup you're totally right, PR lgtm. Can merge this if you approve!

Also, looks like I lost permission to push to this repo. Any chance you could check on that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, sorry! Should be fixed now!

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
Expand Down Expand Up @@ -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
62 changes: 53 additions & 9 deletions lib/memo_wise.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
Loading