This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Optionally measure size of cache by sum of length of values #1815
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erikjohnston
force-pushed
the
erikj/iter_cache_size
branch
from
87c56c6 to
46aebbb
Compare
| @@ -58,6 +58,18 @@ def __init__(self, max_size, keylen=1, cache_type=dict): | |||
|
|
|||
| lock = threading.Lock() | |||
|
|
|||
| def cache_len(): | |||
| if size_callback is not None: | |||
| return sum(size_callback(node.value) for node in cache.itervalues()) | |||
There was a problem hiding this comment.
This is probably sub-optimal since it iterates the entire cache. You will probably need to store the current size somewhere and update it as things are added or removed.
There was a problem hiding this comment.
If you want to be awful you can move the if statement outside the function def and define the function twice...
| ) | ||
|
|
||
| def __len__(self): | ||
| return len(self._cache) | ||
| if self.iterable: | ||
| return sum(len(value.value) for value in self._cache.itervalues()) |
There was a problem hiding this comment.
This is probably sub-optimal since it iterates the entire cache. You will probably need to store the current size somewhere and update it as things are added or removed.
Instead of calculating the size of the cache repeatedly, which can take a long time now that it can use a callback, instead cache the size and update that on insertion and deletion. This requires changing the cache descriptors to have two caches, one for pending deferreds and the other for the actual values. There's no reason to evict from the pending deferreds as they won't take up any more memory.
erikjohnston
force-pushed
the
erikj/iter_cache_size
branch
from
c0698e0 to
d906206
Compare
|
|
||
| @synchronized | ||
| def cache_set_default(key, value): | ||
| node = cache.get(key, None) | ||
| if node is not None: | ||
| evict() # As the new node may be bigger than the old node. |
There was a problem hiding this comment.
This doesn't seem necessary if we aren't modifying the cache.
| @@ -128,7 +128,7 @@ def test_set(self): | |||
| m = Mock() | |||
| cache = LruCache(1) | |||
|
|
|||
| cache.set("key", "value", m) | |||
| cache.set("key", "value", [m]) | |||
There was a problem hiding this comment.
Maybe use callbacks=[m] here?
erikjohnston
force-pushed
the
erikj/iter_cache_size
branch
from
2e8afb5 to
9e8e236
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.