memo_wise
memo_wise copied to clipboard
Change `prepend MemoWise` to `extend MemoWise`
Co-authored-by: Jemma Issroff [email protected]
What the refactor does:
The only public facing change in this refactor is that instead of prepend MemoWise
, clients will now extend MemoWise
. Internally, this refactor simplifies the machinery necessary to get memoization working, including eliminating back and forth of singleton classes and deferring definitions of preset_memo_wise
and reset_memo_wise
. With this change, MemoWise will also no longer redefine methods, simplifying delegation to the original method and making it easier to determine which methods are memoized.
Guiding principles for the refactor:
- Mixing in
MemoWise
module should be as nonintrusive as possible. By utilizingextend
, we make sure that mixing inMemoWise
only adds thememo_wise
method on the the class, which is the main API of the library. - In the same spirit, currently the library overrides a lot of methods (including
initialize
) and eagerly defines many instance variables on the mixin site, thus unnecessarily polluting the target. - Internal implementation, in the current form, is more complex that it could be. For example, the internal api has to resort to walking the
ObjectSpace
to find attached objects to singleton classes, or the library needs to keep checking if the mixin site is a singleton class or not. - The library is essentially doing an "alias-method-chain" dance which is brittle and makes delegating arguments to the actual method needlessly complex. Instead, prepending a module on the class which defines the same methods allows us to just use
super
to delegate to the actual instance. Moreover, this method of prepended methods plays nicer with other forms of method wrapping that other libraries could be using as well (for example, in its current form MemoWise would not play nicely with Sorbet runtime).
How the refactor works:
Step 1. Call extend MemoWise
from a class/module.
This exposes the method memo_wise
to the class/module as a class method.
Step 2. Call the memo_wise
method on an instance method (for example, memo_wise :example_method
).
With the first memo_wise
method call, a module named MemoWiseMethods
is created. This module has preset_memo_wise
, reset_memo_wise
and freeze
as instance methods. The memo_wise
call also adds a memoized shadow instance method on this module (for example, MemoWiseMethods#example_method
).
Step 3. The MemoWiseMethods
module is automatically prepend
ed on the original class/module.
The use of prepend
means that the shadow methods on this module take precedence over the original methods. Calling super
from within the shadow methods is sufficient for accessing the original methods (instead of doing the "alias-method-chain" dance).
Notes:
- Calling
memo_wise
on class methods makes the singleton class of the class/moduleextend MemoWise
, and callsmemo_wise
for the instance methods on the singleton class. Accordingly,preset_memo_wise
andreset_memo_wise
are only defined as class methods the first timememo_wise
is called on a class method (withself:
). - Frozen objects are supported by shadowing the
freeze
method and initializing internal state before callingsuper
.
Before merging:
- [x] Copy the table printed at the end of the latest benchmark results into the
README.md
and update this PR - [ ] If this change merits an update to
CHANGELOG.md
, add an entry following Keep a Changelog guidelines with semantic versioning
Codecov Report
Merging #253 (cb34c22) into main (8209474) will not change coverage. The diff coverage is
100.00%
.
@@ Coverage Diff @@
## main #253 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 3 +1
Lines 166 144 -22
Branches 83 67 -16
=========================================
- Hits 166 144 -22
Impacted Files | Coverage Δ | |
---|---|---|
lib/memo_wise.rb | 100.00% <100.00%> (ø) |
|
lib/memo_wise/internal_api.rb | 100.00% <100.00%> (ø) |
|
lib/memo_wise/module_builder.rb | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 8209474...cb34c22. Read the comment docs.
So what's up with this here PR? I'm looking to get working with Sorbet in my app and it's straight incompatible with prepend
, which is a bummer. The branch has no problem with my tests, but if y'all abandoned this effort for whatever reason I'd rather not depend on it, you know?
Hi @dhnaranjo @halo! Sorry I missed the question above! We had paused this work when we found it wasn't as performant in our benchmarks and we were unable to figure out why. I'm in the process of upgrading our benchmarks to Ruby 3.2 and will take a look at how this branch performs under 3.2 once that's done!
While I don't use Sorbet, I do want this gem to be usable by folks who do.