Skip to content

LRUStoreCache: cache "contains" by contains checks #1499

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/release.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ Maintenance
* Add ``docs`` requirements to ``pyproject.toml``
By :user:`John A. Kirkham <jakirkham>` :issue:`1494`.

* Fixed caching issue in ``LRUStoreCache``.
By :user:`Mads R. B. Kristensen <madsbk>` :issue:`1499`.

.. _release_2.16.0:

2.16.0
Expand Down
2 changes: 1 addition & 1 deletion zarr/_storage/v3.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ def __init__(self, store, max_size: int):
self._max_size = max_size
self._current_size = 0
self._keys_cache = None
self._contains_cache = None
self._contains_cache = {}
self._listdir_cache: Dict[Path, Any] = dict()
self._values_cache: Dict[Path, Any] = OrderedDict()
self._mutex = Lock()
Expand Down
10 changes: 5 additions & 5 deletions zarr/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -2393,7 +2393,7 @@ def __init__(self, store: StoreLike, max_size: int):
self._max_size = max_size
self._current_size = 0
self._keys_cache = None
self._contains_cache = None
self._contains_cache: Dict[Any, Any] = {}
self._listdir_cache: Dict[Path, Any] = dict()
self._values_cache: Dict[Path, Any] = OrderedDict()
self._mutex = Lock()
Expand Down Expand Up @@ -2434,9 +2434,9 @@ def __iter__(self):

def __contains__(self, key):
with self._mutex:
if self._contains_cache is None:
self._contains_cache = set(self._keys())
return key in self._contains_cache
if key not in self._contains_cache:
self._contains_cache[key] = key in self._store
return self._contains_cache[key]

def clear(self):
self._store.clear()
Expand Down Expand Up @@ -2506,7 +2506,7 @@ def invalidate_keys(self):

def _invalidate_keys(self):
self._keys_cache = None
self._contains_cache = None
self._contains_cache.clear()
self._listdir_cache.clear()

def _invalidate_value(self, key):
Expand Down
15 changes: 9 additions & 6 deletions zarr/tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -2196,7 +2196,10 @@ def test_cache_keys(self):
assert keys == sorted(cache.keys())
assert 1 == store.counter["keys"]
assert foo_key in cache
assert 0 == store.counter["__contains__", foo_key]
assert 1 == store.counter["__contains__", foo_key]
# the next check for `foo_key` is cached
assert foo_key in cache
assert 1 == store.counter["__contains__", foo_key]
assert keys == sorted(cache)
assert 0 == store.counter["__iter__"]
assert 1 == store.counter["keys"]
Expand All @@ -2215,23 +2218,23 @@ def test_cache_keys(self):
keys = sorted(cache.keys())
assert keys == [bar_key, baz_key, foo_key]
assert 3 == store.counter["keys"]
assert 0 == store.counter["__contains__", foo_key]
assert 1 == store.counter["__contains__", foo_key]
assert 0 == store.counter["__iter__"]
cache.invalidate_keys()
keys = sorted(cache)
assert keys == [bar_key, baz_key, foo_key]
assert 4 == store.counter["keys"]
assert 0 == store.counter["__contains__", foo_key]
assert 1 == store.counter["__contains__", foo_key]
assert 0 == store.counter["__iter__"]
cache.invalidate_keys()
assert foo_key in cache
assert 5 == store.counter["keys"]
assert 0 == store.counter["__contains__", foo_key]
assert 4 == store.counter["keys"]
assert 2 == store.counter["__contains__", foo_key]
assert 0 == store.counter["__iter__"]

# check these would get counted if called directly
assert foo_key in store
assert 1 == store.counter["__contains__", foo_key]
assert 3 == store.counter["__contains__", foo_key]
assert keys == sorted(store)
assert 1 == store.counter["__iter__"]

Expand Down