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

Memoizing cache decorator with cache lease. #182

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grantjenks
Copy link
Owner

def memoize_lease(cache, expire, lease, name=None, typed=False, tag=None):

The expire argument is a "hard deadline" for evicting cache entries. Set
expire to None to avoid evictions due to expiration.

The lease represents a "soft deadline" for the memoized cache entry. Once
the lease time has passed, cache entries will be updated asynchronously
using a background thread. At most one background thread will be started
for each cache entry. While the background thread is executing, memoized
cache entries will continue to be treated as "cache hits" until expiration.

If name is set to None (default), the callable name will be determined
automatically.

If typed is set to True, function arguments of different types will be
cached separately. For example, f(3) and f(3.0) will be treated as distinct
calls with distinct results.

The original underlying function is accessible through the __wrapped__
attribute. This is useful for introspection, for bypassing the cache, or
for rewrapping the function with a different cache.

from diskcache import Cache
cache = Cache()
@memoize_lease(cache, expire=10, lease=1)
... def fib(number):
... if number == 0:
... return 0
... elif number == 1:
... return 1
... else:
... return fib(number - 1) + fib(number - 2)
print(fib(100))
354224848179261915075

An additional __cache_key__ attribute can be used to generate the cache
key used for the given arguments.

key = fib.cache_key(100)
del cache[key]

Remember to call memoize when decorating a callable. If you forget, then a
TypeError will occur.

:param cache: cache to store callable arguments and return values
:param float expire: seconds until arguments expire
:param float lease: minimum seconds after last execution
we want to update the cache value
:param str name: name given for callable (default None, automatic)
:param bool typed: cache different types separately (default False)
:param str tag: text to associate with arguments (default None)
:return: callable decorator

@grantjenks
Copy link
Owner Author

WIP for #178

@grantjenks
Copy link
Owner Author

I wonder about the use of "diff" for the added cache key to guard against multiple background threads. If the "diff" happened to be short on one call and long on the next then it might start multiple background threads. The stampede version has that problem generally but maybe the lease version should have a "buffer" of some kind.

@grantjenks grantjenks reopened this Jan 22, 2021
@yurj
Copy link

yurj commented Jan 31, 2021

I wonder about the use of "diff" for the added cache key to guard against multiple background threads. If the "diff" happened to be short on one call and long on the next then it might start multiple background threads. The stampede version has that problem generally but maybe the lease version should have a "buffer" of some kind.

The last time execution is the only available guard. You can use the lock on the entry but it would add complexity (logic to tell if the remote call had some problem and unlocking). Consider that this strategy is about remote calls, so we can assume we don't have a storm of requests in the difference between actual exec time and last exec time. Any other complexity can stay in the "timer" function. For example, we can set a timeout in the remote call to protect us against multiple background threads. We can also try to see if the thread we're going to call have the same signature (parameters) of the running one, but again more complexity.

@yurj
Copy link

yurj commented Jan 31, 2021

I agree with the pull request

@bakaiadam
Copy link

I 'm looking something like that for my application tht already uses diskcache. Any idea when will this be merged to the upstream?

Copy link

@yurj yurj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

3 participants