Add a functools.cache variant for methods to avoid keeping instances alive

[stevendaprano]

functools.cache, when applied to a method, caches the self argument. This can also keep the instance alive long after it is no longer needed. See for example this discussion.

Following the same API as singledispatch and singledispatchmethod, we should consider adding a cachemethod decorator that either avoids caching self, or uses a weakref for it.

[sobolevn]

I would like to work on it, if no one else has started yet :)
Otherwise, please ping me for a review 👀

[rhettinger]

See: https://stackoverflow.com/a/68052994/424499

If something like this does get added, it should come with clear guidance on when it would be beneficial. There is a narrow set of circumstances where this would be the best solution as compared to @lru_cache.

I would like to work on it, if no one else has started yet :)

The Implementation is the easy part. The OP hasn't even decided whether he wants self to be skipped or to be proxied. Really, there needs to be a review of use cases before jumping to a PR. Also, the interaction with other decorators (property cachemethod and staticmethod) should be thought out in advance.

[kenahoo]

There is a narrow set of circumstances where this would be the best solution as compared to @lru_cache.

I would basically never counsel someone to use the existing @lru_cache on an instance method. That's the one I would call narrow-purposed in instance-method land.

The OP hasn't even decided whether he wants self to be skipped or to be proxied.

Assuming I am the OP, I'm not sure why you're saying that. I feel like the desired behavior is clear from the original post. I'm not sure exactly what you mean by those terms in this context, though.

If "skipped" means a global-level cache shared by all instances of the class, keyed only on the rest of the arguments but excluding self, I definitely would not want that. I'm not sure what "proxied" here would mean.

FWIW I think an implementation that most closely resembles the following self-internal-cache implementation might be a good choice initially:

class Baz:
    def meth(self, key):
        return key + key

    def wrapped_meth(self, key):
        if not hasattr(self, '_cache'):
            setattr(self, '_cache', {})
        if key not in self._cache:
            self._cache[key] = self.meth(key)
        return self._cache[key]

@sobolevn An implementation (or multiple, if different approaches are considered) would be most welcome for discussion purposes IMO. Some of the things that need to be thought out are what to call the decorator, whether there should be parallel variations like lru_cache and cache (maybe cache_method and lru_cache_method? Not sure.), etc.

I think it might also be worth discussing whether the existing lru_cache should sense whether it's being used to wrap an instance method, and whether it should

• "do the right thing" (in my mind there's only one right thing [because my feeling is that people have confused the current implementation for a "contract"], but I'm aware that backward-compatibility concerns might dominate) for the method case;
• warn the user that it's probably doing something they don't want
• continue to have the current behavior and not warn.

[rhettinger]

I'm going to close this for the time being. Shortly (not right now), I will kick off a new discussion with an outline of the perceived problem, various use cases, and what things can or should be done about it. I've been thinking about this for months and have been putting together an analysis.

Note the python-help post was not useful. The example was for something that didn't use self at all and could have easily been cached as a staticmethod. Also, it focused on @cache which is explicitly documented to be unbounded. The tone was also presumptuous, aggressive, and mildly alarmist.

Note, there is no rush here — we can take the time to deliberate. The lru_cache has been about for 13 years. No new problem has suddenly arisen.

I would basically never counsel someone to use the existing @lru_cache on an instance method.

The are plenty of valid uses in the wild. AFAICT, the only downside is needing to wait for an entry to age of the cache rather than having it immediately cleared where the instance is no longer needed. That mostly only matters when the instances are very large or that have a __del__ method. But at least the user has control over the maximum number of entries that get kept.

Note that adding weakrefs also has its problems. There is a speed penalty. Noot all instances are weak referenceable, and many users don't fully understand the dance they would have to do to add weak reference support. Also, if there are two instance that are equal but not identical, you lose the benefit of caching and force a potentially expensive recomputation.

Assuming I am the OP,

No, that is @stevendaprano, the one who opened the issue and said, "we should consider adding a cachemethod decorator that either avoids caching self, or uses a weakref for it".

warn the user that it's probably doing something they don't want

IIRC there is already a lint tool that does this. That said, the tool has no ability to differentiate a legitimate use case from a problematic case.

[stevendaprano]

Thank you Raymond for looking into this. I'm not a heavy user of the lru_cache and I completely forgot it existed! I see that you have closed this issue, but you also mention taking time to deliberate. I take it you consider that there is something to deliberate then.

[rhettinger]

I take it you consider that there is something to deliberate then.

Yes, absolutely. At a minimum, there needs to be a recommended best practice for common cases.

[rhettinger]

Right now, we have the FAQ entry: How do I cache method calls?

Already in progress was an update to show how to stack @cache and @lru_cache with @staticmethod and @classmethod:

@staticmethod
@cache
def sm(x, y): ...

@classmethod
@cache
def cm(cls, x, y): ...

We could add entries to show how to do per instance caching and weakref caching. If we do, then we should list out the advantages and disadvantages for the two different ways of freeing objects immediately rather than waiting for them to age out of a cache or for the cache to be cleared:

If anyone is interested, here's the code that I've been experimented with to evaluate the various approaches:

from dataclasses import dataclass
from weakref import WeakKeyDictionary, ref
from functools import wraps, cache, lru_cache

def PIPM_MVC(func):
    """Per instance, per method, multiple variable cache

    Advantages:
    * Lifetime of cache exactly matches the lifetime of an instance.
    * Does not keep an instance alive.
    * Works on non-hashable instances

    Disadvantages:    
    * High overhead for space and time
    * No ability to manage total memory consumption
    * Hard to find and clear all the caches
    * No hit/miss instrumentation
    * Loses caching of EBNI instances.
    * Does not and cache detect changes to instance attributes

    When to use:
    * You want to cache ALL living instances, not just recently used
    * The underlying method is slower than cost of the cache wrapper
    * When instances are large relative to the size of the cache entries.
    * When you don't need or want cache hits for with equal but distinct instances.

    """
    cachename = f'_cached_{func.__qualname__}_'

    @wraps(func)
    def wrapper(self, *args):
        try:
            cachedict = getattr(self, cachename)
        except AttributeError:
            cachedict = {}
            setattr(self, cachename, cachedict)
        try:
            return cachedict[args]
        except KeyError:
            pass
        ans = func(self, *args)
        cachedict[args] = ans
        return ans

    return wrapper

def weak_lru(maxsize=128, typed=False):
    'LRU Cache decorator that keeps a weak reference to "self"'
    def wrapper(func):

        @lru_cache(maxsize, typed)
        def _func(_self, *args, **kwargs):
            return func(_self(), *args, **kwargs)

        @wraps(func)
        def inner(self, *args, **kwargs):
            return _func(ref(self), *args, **kwargs)

        return inner

    return wrapper

@dataclass(unsafe_hash=True)
class A:

    x: int
    
    def __del__(self):
        print(f'Died: A({self.x!r})  {hex(id(self))=!r}')

    @PIPM_MVC
    def m1(self, y):
        ans = self.x + y
        print(f'm1({y!r}) adds {self.x!r} giving {ans!r}')
        return ans   

    @lru_cache
    def m2(self, y):
        ans = self.x * y
        print(f'm2({y!r}) multiplies {self.x!r} giving {ans!r}')
        return ans

    @weak_lru()
    def m3(self, y):
        ans = self.x ** y
        print(f'm2({y!r}) raises to {self.x!r} giving {ans!r}')
        return ans

[CAM-Gerlach]

From a core workflow perspective, per discussion of this issue with the other core devs on the Discord there was a unanimous consensus that the issue should be left open if the relevant core dev has acknowledged it as valid enough to be worth further exploration and discussion (as indeed has been the case here). Therefore, re-opening accordingly. (To be clear, this is not expressing any particular opinion with regard to the merits of this proposal's content, just ensuring the status is set appropriately per the statements of those who are.)

Also just a quick tip—if the issue is in fact invalid, wontfix, duplicate, etc., it can (and should) be closed as "not planned" rather than closed as "completed". You can click the drop down arrow next to the close button to change the close reason.

[kenahoo]

Thanks @CAM-Gerlach , I appreciate the followup.

[aaronkurz]

Are there any updates on this? I think even if there are no updates, the documentation could/should be more explicit in stating implications for use with instance methods.