From 6d302f6396b7531eaa22dc5b544d233334e6fc92 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Sat, 5 Jul 2014 14:43:12 +0200 Subject: [PATCH] Fixed pyinotify performance regression in 15f82c7011 Refs #9722. Thanks Tim Graham for the review. --- django/contrib/staticfiles/handlers.py | 4 +++ django/utils/autoreload.py | 43 ++++++++++++++++++++------ tests/utils_tests/test_autoreload.py | 17 ++++++++++ 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/django/contrib/staticfiles/handlers.py b/django/contrib/staticfiles/handlers.py index bb54db6b7d..e711f9a476 100644 --- a/django/contrib/staticfiles/handlers.py +++ b/django/contrib/staticfiles/handlers.py @@ -12,6 +12,10 @@ class StaticFilesHandler(WSGIHandler): WSGI middleware that intercepts calls to the static files directory, as defined by the STATIC_URL setting, and serves those files. """ + # May be used to differentiate between handler types (e.g. in a + # request_finished signal) + handles_files = True + def __init__(self, application): self.application = application self.base_url = urlparse(self.get_base_url()) diff --git a/django/utils/autoreload.py b/django/utils/autoreload.py index d2369067ab..e95b6128fd 100644 --- a/django/utils/autoreload.py +++ b/django/utils/autoreload.py @@ -77,20 +77,31 @@ _mtimes = {} _win = (sys.platform == "win32") _error_files = [] +_cached_modules = set() +_cached_filenames = [] - -def gen_filenames(): +def gen_filenames(only_new=False): """ - Yields a generator over filenames referenced in sys.modules and translation + Returns a list of filenames referenced in sys.modules and translation files. """ # N.B. ``list(...)`` is needed, because this runs in parallel with # application code which might be mutating ``sys.modules``, and this will # fail with RuntimeError: cannot mutate dictionary while iterating - filenames = [filename.__file__ for filename in list(sys.modules.values()) - if hasattr(filename, '__file__')] + global _cached_modules, _cached_filenames + module_values = set(sys.modules.values()) + if _cached_modules == module_values: + # No changes in module list, short-circuit the function + if only_new: + return [] + else: + return _cached_filenames - if settings.USE_I18N: + new_modules = module_values - _cached_modules + new_filenames = [filename.__file__ for filename in new_modules + if hasattr(filename, '__file__')] + + if not _cached_filenames and settings.USE_I18N: # Add the names of the .mo files that can be generated # by compilemessages management command to the list of files watched. basedirs = [os.path.join(os.path.dirname(os.path.dirname(__file__)), @@ -105,9 +116,14 @@ def gen_filenames(): for dirpath, dirnames, locale_filenames in os.walk(basedir): for filename in locale_filenames: if filename.endswith('.mo'): - filenames.append(os.path.join(dirpath, filename)) + new_filenames.append(os.path.join(dirpath, filename)) - for filename in filenames + _error_files: + if only_new: + filelist = new_filenames + else: + filelist = _cached_filenames + new_filenames + _error_files + filenames = [] + for filename in filelist: if not filename: continue if filename.endswith(".pyc") or filename.endswith(".pyo"): @@ -115,7 +131,10 @@ def gen_filenames(): if filename.endswith("$py.class"): filename = filename[:-9] + ".py" if os.path.exists(filename): - yield filename + filenames.append(filename) + _cached_modules = _cached_modules.union(new_modules) + _cached_filenames += new_filenames + return filenames def reset_translations(): @@ -145,6 +164,10 @@ def inotify_code_changed(): notifier = pyinotify.Notifier(wm, EventHandler()) def update_watch(sender=None, **kwargs): + if sender and getattr(sender, 'handles_files', False): + # No need to update watches when request serves files. + # (sender is supposed to be a django.core.handlers.BaseHandler subclass) + return mask = ( pyinotify.IN_MODIFY | pyinotify.IN_DELETE | @@ -153,7 +176,7 @@ def inotify_code_changed(): pyinotify.IN_MOVED_TO | pyinotify.IN_CREATE ) - for path in gen_filenames(): + for path in gen_filenames(only_new=True): wm.add_watch(path, mask) # New modules may get imported when a request is processed. diff --git a/tests/utils_tests/test_autoreload.py b/tests/utils_tests/test_autoreload.py index 1024b02efe..463cc1bcd0 100644 --- a/tests/utils_tests/test_autoreload.py +++ b/tests/utils_tests/test_autoreload.py @@ -9,6 +9,12 @@ LOCALE_PATH = os.path.join(os.path.dirname(__file__), 'locale') class TestFilenameGenerator(TestCase): + def setUp(self): + # Empty cached variables + from django.utils import autoreload + autoreload._cached_modules = set() + autoreload._cached_filenames = [] + def test_django_locales(self): """ Test that gen_filenames() also yields the built-in django locale files. @@ -64,3 +70,14 @@ class TestFilenameGenerator(TestCase): os.path.join(os.path.dirname(conf.__file__), 'locale', 'nl', 'LC_MESSAGES', 'django.mo'), filenames) + + def test_only_new_files(self): + """ + When calling a second time gen_filenames with only_new = True, only + files from newly loaded modules should be given. + """ + filenames1 = list(gen_filenames()) + from fractions import Fraction + filenames2 = list(gen_filenames(only_new=True)) + self.assertEqual(len(filenames2), 1) + self.assertTrue(filenames2[0].endswith('fractions.py'))