From 12ce0650239e1986e8237c7f2f062f77803a293d Mon Sep 17 00:00:00 2001 From: Jacob Evelyn Date: Tue, 30 Jan 2024 09:58:06 -0500 Subject: [PATCH] Allow memoization when `include`d modules `prepend MemoWise` and define `initialize` Fixes #302 Co-authored-by: alpaca-tc --- CHANGELOG.md | 7 ++++--- README.md | 40 +++++++++++++++++------------------ lib/memo_wise.rb | 21 +++++++++++++++---- spec/memo_wise_spec.rb | 47 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 100a5a7..2d821f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -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) diff --git a/README.md b/README.md index 70e86bd..a235ee7 100644 --- a/README.md +++ b/README.md @@ -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). @@ -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: diff --git a/lib/memo_wise.rb b/lib/memo_wise.rb index dace3d6..e793603 100644 --- a/lib/memo_wise.rb +++ b/lib/memo_wise.rb @@ -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(...) @@ -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) @@ -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. @@ -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 diff --git a/spec/memo_wise_spec.rb b/spec/memo_wise_spec.rb index 2a3f4b9..6e73d3a 100644 --- a/spec/memo_wise_spec.rb +++ b/spec/memo_wise_spec.rb @@ -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 @@ -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