From 9b7455e918a437c3db91e88dcbf6d9c93fef96f8 Mon Sep 17 00:00:00 2001 From: Bouke Haarsma Date: Fri, 1 Nov 2013 21:15:41 +0100 Subject: [PATCH] Fixed #21351 -- Replaced memoize with Python's lru_cache. Replaced the custom, untested memoize with a similar decorator from Python's 3.2 stdlib. Although some minor performance degradation (see ticket), it is expected that in the long run lru_cache will outperform memoize once it is implemented in C. Thanks to EvilDMP for the report and Baptiste Mispelon for the idea of replacing memoize with lru_cache. --- django/contrib/staticfiles/finders.py | 10 +- django/core/management/commands/loaddata.py | 9 +- django/core/urlresolvers.py | 23 +-- django/test/utils.py | 2 +- django/utils/functional.py | 5 + django/utils/lru_cache.py | 173 ++++++++++++++++++++ django/utils/translation/trans_real.py | 6 +- docs/internals/deprecation.txt | 2 + docs/releases/1.7.txt | 10 ++ tests/decorators/tests.py | 6 +- tests/deprecation/tests.py | 16 ++ tests/staticfiles_tests/tests.py | 13 +- tests/urlpatterns_reverse/tests.py | 17 +- 13 files changed, 254 insertions(+), 38 deletions(-) create mode 100644 django/utils/lru_cache.py diff --git a/django/contrib/staticfiles/finders.py b/django/contrib/staticfiles/finders.py index 3d93d2a447..cf5eb4f85b 100644 --- a/django/contrib/staticfiles/finders.py +++ b/django/contrib/staticfiles/finders.py @@ -4,16 +4,14 @@ import os from django.conf import settings from django.core.exceptions import ImproperlyConfigured from django.core.files.storage import default_storage, Storage, FileSystemStorage -from django.utils.functional import empty, memoize, LazyObject +from django.utils.functional import empty, LazyObject from django.utils.module_loading import import_by_path from django.utils._os import safe_join -from django.utils import six +from django.utils import six, lru_cache from django.contrib.staticfiles import utils from django.contrib.staticfiles.storage import AppStaticStorage -_finders = OrderedDict() - class BaseFinder(object): """ @@ -254,7 +252,8 @@ def get_finders(): yield get_finder(finder_path) -def _get_finder(import_path): +@lru_cache.lru_cache(maxsize=None) +def get_finder(import_path): """ Imports the staticfiles finder class described by import_path, where import_path is the full Python path to the class. @@ -264,4 +263,3 @@ def _get_finder(import_path): raise ImproperlyConfigured('Finder "%s" is not a subclass of "%s"' % (Finder, BaseFinder)) return Finder() -get_finder = memoize(_get_finder, _finders, 1) diff --git a/django/core/management/commands/loaddata.py b/django/core/management/commands/loaddata.py index 72c5d0854f..6aec0c5703 100644 --- a/django/core/management/commands/loaddata.py +++ b/django/core/management/commands/loaddata.py @@ -14,8 +14,9 @@ from django.core.management.color import no_style from django.db import (connections, router, transaction, DEFAULT_DB_ALIAS, IntegrityError, DatabaseError) from django.db.models import get_app_paths +from django.utils import lru_cache from django.utils.encoding import force_text -from django.utils.functional import cached_property, memoize +from django.utils.functional import cached_property from django.utils._os import upath from itertools import product @@ -164,7 +165,8 @@ class Command(BaseCommand): RuntimeWarning ) - def _find_fixtures(self, fixture_label): + @lru_cache.lru_cache(maxsize=None) + def find_fixtures(self, fixture_label): """ Finds fixture files for a given label. """ @@ -220,9 +222,6 @@ class Command(BaseCommand): return fixture_files - _label_to_fixtures_cache = {} - find_fixtures = memoize(_find_fixtures, _label_to_fixtures_cache, 2) - @cached_property def fixture_dirs(self): """ diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index 13919144e5..634eb7519a 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -16,18 +16,14 @@ from django.http import Http404 from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist from django.utils.datastructures import MultiValueDict from django.utils.encoding import force_str, force_text, iri_to_uri -from django.utils.functional import memoize, lazy +from django.utils.functional import lazy from django.utils.http import urlquote from django.utils.module_loading import module_has_submodule from django.utils.regex_helper import normalize -from django.utils import six +from django.utils import six, lru_cache from django.utils.translation import get_language -_resolver_cache = {} # Maps URLconf modules to RegexURLResolver instances. -_ns_resolver_cache = {} # Maps namespaces to RegexURLResolver instances. -_callable_cache = {} # Maps view and url pattern names to their view functions. - # SCRIPT_NAME prefixes for each thread are stored here. If there's no entry for # the current thread (which is the only one we ever access), it is assumed to # be empty. @@ -80,6 +76,7 @@ class NoReverseMatch(Exception): pass +@lru_cache.lru_cache(maxsize=None) def get_callable(lookup_view, can_fail=False): """ Convert a string version of a function name to the callable object. @@ -119,17 +116,17 @@ def get_callable(lookup_view, can_fail=False): "Could not import %s. View does not exist in module %s." % (lookup_view, mod_name)) return lookup_view -get_callable = memoize(get_callable, _callable_cache, 1) +@lru_cache.lru_cache(maxsize=None) def get_resolver(urlconf): if urlconf is None: from django.conf import settings urlconf = settings.ROOT_URLCONF return RegexURLResolver(r'^/', urlconf) -get_resolver = memoize(get_resolver, _resolver_cache, 1) +@lru_cache.lru_cache(maxsize=None) def get_ns_resolver(ns_pattern, resolver): # Build a namespaced resolver for the given parent urlconf pattern. # This makes it possible to have captured parameters in the parent @@ -137,7 +134,6 @@ def get_ns_resolver(ns_pattern, resolver): ns_resolver = RegexURLResolver(ns_pattern, resolver.url_patterns) return RegexURLResolver(r'^/', [ns_resolver]) -get_ns_resolver = memoize(get_ns_resolver, _ns_resolver_cache, 2) def get_mod_func(callback): @@ -523,12 +519,9 @@ reverse_lazy = lazy(reverse, str) def clear_url_caches(): - global _resolver_cache - global _ns_resolver_cache - global _callable_cache - _resolver_cache.clear() - _ns_resolver_cache.clear() - _callable_cache.clear() + get_callable.cache_clear() + get_resolver.cache_clear() + get_ns_resolver.cache_clear() def set_script_prefix(prefix): diff --git a/django/test/utils.py b/django/test/utils.py index 1599a5358b..550d7642b9 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -427,7 +427,7 @@ class TransRealMixin(object): trans_real._translations = {} trans_real._active = local() trans_real._default = None - trans_real._checked_languages = {} + trans_real.check_for_language.cache_clear() def tearDown(self): self.flush_caches() diff --git a/django/utils/functional.py b/django/utils/functional.py index 65ec4b53c4..d52323b220 100644 --- a/django/utils/functional.py +++ b/django/utils/functional.py @@ -2,6 +2,7 @@ import copy import operator from functools import wraps import sys +import warnings from django.utils import six from django.utils.six.moves import copyreg @@ -24,6 +25,10 @@ def memoize(func, cache, num_args): Only the first num_args are considered when creating the key. """ + warnings.warn(u"memoize wrapper is deprecated and will be removed in " + u"Django 1.9. Use django.utils.lru_cache instead.", + PendingDeprecationWarning, 2) + @wraps(func) def wrapper(*args): mem_args = args[:num_args] diff --git a/django/utils/lru_cache.py b/django/utils/lru_cache.py new file mode 100644 index 0000000000..d60a373743 --- /dev/null +++ b/django/utils/lru_cache.py @@ -0,0 +1,173 @@ +try: + from functools import lru_cache + +except ImportError: + # backport of Python's 3.2 lru_cache, written by Raymond Hettinger and + # licensed under MIT license, from: + # + # Should be removed when Django only supports Python 3.2 and above. + + from collections import namedtuple + from functools import update_wrapper + from threading import RLock + + _CacheInfo = namedtuple("CacheInfo", ["hits", "misses", "maxsize", "currsize"]) + + class _HashedSeq(list): + __slots__ = 'hashvalue' + + def __init__(self, tup, hash=hash): + self[:] = tup + self.hashvalue = hash(tup) + + def __hash__(self): + return self.hashvalue + + def _make_key(args, kwds, typed, + kwd_mark = (object(),), + fasttypes = {int, str, frozenset, type(None)}, + sorted=sorted, tuple=tuple, type=type, len=len): + 'Make a cache key from optionally typed positional and keyword arguments' + key = args + if kwds: + sorted_items = sorted(kwds.items()) + key += kwd_mark + for item in sorted_items: + key += item + if typed: + key += tuple(type(v) for v in args) + if kwds: + key += tuple(type(v) for k, v in sorted_items) + elif len(key) == 1 and type(key[0]) in fasttypes: + return key[0] + return _HashedSeq(key) + + def lru_cache(maxsize=100, typed=False): + """Least-recently-used cache decorator. + + If *maxsize* is set to None, the LRU features are disabled and the cache + can grow without bound. + + If *typed* is True, arguments of different types will be cached separately. + For example, f(3.0) and f(3) will be treated as distinct calls with + distinct results. + + Arguments to the cached function must be hashable. + + View the cache statistics named tuple (hits, misses, maxsize, currsize) with + f.cache_info(). Clear the cache and statistics with f.cache_clear(). + Access the underlying function with f.__wrapped__. + + See: http://en.wikipedia.org/wiki/Cache_algorithms#Least_Recently_Used + + """ + + # Users should only access the lru_cache through its public API: + # cache_info, cache_clear, and f.__wrapped__ + # The internals of the lru_cache are encapsulated for thread safety and + # to allow the implementation to change (including a possible C version). + + def decorating_function(user_function): + + cache = dict() + stats = [0, 0] # make statistics updateable non-locally + HITS, MISSES = 0, 1 # names for the stats fields + make_key = _make_key + cache_get = cache.get # bound method to lookup key or return None + _len = len # localize the global len() function + lock = RLock() # because linkedlist updates aren't threadsafe + root = [] # root of the circular doubly linked list + root[:] = [root, root, None, None] # initialize by pointing to self + nonlocal_root = [root] # make updateable non-locally + PREV, NEXT, KEY, RESULT = 0, 1, 2, 3 # names for the link fields + + if maxsize == 0: + + def wrapper(*args, **kwds): + # no caching, just do a statistics update after a successful call + result = user_function(*args, **kwds) + stats[MISSES] += 1 + return result + + elif maxsize is None: + + def wrapper(*args, **kwds): + # simple caching without ordering or size limit + key = make_key(args, kwds, typed) + result = cache_get(key, root) # root used here as a unique not-found sentinel + if result is not root: + stats[HITS] += 1 + return result + result = user_function(*args, **kwds) + cache[key] = result + stats[MISSES] += 1 + return result + + else: + + def wrapper(*args, **kwds): + # size limited caching that tracks accesses by recency + key = make_key(args, kwds, typed) if kwds or typed else args + with lock: + link = cache_get(key) + if link is not None: + # record recent use of the key by moving it to the front of the list + root, = nonlocal_root + link_prev, link_next, key, result = link + link_prev[NEXT] = link_next + link_next[PREV] = link_prev + last = root[PREV] + last[NEXT] = root[PREV] = link + link[PREV] = last + link[NEXT] = root + stats[HITS] += 1 + return result + result = user_function(*args, **kwds) + with lock: + root, = nonlocal_root + if key in cache: + # getting here means that this same key was added to the + # cache while the lock was released. since the link + # update is already done, we need only return the + # computed result and update the count of misses. + pass + elif _len(cache) >= maxsize: + # use the old root to store the new key and result + oldroot = root + oldroot[KEY] = key + oldroot[RESULT] = result + # empty the oldest link and make it the new root + root = nonlocal_root[0] = oldroot[NEXT] + oldkey = root[KEY] + oldvalue = root[RESULT] + root[KEY] = root[RESULT] = None + # now update the cache dictionary for the new links + del cache[oldkey] + cache[key] = oldroot + else: + # put result in a new link at the front of the list + last = root[PREV] + link = [last, root, key, result] + last[NEXT] = root[PREV] = cache[key] = link + stats[MISSES] += 1 + return result + + def cache_info(): + """Report cache statistics""" + with lock: + return _CacheInfo(stats[HITS], stats[MISSES], maxsize, len(cache)) + + def cache_clear(): + """Clear the cache and cache statistics""" + with lock: + cache.clear() + root = nonlocal_root[0] + root[:] = [root, root, None, None] + stats[:] = [0, 0] + + wrapper.__wrapped__ = user_function + wrapper.cache_info = cache_info + wrapper.cache_clear = cache_clear + return update_wrapper(wrapper, user_function) + + return decorating_function diff --git a/django/utils/translation/trans_real.py b/django/utils/translation/trans_real.py index ff2d8ca25d..7c545dbd3d 100644 --- a/django/utils/translation/trans_real.py +++ b/django/utils/translation/trans_real.py @@ -14,10 +14,9 @@ import warnings from django.dispatch import receiver from django.test.signals import setting_changed from django.utils.encoding import force_str, force_text -from django.utils.functional import memoize from django.utils._os import upath from django.utils.safestring import mark_safe, SafeData -from django.utils import six +from django.utils import six, lru_cache from django.utils.six import StringIO from django.utils.translation import TranslatorCommentWarning, trim_whitespace @@ -33,7 +32,6 @@ _default = None # This is a cache for normalized accept-header languages to prevent multiple # file lookups when checking the same locale on repeated requests. _accepted = {} -_checked_languages = {} # magic gettext number to separate context from message CONTEXT_SEPARATOR = "\x04" @@ -390,6 +388,7 @@ def all_locale_paths(): return [globalpath] + list(settings.LOCALE_PATHS) +@lru_cache.lru_cache(maxsize=None) def check_for_language(lang_code): """ Checks whether there is a global language file for the given language @@ -401,7 +400,6 @@ def check_for_language(lang_code): if gettext_module.find('django', path, [to_locale(lang_code)]) is not None: return True return False -check_for_language = memoize(check_for_language, _checked_languages, 1) def get_supported_language_variant(lang_code, supported=None, strict=False): diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index f2e2c4c2f7..f2d121e641 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -479,6 +479,8 @@ these changes. * The ``zh-cn`` and ``zh-tw`` language codes will be removed and have been replaced by the ``zh-hans`` and ``zh-hant`` language code respectively. +* The internal ``django.utils.functional.memoize`` will be removed. + 2.0 --- diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 2aab6f26a7..d703f44e4b 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -752,3 +752,13 @@ recently introduced language codes ``zh-hans`` and ``zh-hant`` respectively. If you use these language codes, you should rename the locale directories and update your settings to reflect these changes. The deprecated language codes will be removed in Django 1.9. + +``django.utils.functional.memoize`` function +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The function ``memoize`` is deprecated and should be replaced by the +``functools.lru_cache`` decorator (available from Python 3.2 onwards). + +Django ships a backport of this decorator for older Python versions and it's +available at ``django.utils.lru_cache.lru_cache``. The deprecated function will +be removed in Django 1.9. diff --git a/tests/decorators/tests.py b/tests/decorators/tests.py index db00f36051..9c654e500a 100644 --- a/tests/decorators/tests.py +++ b/tests/decorators/tests.py @@ -1,5 +1,6 @@ from functools import wraps from unittest import TestCase +import warnings from django.contrib.admin.views.decorators import staff_member_required from django.contrib.auth.decorators import login_required, permission_required, user_passes_test @@ -58,11 +59,14 @@ full_decorator = compose( staff_member_required, # django.utils.functional - lambda f: memoize(f, {}, 1), allow_lazy, lazy, ) +# suppress the deprecation warning of memoize +with warnings.catch_warnings(record=True): + fully_decorated = memoize(fully_decorated, {}, 1) + fully_decorated = full_decorator(fully_decorated) diff --git a/tests/deprecation/tests.py b/tests/deprecation/tests.py index 7d9ed16636..2a0922a0a8 100644 --- a/tests/deprecation/tests.py +++ b/tests/deprecation/tests.py @@ -4,6 +4,7 @@ import warnings from django.test import SimpleTestCase, RequestFactory, override_settings from django.utils import six, translation from django.utils.deprecation import RenameMethodsBase +from django.utils.functional import memoize class RenameManagerMethods(RenameMethodsBase): @@ -205,3 +206,18 @@ class DeprecatedChineseLanguageCodes(SimpleTestCase): "The use of the language code 'zh-tw' is deprecated. " "Please use the 'zh-hant' translation instead.", ]) + + +class DeprecatingMemoizeTest(SimpleTestCase): + def test_deprecated_memoize(self): + """ + Ensure the correct warning is raised when memoize is used. + """ + warnings.simplefilter('always') + + with warnings.catch_warnings(record=True) as recorded: + memoize(lambda x: x, {}, 1) + msg = str(recorded.pop().message) + self.assertEqual(msg, + 'memoize wrapper is deprecated and will be removed in Django ' + '1.9. Use django.utils.lru_cache instead.') diff --git a/tests/staticfiles_tests/tests.py b/tests/staticfiles_tests/tests.py index 463ab47030..39ce49817c 100644 --- a/tests/staticfiles_tests/tests.py +++ b/tests/staticfiles_tests/tests.py @@ -55,7 +55,7 @@ class BaseStaticFilesTestCase(object): storage.staticfiles_storage._wrapped = empty # Clear the cached staticfile finders, so they are reinitialized every # run and pick up changes in settings.STATICFILES_DIRS. - finders._finders.clear() + finders.get_finder.cache_clear() testfiles_path = os.path.join(TEST_ROOT, 'apps', 'test', 'static', 'test') # To make sure SVN doesn't hangs itself with the non-ASCII characters @@ -561,7 +561,7 @@ class TestCollectionCachedStorage(BaseCollectionTestCase, Test that post_processing indicates the origin of the error when it fails. Regression test for #18986. """ - finders._finders.clear() + finders.get_finder.cache_clear() err = six.StringIO() with self.assertRaises(Exception): call_command('collectstatic', interactive=False, verbosity=0, stderr=err) @@ -756,6 +756,15 @@ class TestMiscFinder(TestCase): self.assertRaises(ImproperlyConfigured, finders.get_finder, 'foo.bar.FooBarFinder') + def test_cache(self): + finders.get_finder.cache_clear() + for n in range(10): + finders.get_finder( + 'django.contrib.staticfiles.finders.FileSystemFinder') + cache_info = finders.get_finder.cache_info() + self.assertEqual(cache_info.hits, 9) + self.assertEqual(cache_info.currsize, 1) + @override_settings(STATICFILES_DIRS='a string') def test_non_tuple_raises_exception(self): """ diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index b2a6d83443..b4056ac7fa 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -17,6 +17,7 @@ from django.test.utils import override_settings from django.utils import six from . import urlconf_outer, middleware, views +from .views import empty_view resolve_test_data = ( @@ -662,12 +663,20 @@ class ErroneousViewTests(TestCase): class ViewLoadingTests(TestCase): def test_view_loading(self): + self.assertEqual(get_callable('urlpatterns_reverse.views.empty_view'), + empty_view) + + # passing a callable should return the callable + self.assertEqual(get_callable(empty_view), empty_view) + + def test_exceptions(self): # A missing view (identified by an AttributeError) should raise # ViewDoesNotExist, ... - six.assertRaisesRegex(self, ViewDoesNotExist, ".*View does not exist in.*", - get_callable, - 'urlpatterns_reverse.views.i_should_not_exist') + six.assertRaisesRegex(self, ViewDoesNotExist, + ".*View does not exist in.*", + get_callable, + 'urlpatterns_reverse.views.i_should_not_exist') # ... but if the AttributeError is caused by something else don't # swallow it. self.assertRaises(AttributeError, get_callable, - 'urlpatterns_reverse.views_broken.i_am_broken') + 'urlpatterns_reverse.views_broken.i_am_broken')