Merge pull request #57 from NextThought/issue52-refactor

Use the shared interface cache to speed up modified notifications

setup.py

@@ -76,12 +76,16 @@ def _c(m):
             ('internalization.legacy_factories', ()),
             ('internalization.factories', ()),
             ('internalization.fields', ()),
-            ('internalization.events', ()),
+            ('internalization.events', ('_interface_cache',)),
             ('internalization.externals', ()),
             ('internalization.updater', ()),
             ('externalization', ('_base_interfaces',)),
-            ('datastructures', ('_base_interfaces', 'externalization',
-                                'internalization')),
+            ('_interface_cache', ()),
+            ('datastructures', (
+                '_base_interfaces',
+                '_interface_cache',
+                'externalization',
+                'internalization')),
     ):
         deps = ([_py_source(mod) for mod in deps]
                 + [_pxd(mod) for mod in deps]

src/nti/externalization/__interface_cache.pxd

@@ -0,0 +1,20 @@
+import cython
+
+cdef cache_instances
+
+@cython.final
+@cython.internal
+@cython.freelist(1000)
+cdef class InterfaceCache(object):
+    cdef __weakref__
+    cdef iface
+    cdef frozenset ext_all_possible_keys
+    cdef ext_accept_external_id
+    cdef frozenset ext_primitive_out_ivars
+    cdef dict modified_event_attributes
+
+@cython.locals(x=InterfaceCache)
+cdef _cache_cleanUp(instances)
+
+cpdef InterfaceCache cache_for(externalizer, ext_self)
+cpdef InterfaceCache cache_for_key(key, ext_self)

src/nti/externalization/_datastructures.pxd

@@ -15,6 +15,8 @@ from nti.externalization.__base_interfaces cimport StandardInternalFields as SIF
 
 from nti.externalization.internalization._fields cimport validate_named_field_value
 
+from nti.externalization.__interface_cache cimport cache_for
+
 cdef IInternalObjectIO
 cdef SEF StandardExternalFields
 cdef SIF StandardInternalFields
@@ -72,19 +74,5 @@ cdef class ModuleScopedInterfaceObjectIO(InterfaceObjectIO):
     )
     cpdef _ext_schemas_to_consider(self, ext_self)
 
-@cython.final
-@cython.internal
-@cython.freelist(1000)
-cdef class _InterfaceCache(object):
-    cdef __weakref__
-    cdef iface
-    cdef frozenset ext_all_possible_keys
-    cdef ext_accept_external_id
-    cdef frozenset ext_primitive_out_ivars
-
-
-cdef _InterfaceCache _cache_for(externalizer, ext_self)
-@cython.locals(x=_InterfaceCache)
-cdef _cache_cleanUp()
 
 cdef tuple _primitives

src/nti/externalization/_interface_cache.py

@@ -0,0 +1,84 @@
+# cython: auto_pickle=False,embedsignature=True,always_allow_keywords=False
+# -*- coding: utf-8 -*-
+"""
+A cache based on the interfaces provided by an object.
+
+"""
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+
+from weakref import WeakSet
+
+from zope.interface import providedBy
+
+cache_instances = WeakSet()
+
+
+class InterfaceCache(object):
+    # Although having a __dict__ would be more convenient,
+    # since this object is used from multiple modules,
+    # Cython code is much more effective if the fields are
+    # defined in the .pxd --- which translate to slots
+    # in Python
+
+    __slots__ = (
+        'iface',
+        'ext_all_possible_keys',
+        'ext_accept_external_id',
+        'ext_primitive_out_ivars',
+        'modified_event_attributes',
+        '__weakref__'
+    )
+
+    def __init__(self):
+        self.iface = None
+        self.ext_all_possible_keys = None
+        self.ext_accept_external_id = None
+        self.ext_primitive_out_ivars = None
+        self.modified_event_attributes = {}
+
+
+
+def cache_for_key(key, ext_self):
+    # The Declaration objects maintain a _v_attrs that
+    # gets blown away on changes to themselves or their
+    # dependents, including adding interfaces dynamically to an instance
+    # (In that case, the provided object actually gets reset)
+    cache_place = providedBy(ext_self)
+    try:
+        attrs = cache_place._v_attrs # pylint:disable=protected-access
+    except AttributeError:
+        attrs = cache_place._v_attrs = {}
+
+    try:
+        cache = attrs[key]
+    except KeyError:
+        cache = InterfaceCache()
+        attrs[key] = cache
+        cache_instances.add(cache)
+
+    return cache
+
+
+def cache_for(externalizer, ext_self):
+    return cache_for_key(type(externalizer), ext_self)
+
+
+def _cache_cleanUp(instances):
+    for x in list(instances):
+        x.__init__()
+
+
+try:
+    from zope.testing import cleanup
+except ImportError: # pragma: no cover
+    pass
+else:
+    cleanup.addCleanUp(_cache_cleanUp, args=(cache_instances,))
+
+
+# pylint:disable=wrong-import-position
+from nti.externalization._compat import import_c_accel
+import_c_accel(globals(), 'nti.externalization.__interface_cache')

src/nti/externalization/datastructures.py

@@ -15,7 +15,7 @@
 
 # stdlib imports
 import numbers
-from weakref import WeakSet
+
 
 import six
 from six import iteritems
@@ -41,6 +41,8 @@
 from ._base_interfaces import get_standard_internal_fields
 from ._base_interfaces import NotGiven
 
+from ._interface_cache import cache_for
+
 StandardExternalFields = get_standard_external_fields()
 StandardInternalFields = get_standard_internal_fields()
 
@@ -299,63 +301,17 @@ def _ext_setattr(self, ext_self, k, v):
         setattr(ext_self, k, v)
 
     def _ext_accept_update_key(self, k, ext_self, ext_keys):
-        sup = super(ExternalizableInstanceDict, self)
-        return (sup._ext_accept_update_key(k, ext_self, ext_keys)
-                or (self._update_accepts_type_attrs and hasattr(ext_self, k)))
+        return (
+            super(ExternalizableInstanceDict, self)._ext_accept_update_key(k, ext_self, ext_keys)
+            or (self._update_accepts_type_attrs and hasattr(ext_self, k))
+        )
 
     __repr__ = make_repr()
 
 
 
 _primitives = six.string_types + (numbers.Number, bool)
 
-# TODO: Refactor into a module and share with internalization.py
-class _InterfaceCache(object):
-    __slots__ = ('iface', 'ext_all_possible_keys',
-                 'ext_accept_external_id',
-                 'ext_primitive_out_ivars',
-                 '__weakref__')
-
-    def __init__(self):
-        self.iface = None
-        self.ext_all_possible_keys = None
-        self.ext_accept_external_id = None
-        self.ext_primitive_out_ivars = None
-
-_cache_instances = WeakSet()
-
-def _cache_for(externalizer, ext_self):
-    # The Declaration objects maintain a _v_attrs that
-    # gets blown away on changes to themselves or their
-    # dependents, including adding interfaces dynamically to an instance
-    # (In that case, the provided object actually gets reset)
-    cache_place = interface.providedBy(ext_self)
-    try:
-        attrs = cache_place._v_attrs
-    except AttributeError:
-        attrs = cache_place._v_attrs = {}
-    key = type(externalizer)
-    if key in attrs:
-        cache = attrs[key]
-    else:
-        cache = _InterfaceCache()
-        attrs[key] = cache
-        _cache_instances.add(cache)
-    return cache
-
-
-def _cache_cleanUp():
-    for x in list(_cache_instances):
-        x.__init__()
-
-try:
-    from zope.testing import cleanup
-except ImportError: # pragma: no cover
-    pass
-else:
-    cleanup.addCleanUp(_cache_cleanUp)
-
-
 
 class InterfaceObjectIO(AbstractDynamicObjectIO):
     """
@@ -397,7 +353,7 @@ def __init__(self, ext_self, iface_upper_bound=None, validate_after_update=True)
         self._ext_self = ext_self
         # Cache all of this data that we use. It's required often and, if not quite a bottleneck,
         # does show up in the profiling data
-        cache = _cache_for(self, ext_self)
+        cache = cache_for(self, ext_self)
         if cache.iface is None:
             cache.iface = self._ext_find_schema(
                 ext_self,
@@ -445,7 +401,7 @@ def _ext_replacement(self):
         return self._ext_self
 
     def _ext_all_possible_keys(self):
-        cache = _cache_for(self, self._ext_self)
+        cache = cache_for(self, self._ext_self)
         if cache.ext_all_possible_keys is None:
             iface = self._iface
             is_method = interface.interfaces.IMethod.providedBy
@@ -470,7 +426,7 @@ def _ext_accept_external_id(self, ext_self, parsed):
         this will return that value; otherwise, returns false.
         """
         __traceback_info__ = ext_self, parsed,
-        cache = _cache_for(self, ext_self)
+        cache = cache_for(self, ext_self)
         if cache.ext_accept_external_id is None:
             try:
                 field = cache.iface['id']
@@ -583,5 +539,6 @@ def _ext_schemas_to_consider(self, ext_self):
                 if x.__module__ == search_module_name
                 and not x.queryTaggedValue('_ext_is_marker_interface')]
 
+# pylint:disable=wrong-import-position,wrong-import-order
 from nti.externalization._compat import import_c_accel
 import_c_accel(globals(), 'nti.externalization._datastructures')

src/nti/externalization/internalization/_events.pxd

@@ -1,7 +1,7 @@
 # definitions for internalization.py
 import cython
 
-
+from nti.externalization.__interface_cache cimport cache_for_key
 
 # imports
 cdef providedBy
@@ -15,8 +15,10 @@ cdef class _Attributes(object):
     cdef public interface
     cdef public set attributes
 
+
 @cython.locals(
-    attrs=_Attributes,
+    attrs=dict,
+    attributes=_Attributes,
 )
 cdef _make_modified_attributes(containedObject, external_keys)
 

src/nti/externalization/internalization/events.py

@@ -17,7 +17,7 @@
 from zope.lifecycleevent import IAttributes
 
 from nti.externalization.interfaces import ObjectModifiedFromExternalEvent
-
+from nti.externalization._interface_cache import cache_for_key
 
 logger = __import__('logging').getLogger(__name__)
 
@@ -41,26 +41,59 @@ def __init__(self, iface):
 
 classImplements(_Attributes, IAttributes)
 
+
 def _make_modified_attributes(containedObject, external_keys):
-    # TODO: Share the interface cache from datastructures.
-    # {iface -> _Attributes(iface)}
-    attributes = {}
+    cache = cache_for_key(_Attributes, containedObject)
+    # {'attrname': Interface}
+    attr_to_iface = cache.modified_event_attributes
+
+    # Ensure that each attribute name in *names* is
+    # in the _v_attrs dict of *provides*. Also
+    # makes sure that self.attr_to_iface maps each
+    # name to its interface.
+    # Returns a sequence of fresh _Attributes objects.
     provides = providedBy(containedObject)
-    get_iface = provides.get
-    for k in external_keys:
-        iface_providing_attr = None
-        iface_attr = get_iface(k)
-        if iface_attr is not None:
-            iface_providing_attr = iface_attr.interface
-
-        try:
-            attrs = attributes[iface_providing_attr]
-        except KeyError:
-            attrs = attributes[iface_providing_attr] = _Attributes(iface_providing_attr)
-
-        attrs.attributes.add(k)
+    # {iface -> _Attributes(iface)}
+    result = {}
+    # By the time we get here, we're guaranteed that
+    # _v_attrs exists---it's where we're cached
+    attrs = provides._v_attrs # pylint:disable=protected-access
+
+    for name in external_keys:
+        # This is the core loop of provides.get()
+        # (z.i.declaration.Declaration.get)
+        attr = attrs.get(name, cache)
+        # Steady state, misses are rare here.
+        if attr is cache:
+            for iface in provides.__iro__:
+                attr = iface.direct(name)
+                if attr is not None:
+                    attrs[name] = attr
+                    break
+            else:
+                # Not part of any interface
+                attr = attrs[name] = None
+
+        providing_iface = attr_to_iface.get(name, cache)
+        if providing_iface is cache:
+            # In the steady state, misses should be rare here.
+            providing_iface = None
+            if attr is not None:
+                providing_iface = attr.interface
+            attr_to_iface[name] = providing_iface
+
+
+
+        attributes = result.get(providing_iface)
+        if attributes is None:
+            # Misses aren't unexpected here. often attributes
+            # are spread across multiple interfaces
+            attributes = result[providing_iface] = _Attributes(providing_iface)
+
+        attributes.attributes.add(name)
+
+    return result.values()
 
-    return attributes.values()
 
 def _make_modified_event(containedObject, externalObject, updater,
                          attributes, kwargs):
@@ -69,7 +102,7 @@ def _make_modified_event(containedObject, externalObject, updater,
     # Let the updater have its shot at modifying the event, too, adding
     # interfaces or attributes. (Note: this was added to be able to provide
     # sharedWith information on the event, since that makes for a better stream.
-    # If that use case expands, revisit this interface.
+    # If that use case expands, revisit this interface.)
     # XXX: Document and test this.
     try:
         meth = updater._ext_adjust_modified_event # pylint:disable=protected-access