From 7cfe37ad1a9bc198eee717dd0e4cae66c48dfa34 Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 21:59:27 +0100 Subject: [PATCH 01/12] Make `cached_property` derive from `property` --- cached_property.py | 77 +++++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/cached_property.py b/cached_property.py index 67fa01a..487fba8 100644 --- a/cached_property.py +++ b/cached_property.py @@ -14,7 +14,7 @@ asyncio = None -class cached_property(object): +class cached_property(property): """ A property that is only computed once per instance and then replaces itself with an ordinary attribute. Deleting the attribute resets the property. @@ -22,57 +22,61 @@ class cached_property(object): """ # noqa def __init__(self, func): - self.__doc__ = getattr(func, "__doc__") + self.value = self._sentinel = object() self.func = func def __get__(self, obj, cls): if obj is None: return self - if asyncio and asyncio.iscoroutinefunction(self.func): return self._wrap_in_coroutine(obj) + if self.value is self._sentinel: + self.value = self.func(obj) + return self.value - value = obj.__dict__[self.func.__name__] = self.func(obj) - return value + def __set__(self, obj, value): + self.value = value + + def __delete__(self, obj): + self.value = self._sentinel def _wrap_in_coroutine(self, obj): @asyncio.coroutine def wrapper(): - future = asyncio.ensure_future(self.func(obj)) - obj.__dict__[self.func.__name__] = future - return future + if self.value is self._sentinel: + self.value = asyncio.ensure_future(self.func(obj)) + return self.value return wrapper() -class threaded_cached_property(object): +class threaded_cached_property(cached_property): """ A cached_property version for use in environments where multiple threads might concurrently try to access the property. """ def __init__(self, func): - self.__doc__ = getattr(func, "__doc__") - self.func = func + super(threaded_cached_property, self).__init__(func) self.lock = threading.RLock() def __get__(self, obj, cls): if obj is None: return self - obj_dict = obj.__dict__ - name = self.func.__name__ with self.lock: - try: - # check if the value was computed before the lock was acquired - return obj_dict[name] + return super(threaded_cached_property, self).__get__(obj, cls) - except KeyError: - # if not, do the calculation and release the lock - return obj_dict.setdefault(name, self.func(obj)) + def __set__(self, obj, value): + with self.lock: + super(threaded_cached_property, self).__set__(obj, value) + + def __delete__(self, obj): + with self.lock: + super(threaded_cached_property, self).__delete__(obj) -class cached_property_with_ttl(object): +class cached_property_with_ttl(cached_property): """ A property that is only computed once per instance and then replaces itself with an ordinary attribute. Setting the ttl to a number expresses how long @@ -97,31 +101,20 @@ def __get__(self, obj, cls): return self now = time() - obj_dict = obj.__dict__ - name = self.__name__ - try: - value, last_updated = obj_dict[name] - except KeyError: - pass - else: - ttl_expired = self.ttl and self.ttl < now - last_updated - if not ttl_expired: + if self.value is not self._sentinel: + value, last_updated = self.value + if not self.ttl or self.ttl > now - last_updated: return value - value = self.func(obj) - obj_dict[name] = (value, now) + self.value = value, _ = (self.func(obj), now) return value - def __delete__(self, obj): - obj.__dict__.pop(self.__name__, None) - def __set__(self, obj, value): - obj.__dict__[self.__name__] = (value, time()) + super(cached_property_with_ttl, self).__set__(obj, (value, time())) def _prepare_func(self, func): - self.func = func + super(cached_property_with_ttl, self).__init__(func) if func: - self.__doc__ = func.__doc__ self.__name__ = func.__name__ self.__module__ = func.__module__ @@ -131,7 +124,7 @@ def _prepare_func(self, func): timed_cached_property = cached_property_with_ttl -class threaded_cached_property_with_ttl(cached_property_with_ttl): +class threaded_cached_property_with_ttl(cached_property_with_ttl, threaded_cached_property): """ A cached_property version for use in environments where multiple threads might concurrently try to access the property. @@ -145,6 +138,14 @@ def __get__(self, obj, cls): with self.lock: return super(threaded_cached_property_with_ttl, self).__get__(obj, cls) + def __set__(self, obj, value): + with self.lock: + return super(threaded_cached_property_with_ttl, self).__set__(obj, value) + + def __delete__(self, obj): + with self.lock: + return super(threaded_cached_property_with_ttl, self).__delete__(obj) + # Alias to make threaded_cached_property_with_ttl easier to use threaded_cached_property_ttl = threaded_cached_property_with_ttl From 63d91a20376b22f6f2ebfb3d02c26cee082aa25a Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 22:18:12 +0100 Subject: [PATCH 02/12] Extract magic attributes from `cached_property.func` when needed --- cached_property.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/cached_property.py b/cached_property.py index 487fba8..db9ec86 100644 --- a/cached_property.py +++ b/cached_property.py @@ -34,6 +34,11 @@ def __get__(self, obj, cls): self.value = self.func(obj) return self.value + def __getattribute__(self, name): + if name in ('__doc__', '__name__', '__module__'): + return getattr(self.func, name) + return super(cached_property, self).__getattribute__(name) + def __set__(self, obj, value): self.value = value @@ -63,7 +68,6 @@ def __init__(self, func): def __get__(self, obj, cls): if obj is None: return self - with self.lock: return super(threaded_cached_property, self).__get__(obj, cls) @@ -90,10 +94,10 @@ def __init__(self, ttl=None): else: func = None self.ttl = ttl - self._prepare_func(func) + super(cached_property_with_ttl, self).__init__(func) def __call__(self, func): - self._prepare_func(func) + super(cached_property_with_ttl, self).__init__(func) return self def __get__(self, obj, cls): @@ -112,12 +116,6 @@ def __get__(self, obj, cls): def __set__(self, obj, value): super(cached_property_with_ttl, self).__set__(obj, (value, time())) - def _prepare_func(self, func): - super(cached_property_with_ttl, self).__init__(func) - if func: - self.__name__ = func.__name__ - self.__module__ = func.__module__ - # Aliases to make cached_property_with_ttl easier to use cached_property_ttl = cached_property_with_ttl From b30a434880d73c5728a8de0aea4556f83b14cfee Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 22:29:02 +0100 Subject: [PATCH 03/12] Add self (@althonos) to AUTHORS.rst --- AUTHORS.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index 4e5f411..eadff81 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -18,3 +18,4 @@ Contributors * Ionel Cristian Mărieș (@ionelmc) * Malyshev Artem (@proofit404) * Volker Braun (@vbraun) +* Martin Larralde (@althonos) From 5b2d0cc0fb6e36aeedcb7770cca0d916ee2885c6 Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 22:24:25 +0100 Subject: [PATCH 04/12] Use `functools.wraps` to extract attributes from wrapped function --- cached_property.py | 5 +++++ tests/test_cached_property.py | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/cached_property.py b/cached_property.py index db9ec86..6a22033 100644 --- a/cached_property.py +++ b/cached_property.py @@ -6,6 +6,7 @@ __license__ = "BSD" from time import time +import functools import threading try: @@ -24,6 +25,10 @@ class cached_property(property): def __init__(self, func): self.value = self._sentinel = object() self.func = func + functools.wraps(func, self) + # self.__name__ = getattr(func, '__name__', None) + # self.__doc__ = getattr(func, '__doc__', None) + # self.__module__ = getattr(func, '__module__', None) def __get__(self, obj, cls): if obj is None: diff --git a/tests/test_cached_property.py b/tests/test_cached_property.py index 5082416..fa19563 100644 --- a/tests/test_cached_property.py +++ b/tests/test_cached_property.py @@ -27,6 +27,8 @@ def add_control(self): @cached_property_decorator def add_cached(self): + """A cached adder. + """ if threadsafe: time.sleep(1) # Need to guard this since += isn't atomic. @@ -68,6 +70,12 @@ def assert_cached(self, check, expected): self.assertEqual(check.add_cached, expected) self.assertEqual(check.cached_total, expected) + def test_magic_attributes(self): + Check = CheckFactory(self.cached_property_factory) + self.assertEqual(Check.add_cached.__doc__.strip(), "A cached adder.") + self.assertEqual(Check.add_cached.__name__.strip(), "add_cached") + self.assertEqual(Check.add_cached.__module__, __name__) + def test_cached_property(self): Check = CheckFactory(self.cached_property_factory) check = Check() From 47e20fe053ddc80e18d3080b42a2d298dce6e6d8 Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 22:28:06 +0100 Subject: [PATCH 05/12] Run `black` on `cached_property.py` --- cached_property.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cached_property.py b/cached_property.py index 6a22033..1838f8d 100644 --- a/cached_property.py +++ b/cached_property.py @@ -40,7 +40,7 @@ def __get__(self, obj, cls): return self.value def __getattribute__(self, name): - if name in ('__doc__', '__name__', '__module__'): + if name in ("__doc__", "__name__", "__module__"): return getattr(self.func, name) return super(cached_property, self).__getattribute__(name) @@ -127,7 +127,9 @@ def __set__(self, obj, value): timed_cached_property = cached_property_with_ttl -class threaded_cached_property_with_ttl(cached_property_with_ttl, threaded_cached_property): +class threaded_cached_property_with_ttl( + cached_property_with_ttl, threaded_cached_property +): """ A cached_property version for use in environments where multiple threads might concurrently try to access the property. From f63dc5026ce43c8a1884d3fefb73cad519bacb3e Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 22:41:08 +0100 Subject: [PATCH 06/12] Implement `__set_name__` on `cached_property` objects --- cached_property.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/cached_property.py b/cached_property.py index 1838f8d..2a0f06f 100644 --- a/cached_property.py +++ b/cached_property.py @@ -5,9 +5,9 @@ __version__ = "1.5.1" __license__ = "BSD" -from time import time import functools import threading +from time import time try: import asyncio @@ -26,9 +26,6 @@ def __init__(self, func): self.value = self._sentinel = object() self.func = func functools.wraps(func, self) - # self.__name__ = getattr(func, '__name__', None) - # self.__doc__ = getattr(func, '__doc__', None) - # self.__module__ = getattr(func, '__module__', None) def __get__(self, obj, cls): if obj is None: @@ -39,10 +36,8 @@ def __get__(self, obj, cls): self.value = self.func(obj) return self.value - def __getattribute__(self, name): - if name in ("__doc__", "__name__", "__module__"): - return getattr(self.func, name) - return super(cached_property, self).__getattribute__(name) + def __set_name__(self, owner, name): + self.__name__ = name def __set__(self, obj, value): self.value = value From b97832a92ebe64d51209c1d88157fc1fbc09a234 Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 22:57:07 +0100 Subject: [PATCH 07/12] Fix `cached_property` sharing cache for all objects --- cached_property.py | 35 ++++++++++++++++++++++------------- tests/test_cached_property.py | 17 +++++++++++++++++ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/cached_property.py b/cached_property.py index 2a0f06f..75aca08 100644 --- a/cached_property.py +++ b/cached_property.py @@ -7,6 +7,7 @@ import functools import threading +import weakref from time import time try: @@ -22,35 +23,43 @@ class cached_property(property): Source: https://github.com/bottlepy/bottle/commit/fa7733e075da0d790d809aa3d2f53071897e6f76 """ # noqa + _sentinel = object() + def __init__(self, func): - self.value = self._sentinel = object() + self.cache = weakref.WeakKeyDictionary() self.func = func - functools.wraps(func, self) + functools.update_wrapper(self, func) def __get__(self, obj, cls): if obj is None: return self + if asyncio and asyncio.iscoroutinefunction(self.func): return self._wrap_in_coroutine(obj) - if self.value is self._sentinel: - self.value = self.func(obj) - return self.value + + value = self.cache.get(obj, self._sentinel) + if value is self._sentinel: + value = self.cache[obj] = self.func(obj) + + return value def __set_name__(self, owner, name): self.__name__ = name def __set__(self, obj, value): - self.value = value + self.cache[obj] = value def __delete__(self, obj): - self.value = self._sentinel + del self.cache[obj] def _wrap_in_coroutine(self, obj): + @asyncio.coroutine def wrapper(): - if self.value is self._sentinel: - self.value = asyncio.ensure_future(self.func(obj)) - return self.value + value = self.cache.get(obj, self._sentinel) + if value is self._sentinel: + self.cache[obj] = value = asyncio.ensure_future(self.func(obj)) + return value return wrapper() @@ -105,12 +114,12 @@ def __get__(self, obj, cls): return self now = time() - if self.value is not self._sentinel: - value, last_updated = self.value + if obj in self.cache: + value, last_updated = self.cache[obj] if not self.ttl or self.ttl > now - last_updated: return value - self.value = value, _ = (self.func(obj), now) + value, _ = self.cache[obj] = (self.func(obj), now) return value def __set__(self, obj, value): diff --git a/tests/test_cached_property.py b/tests/test_cached_property.py index fa19563..73d87d7 100644 --- a/tests/test_cached_property.py +++ b/tests/test_cached_property.py @@ -151,6 +151,23 @@ def test_threads(self): self.assert_cached(check, num_threads) self.assert_cached(check, num_threads) + def test_object_independent(self): + Check = CheckFactory(self.cached_property_factory) + check1 = Check() + check2 = Check() + + self.assert_cached(check1, 1) + self.assert_cached(check1, 1) + self.assert_cached(check2, 1) + self.assert_cached(check2, 1) + + del check1.add_cached + + self.assert_cached(check1, 2) + self.assert_cached(check1, 2) + self.assert_cached(check2, 1) + self.assert_cached(check2, 1) + class TestThreadedCachedProperty(TestCachedProperty): """Tests for threaded_cached_property""" From e5779507f554cdbb8ca1d06935b8654dd0db9b7c Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 23:09:42 +0100 Subject: [PATCH 08/12] Add @VadimPushtaev to AUTHORS.rst --- AUTHORS.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index eadff81..388dc59 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -18,4 +18,5 @@ Contributors * Ionel Cristian Mărieș (@ionelmc) * Malyshev Artem (@proofit404) * Volker Braun (@vbraun) +* Vadim Pushtaev (@VadimPushtaev) * Martin Larralde (@althonos) From 2c2e11d44d491ccf03f8aae42667c2ba8df27a34 Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Fri, 23 Nov 2018 23:21:53 +0100 Subject: [PATCH 09/12] Fix `update_wrapper` failing on missing attributes in Py2 --- cached_property.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cached_property.py b/cached_property.py index 75aca08..338c797 100644 --- a/cached_property.py +++ b/cached_property.py @@ -6,6 +6,7 @@ __license__ = "BSD" import functools +import sys import threading import weakref from time import time @@ -25,10 +26,21 @@ class cached_property(property): _sentinel = object() + if sys.version_info[0] < 3: + + def _update_wrapper(self, func): + self.__doc__ = getattr(func, "__doc__", None) + self.__module__ = getattr(func, "__module__", None) + self.__name__ = getattr(func, "__name__", None) + + else: + + _update_wrapper = functools.update_wrapper + def __init__(self, func): self.cache = weakref.WeakKeyDictionary() self.func = func - functools.update_wrapper(self, func) + self._update_wrapper(func) def __get__(self, obj, cls): if obj is None: @@ -53,7 +65,7 @@ def __delete__(self, obj): del self.cache[obj] def _wrap_in_coroutine(self, obj): - + @asyncio.coroutine def wrapper(): value = self.cache.get(obj, self._sentinel) From c2b6182a1bcdb0d70580ad98650def84d3d66214 Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Sat, 24 Nov 2018 00:03:29 +0100 Subject: [PATCH 10/12] Add test to make sure the property cache is freed as expected --- tests/test_cached_property.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_cached_property.py b/tests/test_cached_property.py index 73d87d7..791cddd 100644 --- a/tests/test_cached_property.py +++ b/tests/test_cached_property.py @@ -151,6 +151,18 @@ def test_threads(self): self.assert_cached(check, num_threads) self.assert_cached(check, num_threads) + def test_garbage_collection(self): + Check = CheckFactory(self.cached_property_factory) + check = Check() + check.add_cached = "foo" + + # check the instance is in the cache + self.assertIn(check, Check.add_cached.cache) + # remove the only reference to the Check instance + del check + # make sure the cache of the deleted object was removed + self.assertEqual(Check.add_cached.cache, {}) + def test_object_independent(self): Check = CheckFactory(self.cached_property_factory) check1 = Check() From 2fd46da3b9da6c593ebf0f51595f2c98e038c259 Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Sat, 24 Nov 2018 00:16:20 +0100 Subject: [PATCH 11/12] Use `len` to check cache emptiness in `tests.test_cached_property` --- tests/test_cached_property.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cached_property.py b/tests/test_cached_property.py index 791cddd..11fc7ed 100644 --- a/tests/test_cached_property.py +++ b/tests/test_cached_property.py @@ -161,7 +161,7 @@ def test_garbage_collection(self): # remove the only reference to the Check instance del check # make sure the cache of the deleted object was removed - self.assertEqual(Check.add_cached.cache, {}) + self.assertEqual(len(Check.add_cached.cache), 0) def test_object_independent(self): Check = CheckFactory(self.cached_property_factory) From 55763c14ee62972042d728e95b3d51f117b0d878 Mon Sep 17 00:00:00 2001 From: Martin Larralde Date: Sat, 24 Nov 2018 00:20:55 +0100 Subject: [PATCH 12/12] Skip `test_garbage_collection` in implementations other than CPython --- tests/test_cached_property.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_cached_property.py b/tests/test_cached_property.py index 11fc7ed..a448d30 100644 --- a/tests/test_cached_property.py +++ b/tests/test_cached_property.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +import platform import time import unittest from threading import Lock, Thread @@ -151,6 +152,8 @@ def test_threads(self): self.assert_cached(check, num_threads) self.assert_cached(check, num_threads) + @unittest.skipUnless(platform.python_implementation() == "CPython", + "unknow garbage collection mechanism") def test_garbage_collection(self): Check = CheckFactory(self.cached_property_factory) check = Check() @@ -161,7 +164,7 @@ def test_garbage_collection(self): # remove the only reference to the Check instance del check # make sure the cache of the deleted object was removed - self.assertEqual(len(Check.add_cached.cache), 0) + self.assertEqual(Check.add_cached.cache, {}) def test_object_independent(self): Check = CheckFactory(self.cached_property_factory)