cache view lookups; see #1557

7 files changed, 102 insertions, 46 deletions

diff --git a/pyramid/config/__init__.py b/pyramid/config/__init__.py
index 401def208..549144fab 100644
--- a/pyramid/config/__init__.py
+++ b/pyramid/config/__init__.py
@@ -4,6 +4,7 @@ import logging
 import operator
 import os
 import sys
+import threading
 import venusian
 
 from webob.exc import WSGIHTTPException as WebobWSGIHTTPException
@@ -485,6 +486,15 @@ class Configurator(
                                                  info=info, event=event)
             _registry.registerSelfAdapter = registerSelfAdapter
 
+        if not hasattr(_registry, '_lock'):
+            _registry._lock = threading.Lock()
+
+        if not hasattr(_registry, '_clear_view_lookup_cache'):
+            def _clear_view_lookup_cache():
+                _registry._view_lookup_cache = {}
+            _registry._clear_view_lookup_cache = _clear_view_lookup_cache
+
+
     # API
 
     def _get_introspector(self):
diff --git a/pyramid/config/views.py b/pyramid/config/views.py
index aba28467d..4c927f5b5 100644
--- a/pyramid/config/views.py
+++ b/pyramid/config/views.py
@@ -1344,6 +1344,8 @@ class ViewsConfiguratorMixin(object):
                         multiview,
                         (IExceptionViewClassifier, request_iface, context),
                         IMultiView, name=name)
+
+            self.registry._clear_view_lookup_cache()
             renderer_type = getattr(renderer, 'type', None) # gard against None
             intrspc = self.introspector
             if (
diff --git a/pyramid/registry.py b/pyramid/registry.py
index 8c05940b9..fb4ec52e2 100644
--- a/pyramid/registry.py
+++ b/pyramid/registry.py
@@ -1,4 +1,5 @@
 import operator
+import threading
 
 from zope.interface import implementer
 
@@ -39,6 +40,17 @@ class Registry(Components, dict):
 
     _settings = None
 
+    def __init__(self, *arg, **kw):
+        # add a registry-instance-specific lock, which is used when the lookup
+        # cache is mutated
+        self._lock = threading.Lock()
+        # add a view lookup cache
+        self._clear_view_lookup_cache()
+        Components.__init__(self, *arg, **kw)
+
+    def _clear_view_lookup_cache(self):
+        self._view_lookup_cache = {}
+
     def __nonzero__(self):
         # defeat bool determination via dict.__len__
         return True
diff --git a/pyramid/router.py b/pyramid/router.py
index eac7f7976..9ce5d2487 100644
--- a/pyramid/router.py
+++ b/pyramid/router.py
@@ -138,47 +138,40 @@ class Router(object):
 
         # find a view callable
         context_iface = providedBy(context)
-        views_iter = _find_views(
+        view_callables = _find_views(
             registry,
             request.request_iface,
             context_iface,
             view_name,
             )
 
-        view_callable = next(views_iter, None)
-
-        # invoke the view callable
-        if view_callable is None:
-            if self.debug_notfound:
-                msg = (
-                    'debug_notfound of url %s; path_info: %r, '
-                    'context: %r, view_name: %r, subpath: %r, '
-                    'traversed: %r, root: %r, vroot: %r, '
-                    'vroot_path: %r' % (
-                        request.url, request.path_info, context,
-                        view_name, subpath, traversed, root, vroot,
-                        vroot_path)
-                    )
-                logger and logger.debug(msg)
-            else:
-                msg = request.path_info
-            raise HTTPNotFound(msg)
-        else:
+        pme = None
+
+        for view_callable in view_callables:
+            # look for views that meet the predicate criteria
             try:
                 response = view_callable(context, request)
-            except PredicateMismatch:
-                # look for other views that meet the predicate
-                # criteria
-                for view_callable in views_iter:
-                    if view_callable is not None:
-                        try:
-                            response = view_callable(context, request)
-                            break
-                        except PredicateMismatch:
-                            pass
-                else:
-                    raise
-        return response
+                return response
+            except PredicateMismatch as _pme:
+                pme = _pme
+
+        if pme is not None:
+            raise pme
+
+        if self.debug_notfound:
+            msg = (
+                'debug_notfound of url %s; path_info: %r, '
+                'context: %r, view_name: %r, subpath: %r, '
+                'traversed: %r, root: %r, vroot: %r, '
+                'vroot_path: %r' % (
+                    request.url, request.path_info, context,
+                    view_name, subpath, traversed, root, vroot,
+                    vroot_path)
+                )
+            logger and logger.debug(msg)
+        else:
+            msg = request.path_info
+        raise HTTPNotFound(msg)
 
     def invoke_subrequest(self, request, use_tweens=False):
         """Obtain a response object from the Pyramid application based on
diff --git a/pyramid/tests/test_config/test_init.py b/pyramid/tests/test_config/test_init.py
index 0ed04eb06..ec58e2466 100644
--- a/pyramid/tests/test_config/test_init.py
+++ b/pyramid/tests/test_config/test_init.py
@@ -343,6 +343,21 @@ class ConfiguratorTests(unittest.TestCase):
                          {'info': '', 'provided': 'provided',
                           'required': 'required', 'name': 'abc', 'event': True})
 
+    def test__fix_registry_adds__lock(self):
+        reg = DummyRegistry()
+        config = self._makeOne(reg)
+        config._fix_registry()
+        self.assertTrue(hasattr(reg, '_lock'))
+
+    def test__fix_registry_adds_clear_view_lookup_cache(self):
+        reg = DummyRegistry()
+        config = self._makeOne(reg)
+        self.assertFalse(hasattr(reg, '_clear_view_lookup_cache'))
+        config._fix_registry()
+        self.assertFalse(hasattr(reg, '_view_lookup_cache'))
+        reg._clear_view_lookup_cache()
+        self.assertEqual(reg._view_lookup_cache, {})
+
     def test_setup_registry_calls_fix_registry(self):
         reg = DummyRegistry()
         config = self._makeOne(reg)
diff --git a/pyramid/tests/test_registry.py b/pyramid/tests/test_registry.py
index 50f49f24d..c9dff5b22 100644
--- a/pyramid/tests/test_registry.py
+++ b/pyramid/tests/test_registry.py
@@ -12,6 +12,16 @@ class TestRegistry(unittest.TestCase):
         registry = self._makeOne()
         self.assertEqual(registry.__nonzero__(), True)
 
+    def test__lock(self):
+        registry = self._makeOne()
+        self.assertTrue(registry._lock)
+
+    def test_clear_view_cache_lookup(self):
+        registry = self._makeOne()
+        registry._view_lookup_cache[1] = 2
+        registry._clear_view_lookup_cache()
+        self.assertEqual(registry._view_lookup_cache, {})
+
     def test_package_name(self):
         package_name = 'testing'
         registry = self._getTargetClass()(package_name)
diff --git a/pyramid/view.py b/pyramid/view.py
index 4bd036428..8cde1d004 100644
--- a/pyramid/view.py
+++ b/pyramid/view.py
@@ -419,16 +419,30 @@ class forbidden_view_config(object):
 
 def _find_views(registry, request_iface, context_iface, view_name):
     registered = registry.adapters.registered
-    view_types = (IView, ISecuredView, IMultiView)
-    for req_type, ctx_type in itertools.product(
-        request_iface.__sro__, context_iface.__sro__
-    ):
-        source_ifaces = (IViewClassifier, req_type, ctx_type)
-        for view_type in view_types:
-            view_callable = registered(
-                source_ifaces,
-                view_type,
-                name=view_name,
-            )
-            if view_callable is not None:
-                yield view_callable
+    cache = registry._view_lookup_cache
+    views = cache.get((request_iface, context_iface, view_name))
+    if views is None:
+        views = []
+        view_types = (IView, ISecuredView, IMultiView)
+        for req_type, ctx_type in itertools.product(
+            request_iface.__sro__, context_iface.__sro__
+        ):
+            source_ifaces = (IViewClassifier, req_type, ctx_type)
+            for view_type in view_types:
+                view_callable = registered(
+                    source_ifaces,
+                    view_type,
+                    name=view_name,
+                )
+                if view_callable is not None:
+                    views.append(view_callable)
+        if views:
+            # do not cache view lookup misses.  rationale: dont allow cache to
+            # grow without bound if somebody tries to hit the site with many
+            # missing URLs.  we could use an LRU cache instead, but then
+            # purposeful misses by an attacker would just blow out the cache
+            # anyway. downside: misses will almost always consume more CPU than
+            # hits in steady state.
+            with registry._lock:
+                cache[(request_iface, context_iface, view_name)] = views
+    return iter(views)