Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

For ActiveRecord models there should be a way to avoid calling memoized methods when you call #destroy

#1527

When you call #destroy on an AR model, it calls #freeze. The ActiveSupport::Memoizable mixin, however, alias_method_chains freeze to add a call to #memoize_all. This is probably not what you want on destroy (or maybe on freeze at all), since memoized methods are often computationally expensive.

If this is really the desired behavior, then there should be a way around it (there is currently not a good one). Either a :freeze => false option could be passed to destroy, or a :on_freeze => false could be passed to #memoize.

Reported by Mike Grafton · December 8th, 2008 @ 03:08 AM

State: stale
Milestone: 3.x
Assigned to: nobody
Importance: none

Activity

  1. josh
    josh
    • State changed from new to open

    I think it makes sense that destroy would freeze an AR object because you should not able to make any changes after.

    And one of the main reasons for freeze to invoke your memoized methods is to make sure the ivar is set since you can't modify them after. Otherwise you'd get an error when you called a memorized method on a frozen object.

    So both have there reasons, however combining leads to the issue that you explained.

    The temporary fixed would be to over the freeze_with_memoizable method.

    
def freeze_with_memoizable
  freeze_without_memoizable
end

    As mentioned a possible API to exclude a method from would be "memoize :foo, :freeze => false" or something like that. But there are some other issues we still have to consider like does this apply to other methods on the object and what happens when you call that method again.

    Heres my proposal: Create 2 separate cases for what will happen to each method on freeze.

    1) Current behavior: On freeze, we prime the cache so the method will be permanently remembered and would work under the frozen context. Default setting.

    
memoize :foo, :on_freeze => :prime
# or just
memoize :foo

    2) Alternate behavior: On freeze, discard the memoization feature. This way the method will still work (unmemoized) while frozen. I think the easiest way to accomplish this would be to alias the original method back the its original name.

    
memoize :foo, :on_freeze => :flush

    Take a stab at it, and I'll help you out if you need some.

    December 8th, 2008 @ 03:49 AM

  2. Mike Grafton
    Mike Grafton

    Cool, thanks.

    I actually think it might be better to throw an error if you called an un-primed memoized method after freeze. Especially if it blew up in an obvious way, with an explicit message like "This memoized method cannot be called after freeze. Use :on_freeze => :prime"

    But I admit, I don't really understand freezing well enough to have a strong opinion. For my case your solution would work, so I'll give it a shot.

    December 8th, 2008 @ 01:54 PM

  3. josh
    josh
    • State changed from open to incomplete

    December 28th, 2008 @ 08:55 PM

  4. Andreas Neuhaus
    Andreas Neuhaus

    The above workaround did not work for me (freeze was still calling memoize_all). However, the following worked for me:

    def freeze freeze_without_memoization end

    January 4th, 2009 @ 11:51 AM

  5. ransom-briggs (at uiowa)
    ransom-briggs (at uiowa)

    +1 for an :on_freeze => :flush

    July 9th, 2009 @ 04:00 PM

  6. josh
    josh
    • Assigned user cleared.

    August 19th, 2009 @ 03:55 PM

  7. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 3.x

    May 4th, 2010 @ 06:48 PM

  8. jerrett
    jerrett
    • Importance changed from to

    I like the :flush idea as well, +1.

    November 4th, 2010 @ 06:23 PM

  9. Evgeniy Dolzhenko
    Evgeniy Dolzhenko

    If "one of the main reasons for freeze to invoke your memoized methods is to make sure the ivar is set since you can't modify them after" why don't we just create the required ivars on freeze without actually priming the cache (and calling memoized methods in turn) then?

    January 10th, 2011 @ 01:35 PM

  10. dspencer
    dspencer

    I'm not sure I'm going about this in the right way; but I made a patch that saves a Set of methods to be primed on freeze; and only primes those.

    I can't seem to attach a patch; pasted it to this gist instead.
    https://gist.github.com/773768

    January 11th, 2011 @ 12:16 AM

  11. Evgeniy Dolzhenko
    Evgeniy Dolzhenko

    Really the "prime on freeze" idea makes little sense to me especially because memoization is arguments based so you'll be able to prime cache only for the methods which don't take any arguments.
    So I can't see the need in these additional options to memoize, just create the required (empty) ivars on freeze which will be filled later when required (turns out freeze is not "deep" in ruby, i.e. you can't add instance variables on the frozen object, but can change them freely)

    January 11th, 2011 @ 07:56 AM

  12. rails
    rails
    • State changed from incomplete to open

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

    April 12th, 2011 @ 01:00 AM

  13. rails
    rails
    • State changed from open to stale

    April 12th, 2011 @ 01:00 AM