From 79f4255b4146be95eab25a738a8b51773f417a78 Mon Sep 17 00:00:00 2001 From: Malthe Borch Date: Fri, 26 Oct 2012 14:32:48 +0200 Subject: [PATCH] Fixed schema caching. Previously, a non-persistent counter would be used as part of the cache key, and changes made to this counter in one process would obviously not propagate to other processes. Instead, the cache key now includes the schema and subtypes which are both retrieved from a FTI-specific volatile cache that uses the modification time as its cache key. Note that the tests are failing in this particular changeset. It's a little tedious to instruct the mocker to respond naturally to the volatile caching attempts. Maybe the passthrough-mode can be used, combined with a single test case that examines the cache invariant. --- docs/HISTORY.txt | 9 ++++ plone/dexterity/content.py | 97 +++++++++++++++------------------ plone/dexterity/schema.py | 106 ++++++++++++++++++++----------------- 3 files changed, 109 insertions(+), 103 deletions(-) diff --git a/docs/HISTORY.txt b/docs/HISTORY.txt index c51b411..1555236 100644 --- a/docs/HISTORY.txt +++ b/docs/HISTORY.txt @@ -4,6 +4,15 @@ Changelog 2.0.1 (unreleased) ------------------ +- Fixed schema caching. Previously, a non-persistent counter would be + used as part of the cache key, and changes made to this counter in + one process would obviously not propagate to other processes. + + Instead, the cache key now includes the schema and subtypes which + are both retrieved from a FTI-specific volatile cache that uses the + modification time as its cache key. + [malthe] + - The default attribute accessor now also looks through subtypes (behaviors) to find a field default. [malthe] diff --git a/plone/dexterity/content.py b/plone/dexterity/content.py index 2c6912b..e922541 100644 --- a/plone/dexterity/content.py +++ b/plone/dexterity/content.py @@ -44,6 +44,7 @@ from plone.autoform.interfaces import READ_PERMISSIONS_KEY from plone.supermodel.utils import mergedTaggedValueDict from plone.dexterity.filerepresentation import DAVResourceMixin, DAVCollectionMixin +from plone.dexterity.interfaces import IDexterityFTI _marker = object() @@ -57,69 +58,57 @@ class FTIAwareSpecification(ObjectSpecificationDescriptor): if inst is None: return getObjectSpecification(cls) - # Find the cached value. This calculation is expensive and called - # hundreds of times during each request, so we require a fast cache - cache = getattr(inst, '_v__providedBy__', None) - # Find the data we need to know if our cache needs to be invalidated - direct_spec = getattr(inst, '__provides__', None) portal_type = getattr(inst, 'portal_type', None) - fti_counter = -1 - if portal_type is not None: - fti_counter = SCHEMA_CACHE.counter(portal_type) - - # See if we have a valid cache. Reasons to do this include: - # - # - We don't have a portal_type yet, so we can't have found the schema - # - The FTI was modified, and the schema cache invalidated globally. - # The fti_counter will have advanced. - # - The instance was modified and persisted since the cache was built. - # - The instance now has a different __provides__, which means that someone - # called directlyProvides/alsoProvides on it. - - if cache is not None and portal_type is not None: - cached_mtime, cached_fti_counter, cached_direct_spec, cached_spec = cache - - if inst._p_mtime == cached_mtime and \ - fti_counter == cached_fti_counter and \ - direct_spec is cached_direct_spec: - return cached_spec - - # We don't have a cache, so we need to build a new spec and maybe cache it - spec = direct_spec - + # If the instance doesn't have a __provides__ attribute, get the # interfaces implied by the class as a starting point. if spec is None: spec = implementedBy(cls) - - # Add the schema from the FTI and behavior subtypes - - dynamically_provided = [] - - if portal_type is not None: - schema = SCHEMA_CACHE.get(portal_type) - if schema is not None: - dynamically_provided.append(schema) - - subtypes = SCHEMA_CACHE.subtypes(portal_type) - if subtypes: - dynamically_provided.extend(subtypes) - - # If we have any dynamically provided interface, prepend them to the spec - # and cache. We can't cache until we have at least the schema, because - # it's possible that we were called before traversal and so could not - # find the schema yet. - - if dynamically_provided: - dynamically_provided.append(spec) - spec = Implements(*dynamically_provided) - - inst._v__providedBy__ = inst._p_mtime, SCHEMA_CACHE.counter(portal_type), direct_spec, spec - + + # If the instance has no portal type, then we're done. + if portal_type is None: + return spec + + fti = queryUtility(IDexterityFTI, name=portal_type) + if fti is None: + return spec + + schema = SCHEMA_CACHE.get(portal_type) + subtypes = SCHEMA_CACHE.subtypes(portal_type) + + # Find the cached value. This calculation is expensive and called + # hundreds of times during each request, so we require a fast cache + cache = getattr(inst, '_v__providedBy__', None) + updated = inst._p_mtime, schema, subtypes, direct_spec + + # See if we have a valid cache. Reasons to do this include: + # + # - The schema was modified. + # - The subtypes were modified. + # - The instance was modified and persisted since the cache was built. + # - The instance has a different direct specification. + if cache is not None: + cached_mtime, cached_schema, cached_subtypes, \ + cached_direct_spec, cached_spec = cache + + if cache[:-1] == updated: + return cached_spec + + dynamically_provided = [] if schema is None else [schema] + dynamically_provided.extend(subtypes) + + # If we have neither a schema, nor a subtype, then we're also done. + if not dynamically_provided: + return spec + + dynamically_provided.append(spec) + spec = Implements(*dynamically_provided) + inst._v__providedBy__ = updated + (spec, ) + return spec diff --git a/plone/dexterity/schema.py b/plone/dexterity/schema.py index eee4e29..cd56ab7 100644 --- a/plone/dexterity/schema.py +++ b/plone/dexterity/schema.py @@ -1,4 +1,5 @@ import new +import functools from threading import RLock from plone.synchronize import synchronized @@ -7,7 +8,7 @@ from zope.interface import implements, alsoProvides from zope.interface.interface import InterfaceClass from zope.component import adapter -from zope.component import queryUtility +from zope.component import queryUtility, getAllUtilitiesRegisteredFor from plone.behavior.interfaces import IBehavior @@ -28,7 +29,32 @@ from plone.alterego import dynamic generated = dynamic.create('plone.dexterity.schema.generated') transient = new.module("transient") -# Schema cache +def invalidate_cache(fti): + fti._p_activate() + fti.__dict__.pop('_v_schema_get', None) + fti.__dict__.pop('_v_schema_subtypes', None) + + +def volatile(func): + @functools.wraps(func) + def decorator(self, portal_type): + fti = queryUtility(IDexterityFTI, name=portal_type) + if fti is not None: + key = '_v_schema_%s' % func.__name__ + cache = getattr(fti, key, None) + if cache is not None: + mtime, value = cache + if fti._p_mtime == mtime: + return value + + value = func(self, fti) + + if fti is not None and value is not None: + setattr(fti, key, (fti._p_mtime, value)) + + return value + return decorator + class SchemaCache(object): """Simple schema cache. @@ -50,62 +76,44 @@ class SchemaCache(object): >>> from plone.dexterity.schema import SCHEMA_CACHE >>> my_schema = SCHEMA_CACHE.get(portal_type) - Invalidate the cache by calling invalidate() (for one portal_type) or - clear() (for all cached values), or simply raise a SchemaInvalidatedEvent. + The cache uses the FTI's modification time as its invariant. """ lock = RLock() - cache = {} - subtypes_cache = {} - counter_values = {} @synchronized(lock) - def get(self, portal_type): - cached = self.cache.get(portal_type, None) - if cached is None: - fti = queryUtility(IDexterityFTI, name=portal_type) - if fti is not None: - try: - cached = self.cache[portal_type] = fti.lookupSchema() - except (AttributeError, ValueError): - pass - return cached - + @volatile + def get(self, fti): + if fti is None: + return + try: + return fti.lookupSchema() + except (AttributeError, ValueError): + pass + @synchronized(lock) - def subtypes(self, portal_type): - cached = self.subtypes_cache.get(portal_type, None) - if cached is None: - subtypes = [] - fti = queryUtility(IDexterityFTI, name=portal_type) - if fti is None: - return () - for behavior_name in fti.behaviors: - behavior = queryUtility(IBehavior, name=behavior_name) - if behavior is not None and behavior.marker is not None: - subtypes.append(behavior.marker) - cached = self.subtypes_cache[portal_type] = tuple(subtypes) - return cached - - @synchronized(lock) - def counter(self, portal_type): - counter = self.counter_values.get(portal_type, None) - if counter is None: - counter = self.counter_values[portal_type] = 0 - return counter - - @synchronized(lock) - def invalidate(self, portal_type): - self.cache[portal_type] = None - self.subtypes_cache[portal_type] = None - if portal_type in self.counter_values: - self.counter_values[portal_type] += 1 - else: - self.counter_values[portal_type] = 0 + @volatile + def subtypes(self, fti): + if fti is None: + return () + subtypes = [] + for behavior_name in fti.behaviors: + behavior = queryUtility(IBehavior, name=behavior_name) + if behavior is not None and behavior.marker is not None: + subtypes.append(behavior.marker) + return tuple(subtypes) @synchronized(lock) def clear(self): - self.cache.clear() - self.subtypes_cache.clear() + for fti in getAllUtilitiesRegisteredFor(IDexterityFTI): + invalidate_cache(fti) + + + @synchronized(lock) + def invalidate(self, portal_type): + fti = queryUtility(IDexterityFTI, name=portal_type) + invalidate_cache(fti) + SCHEMA_CACHE = SchemaCache()