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.

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

Use method-specific instance variables for methods that take arguments #243

Open
JacobEvelyn opened this issue Dec 7, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@JacobEvelyn
Copy link
Member

This was a suggestion made by @sampersand in the RubyConf Discord channel for our recent talk. The idea is that instead of using @_memo_wise for all method types, we'd use a different instance variable for each method, like @_memo_wise_method_#{method_name}. This should save us an array lookup for a slight performance boost.

There are a few challenges:

  • we'd need to avoid name collisions, for example when memoizing both a data? and a data! and a data method
    • probably a simple .sub("?", "__qmark__").sub("!", "__bang__") would be sufficient
    • @jemmaissroff may have insight into whether there's a max instance variable length to be worried about (or a length at which performance decreases)
      • if so, we could instead use a scheme like @_memo_wise_method_#{counter} or use UUIDs, etc.
  • to support frozen objects we need to ensure that these instance variables are initialized to empty hashes before freezing
    • this could probably be done either in an overridden initialize or freeze method
  • this approach will not support resetting and presetting zero-arity methods on frozen objects
    • a simple path forward would be to continue using our array for zero-arity methods
    • are there good alternatives?

Would love discussion on this idea and its tradeoffs!

@JacobEvelyn JacobEvelyn added the enhancement New feature or request label Dec 7, 2021
@ms-ati
Copy link
Contributor

ms-ati commented Dec 7, 2021

I like retaining the array for zero-arity methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants