Skip to content

Commit

Permalink
Allow memoization when included modules prepend MemoWise and defi…
Browse files Browse the repository at this point in the history
…ne `initialize`

Fixes #302

Co-authored-by: alpaca-tc <[email protected]>
  • Loading branch information
JacobEvelyn and alpaca-tc committed Jan 30, 2024
1 parent 7cf8ed2 commit 12ce065
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 27 deletions.
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ _No breaking changes!_

**Project enhancements:**

- Fixed a bug that overwrote existing self.extended method definitions. [[#324]](https://github.com/panorama-ed/memo_wise/pull/314)
- Fixed a bug that overwrote existing self.inherited method definitions. [[#325]](https://github.com/panorama-ed/memo_wise/pull/315)
- Fixed a bug that overwrote existing `self.extended` method definitions [[#324]](https://github.com/panorama-ed/memo_wise/pull/324)
- Fixed a bug that overwrote existing `self.inherited` method definitions [[#325]](https://github.com/panorama-ed/memo_wise/pull/325)
- Allow memoization when `include`d modules `prepend MemoWise` and define `initialize` [[#327]](https://github.com/panorama-ed/memo_wise/pull/327)

## [v1.8.0](https://github.com/panorama-ed/memo_wise/compare/v1.7.0...v1.8.0) - 2023-10-25

Expand All @@ -27,7 +28,7 @@ _No breaking changes!_
**Project enhancements:**

- Switched RuboCop configuration from `panolint` to `panolint-ruby` [[#312]](https://github.com/panorama-ed/memo_wise/pull/312)
- Updated benchmark results in `README.md` to Ruby 3.2.2 and 2.7.8 [[#313]](https://github.com/panorama-ed/memo_wise/pull/297)
- Updated benchmark results in `README.md` to Ruby 3.2.2 and 2.7.8 [[#313]](https://github.com/panorama-ed/memo_wise/pull/313)
- Updated `Dry::Core` gem version to 1.0.0 in benchmarks [[#297]](https://github.com/panorama-ed/memo_wise/pull/297)
- Updated `Memery` gem version to 1.5.0 in benchmarks [[#313]](https://github.com/panorama-ed/memo_wise/pull/313)
- Updated `Memoized` gem version to 1.1.1 in benchmarks [[#288]](https://github.com/panorama-ed/memo_wise/pull/288)
Expand Down
40 changes: 20 additions & 20 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,21 +112,21 @@ For more usage details, see our detailed [documentation](#documentation).

## Benchmarks

Benchmarks are run in GitHub Actions, and the tables below are updated with every code change. **Values >1.00x represent how much _slower_ each gem’s memoized value retrieval is than the latest commit of `MemoWise`**, according to [`benchmark-ips`](https://github.com/evanphx/benchmark-ips) (2.11.0).
Benchmarks are run in GitHub Actions, and the tables below are updated with every code change. **Values >1.00x represent how much _slower_ each gem’s memoized value retrieval is than the latest commit of `MemoWise`**, according to [`benchmark-ips`](https://github.com/evanphx/benchmark-ips) (2.13.0).

Results using Ruby 3.2.2:
Results using Ruby 3.2.3:

|Method arguments|`Dry::Core`\* (1.0.1)|`Memery` (1.5.0)|
|--|--|--|
|`()` (none)|0.60x|3.58x|
|`(a)`|1.37x|7.41x|
|`(a, b)`|1.20x|6.43x|
|`(a:)`|1.47x|13.60x|
|`(a:, b:)`|1.20x|10.55x|
|`(a, b:)`|1.21x|10.36x|
|`(a, *args)`|0.79x|1.52x|
|`(a:, **kwargs)`|0.77x|2.02x|
|`(a, *args, b:, **kwargs)`|0.69x|1.38x|
|`()` (none)|0.59x|3.51x|
|`(a)`|1.50x|9.02x|
|`(a, b)`|1.21x|7.12x|
|`(a:)`|1.47x|14.54x|
|`(a:, b:)`|1.20x|11.01x|
|`(a, b:)`|1.22x|11.15x|
|`(a, *args)`|0.80x|1.67x|
|`(a:, **kwargs)`|0.77x|2.08x|
|`(a, *args, b:, **kwargs)`|0.68x|1.41x|

\* `Dry::Core`
[may cause incorrect behavior caused by hash collisions](https://github.com/dry-rb/dry-core/issues/63).
Expand All @@ -135,15 +135,15 @@ Results using Ruby 2.7.8 (because these gems raise errors in Ruby 3.x):

|Method arguments|`DDMemoize` (1.0.0)|`Memoist` (0.16.2)|`Memoized` (1.1.1)|`Memoizer` (1.0.3)|
|--|--|--|--|--|
|`()` (none)|22.09x|2.35x|23.72x|2.60x|
|`(a)`|20.98x|14.43x|21.20x|12.20x|
|`(a, b)`|17.45x|12.94x|17.69x|11.13x|
|`(a:)`|29.80x|23.38x|25.17x|21.57x|
|`(a:, b:)`|27.00x|22.26x|23.30x|20.91x|
|`(a, b:)`|25.91x|21.20x|21.88x|19.51x|
|`(a, *args)`|3.07x|2.27x|3.17x|1.95x|
|`(a:, **kwargs)`|2.74x|2.28x|2.51x|2.10x|
|`(a, *args, b:, **kwargs)`|2.14x|1.84x|1.95x|1.72x|
|`()` (none)|23.21x|2.30x|24.10x|2.68x|
|`(a)`|21.81x|14.61x|21.37x|12.07x|
|`(a, b)`|18.87x|13.44x|18.00x|11.58x|
|`(a:)`|29.97x|23.28x|25.77x|21.35x|
|`(a:, b:)`|27.37x|22.26x|23.96x|20.63x|
|`(a, b:)`|26.22x|20.88x|22.13x|19.28x|
|`(a, *args)`|3.23x|2.26x|3.20x|1.98x|
|`(a:, **kwargs)`|2.77x|2.27x|2.54x|2.09x|
|`(a, *args, b:, **kwargs)`|2.18x|1.85x|1.97x|1.74x|

You can run benchmarks yourself with:

Expand Down
21 changes: 17 additions & 4 deletions lib/memo_wise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ module MemoWise
# [this article](https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/)
# for more information.
#

# :nocov:
all_args = RUBY_VERSION < "2.7" ? "*" : "..."
# :nocov:
class_eval <<~HEREDOC, __FILE__, __LINE__ + 1
INITIALIZE_LITERAL = <<~HEREDOC.freeze
# On Ruby 2.7 or greater:
#
# def initialize(...)
Expand All @@ -72,11 +71,14 @@ module MemoWise
# super
# end
def initialize(#{all_args})
def initialize(#{RUBY_VERSION < '2.7' ? '*' : '...'})
MemoWise::InternalAPI.create_memo_wise_state!(self)
super
end
HEREDOC
# :nocov:

class_eval(INITIALIZE_LITERAL, __FILE__, __LINE__ + 1)

module CreateMemoWiseStateOnExtended
def extended(base)
Expand All @@ -94,6 +96,15 @@ def inherited(subclass)
end
private_constant(:CreateMemoWiseStateOnInherited)

module CreateMemoWiseStateOnIncluded
def included(base)
base.prepend(Module.new do
class_eval(INITIALIZE_LITERAL, __FILE__, __LINE__ + 1)
end)
end
end
private_constant(:CreateMemoWiseStateOnIncluded)

# @private
#
# Private setup method, called automatically by `prepend MemoWise` in a class.
Expand Down Expand Up @@ -176,6 +187,8 @@ def memo_wise(method_name_or_hash)
if klass.is_a?(Class) && !klass.singleton_class?
klass.singleton_class.prepend(CreateMemoWiseStateOnInherited)
else
klass.singleton_class.prepend(CreateMemoWiseStateOnIncluded) if klass.is_a?(Module) && !klass.singleton_class?

klass.prepend(CreateMemoWiseStateOnInherited)
end

Expand Down
47 changes: 47 additions & 0 deletions spec/memo_wise_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -351,11 +351,32 @@ def module2_method
end
end

let(:klass_with_initializer) do
Class.new do
include Module1
def initialize(*); end
end
end

let(:module_with_initializer) do
Module.new do
include Module1
def initialize(*); end
end
end

let(:klass_with_module_with_initializer) do
Class.new do
include Module3
end
end

let(:instance) { klass.new }

before(:each) do
stub_const("Module1", module1)
stub_const("Module2", module2)
stub_const("Module3", module_with_initializer)
end

it "memoizes inherited methods separately" do
Expand All @@ -364,6 +385,32 @@ def module2_method
expect(Array.new(4) { instance.module2_method }).to all eq("module2_method")
expect(instance.module2_method_counter).to eq(1)
end

it "can memoize klass with initializer" do
instance = klass_with_initializer.new(true)
expect { instance.module1_method }.not_to raise_error

expect(Array.new(4) { instance.module1_method }).to all eq("module1_method")
expect(instance.module1_method_counter).to eq(1)
end

it "can memoize klass with module with initializer" do
instance = klass_with_module_with_initializer.new(true)
expect { instance.module1_method }.not_to raise_error

expect(Array.new(4) { instance.module1_method }).to all eq("module1_method")
expect(instance.module1_method_counter).to eq(1)
end

it "can reset klass with initializer" do
instance = klass_with_initializer.new(true)
expect { instance.reset_memo_wise }.not_to raise_error
end

it "can reset klass with module with initializer" do
instance = klass_with_module_with_initializer.new(true)
expect { instance.reset_memo_wise }.not_to raise_error
end
end

context "when the class, its superclass, and its module all memoize methods" do
Expand Down

0 comments on commit 12ce065

Please sign in to comment.