[Checkins] SVN: Zope3/trunk/src/zope/ Fixed a bug in the
adapter-registration cache management for
Jim Fulton
jim at zope.com
Fri Apr 28 14:07:26 EDT 2006
Log message for revision 67729:
Fixed a bug in the adapter-registration cache management for
persistent registries. Unfortunately, the fix takes away about half of
my request-per-second performance win. :(
Changed:
U Zope3/trunk/src/zope/app/component/tests/test_registration.py
U Zope3/trunk/src/zope/component/persistentregistry.py
U Zope3/trunk/src/zope/component/tests.py
U Zope3/trunk/src/zope/interface/adapter.py
-=-
Modified: Zope3/trunk/src/zope/app/component/tests/test_registration.py
===================================================================
--- Zope3/trunk/src/zope/app/component/tests/test_registration.py 2006-04-28 17:57:47 UTC (rev 67728)
+++ Zope3/trunk/src/zope/app/component/tests/test_registration.py 2006-04-28 18:07:25 UTC (rev 67729)
@@ -27,12 +27,16 @@
import transaction
import persistent
+import zope.component.globalregistry
import zope.component.testing as placelesssetup
from zope.testing import doctest
from zope.app.testing import setup
import zope.app.container.contained
from zope import interface
+import zope.app.component.site
+
+
# test class for testing data conversion
class IFoo(interface.Interface):
pass
@@ -42,6 +46,9 @@
def __init__(self, name=''):
self.name = name
+ def __repr__(self):
+ return 'Foo(%r)' % self.name
+
def setUpOld(test):
placelesssetup.setUp(test)
setup.setUpAnnotations()
@@ -330,6 +337,75 @@
"""
+
+class GlobalRegistry:
+ pass
+
+base = zope.component.globalregistry.GlobalAdapterRegistry(
+ GlobalRegistry, 'adapters')
+GlobalRegistry.adapters = base
+def clear_base():
+ base.__init__(GlobalRegistry, 'adapters')
+
+
+def test_deghostification_of_persistent_adapter_registries():
+ """
+
+Note that this test duplicates one from zope.component.tests.
+We should be able to get rid of this one when we get rid of
+__setstate__ implementation we have in back35.
+
+We want to make sure that we see updates corrextly.
+
+ >>> import ZODB.tests.util
+ >>> db = ZODB.tests.util.DB()
+ >>> tm1 = transaction.TransactionManager()
+ >>> c1 = db.open(transaction_manager=tm1)
+ >>> r1 = zope.app.component.site._LocalAdapterRegistry((base,))
+ >>> r2 = zope.app.component.site._LocalAdapterRegistry((r1,))
+ >>> c1.root()[1] = r1
+ >>> c1.root()[2] = r2
+ >>> tm1.commit()
+ >>> r1._p_deactivate()
+ >>> r2._p_deactivate()
+
+ >>> tm2 = transaction.TransactionManager()
+ >>> c2 = db.open(transaction_manager=tm2)
+ >>> r1 = c2.root()[1]
+ >>> r2 = c2.root()[2]
+
+ >>> r1.lookup((), IFoo, '')
+
+ >>> base.register((), IFoo, '', Foo(''))
+ >>> r1.lookup((), IFoo, '')
+ Foo('')
+
+ >>> r2.lookup((), IFoo, '1')
+
+ >>> r1.register((), IFoo, '1', Foo('1'))
+
+ >>> r2.lookup((), IFoo, '1')
+ Foo('1')
+
+ >>> r1.lookup((), IFoo, '2')
+ >>> r2.lookup((), IFoo, '2')
+
+ >>> base.register((), IFoo, '2', Foo('2'))
+
+ >>> r1.lookup((), IFoo, '2')
+ Foo('2')
+
+ >>> r2.lookup((), IFoo, '2')
+ Foo('2')
+
+Cleanup:
+
+ >>> db.close()
+ >>> clear_base()
+
+ """
+
+
def test_suite():
suite = unittest.TestSuite((
doctest.DocFileSuite('deprecated35_statusproperty.txt',
Modified: Zope3/trunk/src/zope/component/persistentregistry.py
===================================================================
--- Zope3/trunk/src/zope/component/persistentregistry.py 2006-04-28 17:57:47 UTC (rev 67728)
+++ Zope3/trunk/src/zope/component/persistentregistry.py 2006-04-28 18:07:25 UTC (rev 67729)
@@ -21,14 +21,28 @@
import zope.component.registry
-class PersistentAdapterRegistry(zope.interface.adapter.AdapterRegistry,
- persistent.Persistent):
+class PersistentAdapterRegistry(
+ zope.interface.adapter.VerifyingAdapterRegistry,
+ persistent.Persistent,
+ ):
def changed(self, originally_changed):
if originally_changed is self:
self._p_changed = True
super(PersistentAdapterRegistry, self).changed(originally_changed)
+
+ def __getstate__(self):
+ state = super(PersistentAdapterRegistry, self).__getstate__().copy()
+ for name in self._delegated:
+ state.pop(name, 0)
+ return state
+
+ def __setstate__(self, state):
+ super(PersistentAdapterRegistry, self).__setstate__(state)
+ self._createLookup()
+ self._v_lookup.changed(self)
+
class PersistentComponents(zope.component.registry.Components):
def _init_registries(self):
Modified: Zope3/trunk/src/zope/component/tests.py
===================================================================
--- Zope3/trunk/src/zope/component/tests.py 2006-04-28 17:57:47 UTC (rev 67728)
+++ Zope3/trunk/src/zope/component/tests.py 2006-04-28 18:07:25 UTC (rev 67729)
@@ -17,6 +17,9 @@
"""
import re
import unittest
+import transaction
+import persistent
+
from zope import interface, component
from zope.interface.verify import verifyObject
from zope.interface.interfaces import IInterface
@@ -26,6 +29,8 @@
from zope.component.interfaces import IComponentArchitecture
from zope.component.interfaces import IComponentLookup
from zope.component.testing import setUp, tearDown
+import zope.component.persistentregistry
+import zope.component.globalregistry
# side effect gets component-based event dispatcher installed.
# we should obviously make this more explicit
@@ -837,6 +842,89 @@
"""
+
+
+class GlobalRegistry:
+ pass
+
+base = zope.component.globalregistry.GlobalAdapterRegistry(
+ GlobalRegistry, 'adapters')
+GlobalRegistry.adapters = base
+def clear_base():
+ base.__init__(GlobalRegistry, 'adapters')
+
+class IFoo(interface.Interface):
+ pass
+class Foo(persistent.Persistent):
+ interface.implements(IFoo)
+ name = ''
+ def __init__(self, name=''):
+ self.name = name
+
+ def __repr__(self):
+ return 'Foo(%r)' % self.name
+
+def test_deghostification_of_persistent_adapter_registries():
+ """
+
+We want to make sure that we see updates corrextly.
+
+ >>> len(base._v_subregistries)
+ 0
+
+ >>> import ZODB.tests.util
+ >>> db = ZODB.tests.util.DB()
+ >>> tm1 = transaction.TransactionManager()
+ >>> c1 = db.open(transaction_manager=tm1)
+ >>> r1 = zope.component.persistentregistry.PersistentAdapterRegistry(
+ ... (base,))
+ >>> r2 = zope.component.persistentregistry.PersistentAdapterRegistry((r1,))
+ >>> c1.root()[1] = r1
+ >>> c1.root()[2] = r2
+ >>> tm1.commit()
+ >>> r1._p_deactivate()
+
+ >>> len(base._v_subregistries)
+ 0
+
+ >>> tm2 = transaction.TransactionManager()
+ >>> c2 = db.open(transaction_manager=tm2)
+ >>> r1 = c2.root()[1]
+ >>> r2 = c2.root()[2]
+
+ >>> r1.lookup((), IFoo, '')
+
+ >>> base.register((), IFoo, '', Foo(''))
+ >>> r1.lookup((), IFoo, '')
+ Foo('')
+
+ >>> r2.lookup((), IFoo, '1')
+
+ >>> r1.register((), IFoo, '1', Foo('1'))
+
+ >>> r2.lookup((), IFoo, '1')
+ Foo('1')
+
+ >>> r1.lookup((), IFoo, '2')
+ >>> r2.lookup((), IFoo, '2')
+
+ >>> base.register((), IFoo, '2', Foo('2'))
+
+ >>> r1.lookup((), IFoo, '2')
+ Foo('2')
+
+ >>> r2.lookup((), IFoo, '2')
+ Foo('2')
+
+Cleanup:
+
+ >>> db.close()
+ >>> clear_base()
+
+ """
+
+
+
def tearDownRegistryTests(tests):
import zope.event
zope.event.subscribers.pop()
Modified: Zope3/trunk/src/zope/interface/adapter.py
===================================================================
--- Zope3/trunk/src/zope/interface/adapter.py 2006-04-28 17:57:47 UTC (rev 67728)
+++ Zope3/trunk/src/zope/interface/adapter.py 2006-04-28 18:07:25 UTC (rev 67729)
@@ -19,105 +19,72 @@
import weakref
from zope.interface import providedBy, Interface, ro
-class readproperty(object):
+_marker = object
+class BaseAdapterRegistry(object):
- def __init__(self, func):
- self.func = func
+ # List of methods copied from lookup sub-objects:
+ _delegated = ('lookup', 'queryMultiAdapter', 'lookup1', 'queryAdapter',
+ 'adapter_hook', 'lookupAll', 'names',
+ 'subscriptions', 'subscribers')
- def __get__(self, inst, class_):
- if inst is None:
- return self
+ # All registries maintain a generation that can be used by verifying
+ # registries
+ _generation = 0
- func = self.func
- return func(inst)
-
+ def __init__(self, bases=()):
-_delegated = ('lookup', 'queryMultiAdapter', 'lookup1', 'queryAdapter',
- 'adapter_hook', 'lookupAll', 'names',
- 'subscriptions', 'subscribers')
+ # {order -> {required -> {provided -> {name -> value}}}}
+ # where "interfaces" is really a nested key. So, for example:
+ # for order == 0, we have:
+ # {provided -> {name -> valie}}
+ # but for order == 2, we have:
+ # {r1 -> {r2 -> {provided -> {name -> valie}}}}
+ self._adapters = []
-_marker = object
-class AdapterRegistry(object):
+ # {order -> {required -> {provided -> {name -> [value]}}}}
+ # where the remarks about adapters above apply
+ self._subscribers = []
- def __init__(self, bases=()):
- self._adapters = []
- self._subscribers = []
+ # Set, with a reference count, keeping track of the interfaces
+ # for which we have provided components:
self._provided = {}
- self._init_non_persistent()
- self.__bases__ = bases
- def _init_non_persistent(self):
- self._v_subregistries = weakref.WeakKeyDictionary()
- self._v_lookup = lookup = AdapterLookup(self)
- for name in _delegated:
- self.__dict__[name] = getattr(lookup, name)
+ # Looup object to perform lookup. We make this a separate object to
+ # to make it easier, in the furture, to implement just the lookup
+ # functionality in C.
+ self._createLookup()
- def __getstate__(self):
- state = super(AdapterRegistry, self).__getstate__().copy()
- for name in _delegated:
- state.pop(name, 0)
- return state
+ # Cache invalidation data. There are really 2 kinds of registries:
+
+ # Invalidating registries have caches that are invalidated
+ # when they or when base registies change. An invalidating
+ # registry can only have invalidating registries as bases.
- def __setstate__(self, state):
- super(AdapterRegistry, self).__setstate__(state)
- self._init_non_persistent()
+ # Verifying registies can't rely on getting invalidation message,
+ # so have to check the generations of base registries to determine
+ # if their cache data are current
- @apply
- def __bases__():
+ # Base registries:
+ self.__bases__ = bases
- def get(self):
- return self.__dict__['__bases__']
+ def _setBases(self, bases):
+ self.__dict__['__bases__'] = bases
+ self.ro = ro.ro(self)
+ self.changed(self)
- def set(self, v):
- old = self.__dict__.get('__bases__', ())
- for r in old:
- if r not in v:
- r._removeSubregistry(self)
- for r in v:
- if r not in old:
- r._addSubregistry(self)
-
- self.__dict__['__bases__'] = v
- self.ro = ro.ro(self)
- self.changed(self)
-
- return property(get, set)
+ __bases__ = property(lambda self: self.__dict__['__bases__'],
+ lambda self, bases: self._setBases(bases),
+ )
- def _addSubregistry(self, r):
- self._v_subregistries[r] = 1
+ def _createLookup(self):
+ self._v_lookup = self.LookupClass(self)
+ for name in self._delegated:
+ self.__dict__[name] = getattr(self._v_lookup, name)
- def _removeSubregistry(self, r):
- if r in self._v_subregistries:
- del self._v_subregistries[r]
-
def changed(self, originally_changed):
- try:
- lookup = self._v_lookup
- except AttributeError:
- pass
- else:
- lookup.changed(originally_changed)
+ self._generation += 1
+ self._v_lookup.changed(originally_changed)
- for sub in self._v_subregistries.keys():
- sub.changed(originally_changed)
-
- @readproperty
- def _v_extendors(self):
- _v_extendors = {}
- for provided in self._provided:
- for i in provided.__iro__:
- extendors = _v_extendors.get(i, ())
- if provided not in extendors:
- _v_extendors[i] = (
- [e for e in extendors if provided.isOrExtends(e)]
- +
- [provided]
- +
- [e for e in extendors if not provided.isOrExtends(e)]
- )
- self._v_extendors = _v_extendors
- return self._v_extendors
-
def register(self, required, provided, name, value):
if value is None:
self.unregister(required, provided, name, value)
@@ -146,8 +113,8 @@
n = self._provided.get(provided, 0) + 1
self._provided[provided] = n
- if n == 1 and '_v_extendors' in self.__dict__:
- del self.__dict__['_v_extendors']
+ if n == 1:
+ self._v_lookup.add_extendor(provided)
self.changed(self)
@@ -195,8 +162,7 @@
n = self._provided[provided] - 1
if n == 0:
del self._provided[provided]
- if '_v_extendors' in self.__dict__:
- del self.__dict__['_v_extendors']
+ self._v_lookup.remove_extendor(provided)
else:
self._provided[provided] = n
@@ -227,8 +193,8 @@
if provided is not None:
n = self._provided.get(provided, 0) + 1
self._provided[provided] = n
- if n == 1 and '_v_extendors' in self.__dict__:
- del self.__dict__['_v_extendors']
+ if n == 1:
+ self._v_lookup.add_extendor(provided)
self.changed(self)
@@ -265,8 +231,7 @@
n = self._provided[provided] + len(new) - len(old)
if n == 0:
del self._provided[provided]
- if '_v_extendors' in self.__dict__:
- del self.__dict__['_v_extendors']
+ self._v_lookup.remove_extendor(provided)
self.changed(self)
@@ -278,8 +243,6 @@
return XXXTwistedFakeOut
-
-
_not_in_mapping = object()
class AdapterLookup(object):
@@ -289,8 +252,9 @@
self._mcache = {}
self._scache = {}
self._required = {}
+ self.init_extendors()
- def changed(self, originally_changed):
+ def changed(self, ignored=None):
self._cache.clear()
self._mcache.clear()
self._scache.clear()
@@ -299,6 +263,53 @@
if r is not None:
r.unsubscribe(self)
self._required.clear()
+
+
+ # Extendors
+ # ---------
+
+ # When given an target interface for an adapter lookup, we need to consider
+ # adapters for interfaces that extend the target interface. This is
+ # what the extendors dictionary is about. It tells us all of the
+ # interfaces that extend an interface for which there are adapters
+ # registered.
+
+ # We could separate this by order and name, thus reducing the
+ # number of provided interfaces to search at run time. The tradeoff,
+ # however, is that we have to store more information. For example,
+ # is the same interface is provided for multiple names and if the
+ # interface extends many interfaces, we'll have to keep track of
+ # a fair bit of information for each name. It's better to
+ # be space efficient here and be time efficient in the cache
+ # implementation.
+
+ # TODO: add invalidation when a provided interface changes, in case
+ # the interface's __iro__ has changed. This is unlikely enough that
+ # we'll take our chances for now.
+
+ def init_extendors(self):
+ self._extendors = {}
+ for p in self._registry._provided:
+ self.add_extendor(p)
+
+ def add_extendor(self, provided):
+ _extendors = self._extendors
+ for i in provided.__iro__:
+ extendors = _extendors.get(i, ())
+ _extendors[i] = (
+ [e for e in extendors if provided.isOrExtends(e)]
+ +
+ [provided]
+ +
+ [e for e in extendors if not provided.isOrExtends(e)]
+ )
+
+ def remove_extendor(self, provided):
+ _extendors = self._extendors
+ for i in provided.__iro__:
+ _extendors[i] = [e for e in _extendors.get(i, ())
+ if e != provided]
+
def _getcache(self, provided, name):
cache = self._cache.get(provided)
@@ -336,7 +347,7 @@
if order >= len(byorder):
continue
- extendors = registry._v_extendors.get(provided)
+ extendors = registry._v_lookup._extendors.get(provided)
if not extendors:
continue
@@ -412,7 +423,7 @@
byorder = registry._adapters
if order >= len(byorder):
continue
- extendors = registry._v_extendors.get(provided)
+ extendors = registry._v_lookup._extendors.get(provided)
if not extendors:
continue
components = byorder[order]
@@ -445,7 +456,7 @@
if provided is None:
extendors = (provided, )
else:
- extendors = registry._v_extendors.get(provided)
+ extendors = registry._v_lookup._extendors.get(provided)
if extendors is None:
continue
@@ -470,7 +481,84 @@
if subscriber is not None:
result.append(subscriber)
return result
+
+class AdapterRegistry(BaseAdapterRegistry):
+
+ LookupClass = AdapterLookup
+
+ def __init__(self, bases=()):
+ # AdapterRegisties are invalidating registries, so
+ # we need to keep track of out invalidating subregistries.
+ self._v_subregistries = weakref.WeakKeyDictionary()
+
+ super(AdapterRegistry, self).__init__(bases)
+
+ def _addSubregistry(self, r):
+ self._v_subregistries[r] = 1
+
+ def _removeSubregistry(self, r):
+ if r in self._v_subregistries:
+ del self._v_subregistries[r]
+
+ def _setBases(self, bases):
+ old = self.__dict__.get('__bases__', ())
+ for r in old:
+ if r not in bases:
+ r._removeSubregistry(self)
+ for r in bases:
+ if r not in old:
+ r._addSubregistry(self)
+
+ super(AdapterRegistry, self)._setBases(bases)
+
+ def changed(self, originally_changed):
+ super(AdapterRegistry, self).changed(originally_changed)
+
+ for sub in self._v_subregistries.keys():
+ sub.changed(originally_changed)
+
+
+class VerifyingAdapterLookup(AdapterLookup):
+
+ def __init__(self, registry):
+ super(VerifyingAdapterLookup, self).__init__(registry)
+
+ def changed(self, originally_changed):
+ super(VerifyingAdapterLookup, self).changed(originally_changed)
+ self._verify_ro = self._registry.ro[1:]
+ self._verify_generations = [r._generation for r in self._verify_ro]
+
+ def _verify(self):
+ if ([r._generation for r in self._verify_ro]
+ != self._verify_generations):
+ self.changed(None)
+
+ def _getcache(self, provided, name):
+ self._verify()
+
+ # The non-use of super here is intentional. A C implementation
+ # will call AdapterLookup's C implementation directly.
+ return AdapterLookup._getcache(self, provided, name)
+ def lookupAll(self, required, provided):
+ self._verify()
+
+ # The non-use of super here is intentional. A C implementation
+ # will call AdapterLookup's C implementation directly.
+ return AdapterLookup.lookupAll(self, required, provided)
+
+ def subscriptions(self, required, provided):
+ self._verify()
+
+ # The non-use of super here is intentional. A C implementation
+ # will call AdapterLookup's C implementation directly.
+ return AdapterLookup.subscriptions(self, required, provided)
+
+
+class VerifyingAdapterRegistry(BaseAdapterRegistry):
+
+ LookupClass = VerifyingAdapterLookup
+
def _convert_None_to_Interface(x):
if x is None:
return Interface
More information about the Checkins
mailing list