Skip to content

Commit

Permalink
cache view lookups; see Pylons#1557
Browse files Browse the repository at this point in the history
  • Loading branch information
mcdonc committed Apr 3, 2015
1 parent b589096 commit c15cbce
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 46 deletions.
10 changes: 10 additions & 0 deletions pyramid/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import operator
import os
import sys
import threading
import venusian

from webob.exc import WSGIHTTPException as WebobWSGIHTTPException
Expand Down Expand Up @@ -485,6 +486,15 @@ def registerSelfAdapter(required=None, provided=None,
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):
Expand Down
2 changes: 2 additions & 0 deletions pyramid/config/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1344,6 +1344,8 @@ def regclosure():
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 (
Expand Down
12 changes: 12 additions & 0 deletions pyramid/registry.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import operator
import threading

from zope.interface import implementer

Expand Down Expand Up @@ -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
Expand Down
59 changes: 26 additions & 33 deletions pyramid/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,47 +138,40 @@ def handle_request(self, request):

# 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
Expand Down
15 changes: 15 additions & 0 deletions pyramid/tests/test_config/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,21 @@ def test__fix_registry_registerSelfAdapter(self):
{'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)
Expand Down
10 changes: 10 additions & 0 deletions pyramid/tests/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ def test___nonzero__(self):
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)
Expand Down
40 changes: 27 additions & 13 deletions pyramid/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,16 +419,30 @@ def callback(context, name, ob):

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)

0 comments on commit c15cbce

Please sign in to comment.