Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #155 from zopefoundation/slots
More memory optimizations for a 6-7% reduction.
  • Loading branch information
jamadden committed Jan 27, 2020
2 parents eeaacb6 + 2c35908 commit fec4a51
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 53 deletions.
25 changes: 19 additions & 6 deletions CHANGES.rst
Expand Up @@ -11,19 +11,32 @@
value forces the Python implementation to be used, ignoring the C
extensions. See `PR 151 <https://github.com/zopefoundation/zope.interface/pull/151>`_.

- Cache the result of ``__hash__`` method in ``InterfaceClass`` as a
speed optimization. The method is called very often (i.e several
hundred thousand times during Plone 5.2 startup). Because the hash value never
changes it can be cached. This improves test performance from 0.614s
down to 0.575s (1.07x faster). In a real world Plone case a reindex
index came down from 402s to 320s (1.26x faster). See `PR 156
<https://github.com/zopefoundation/zope.interface/pull/156>`_.

- Change the C classes ``SpecificationBase`` and its subclass
``ClassProvidesBase`` to store implementation attributes in their structures
instead of their instance dictionaries. This eliminates the use of
an undocumented private C API function, and helps make some
instances require less memory. See `PR 154 <https://github.com/zopefoundation/zope.interface/pull/154>`_.

- Performance optimization of ``__hash__`` method on ``InterfaceClass``.
The method is called very often (i.e several 100.000 times on a Plone 5.2
startup). Because the hash value never changes it can be cached.
This improves test performance from 0.614s down to 0.575s (1.07x faster).
In a real world Plone case a reindex index came down from 402s to 320s (1.26x faster).
See `PR 156 <https://github.com/zopefoundation/zope.interface/pull/156>`_.
- Reduce memory usage in other ways based on observations of usage
patterns in Zope (3) and Plone code bases.

- Specifications with no dependents are common (more than 50%) so
avoid allocating a ``WeakKeyDictionary`` unless we need it.
- Likewise, tagged values are relatively rare, so don't allocate a
dictionary to hold them until they are used.
- Use ``__slots___`` or the C equivalent ``tp_members`` in more
common places. See `PR 155 <https://github.com/zopefoundation/zope.interface/pull/155>`_.

The changes in this release resulted in a 7% memory reduction after
loading about 6,000 modules that define about 2,200 interfaces.

4.7.1 (2019-11-11)
==================
Expand Down
34 changes: 32 additions & 2 deletions src/zope/interface/_zope_interface_coptimizations.c
Expand Up @@ -259,6 +259,7 @@ providedBy(PyObject *ignored, PyObject *ob)

typedef struct {
PyObject_HEAD
PyObject* weakreflist;
/*
In the past, these fields were stored in the __dict__
and were technically allowed to contain any Python object, though
Expand All @@ -267,6 +268,15 @@ typedef struct {
make any assumptions about contents.
*/
PyObject* _implied;
/*
The remainder aren't used in C code but must be stored here
to prevent instance layout conflicts.
*/
PyObject* _dependents;
PyObject* _bases;
PyObject* _v_attrs;
PyObject* __iro__;
PyObject* __sro__;
} Spec;

/*
Expand All @@ -277,19 +287,30 @@ static int
Spec_traverse(Spec* self, visitproc visit, void* arg)
{
Py_VISIT(self->_implied);
Py_VISIT(self->_dependents);
Py_VISIT(self->_v_attrs);
Py_VISIT(self->__iro__);
Py_VISIT(self->__sro__);
return 0;
}

static int
Spec_clear(Spec* self)
{
Py_CLEAR(self->_implied);
Py_CLEAR(self->_dependents);
Py_CLEAR(self->_v_attrs);
Py_CLEAR(self->__iro__);
Py_CLEAR(self->__sro__);
return 0;
}

static void
Spec_dealloc(Spec* self)
{
if (self->weakreflist != NULL) {
PyObject_ClearWeakRefs(OBJECT(self));
}
Spec_clear(self);
Py_TYPE(self)->tp_free(OBJECT(self));
}
Expand Down Expand Up @@ -387,7 +408,12 @@ static struct PyMethodDef Spec_methods[] = {

static PyMemberDef Spec_members[] = {
{"_implied", T_OBJECT_EX, offsetof(Spec, _implied), 0, ""},
{NULL}
{"_dependents", T_OBJECT_EX, offsetof(Spec, _dependents), 0, ""},
{"_bases", T_OBJECT_EX, offsetof(Spec, _bases), 0, ""},
{"_v_attrs", T_OBJECT_EX, offsetof(Spec, _v_attrs), 0, ""},
{"__iro__", T_OBJECT_EX, offsetof(Spec, __iro__), 0, ""},
{"__sro__", T_OBJECT_EX, offsetof(Spec, __sro__), 0, ""},
{NULL},
};


Expand Down Expand Up @@ -417,7 +443,7 @@ static PyTypeObject SpecType = {
/* tp_traverse */ (traverseproc)Spec_traverse,
/* tp_clear */ (inquiry)Spec_clear,
/* tp_richcompare */ (richcmpfunc)0,
/* tp_weaklistoffset */ (long)0,
/* tp_weaklistoffset */ offsetof(Spec, weakreflist),
/* tp_iter */ (getiterfunc)0,
/* tp_iternext */ (iternextfunc)0,
/* tp_methods */ Spec_methods,
Expand Down Expand Up @@ -1779,3 +1805,7 @@ PyInit__zope_interface_coptimizations(void)
return init();
}
#endif

#ifdef __clang__
#pragma clang diagnostic pop
#endif
22 changes: 11 additions & 11 deletions src/zope/interface/declarations.py
Expand Up @@ -62,16 +62,11 @@ def __call__(self, ob):
class Declaration(Specification):
"""Interface declarations"""

__slots__ = ()

def __init__(self, *interfaces):
Specification.__init__(self, _normalizeargs(interfaces))

def changed(self, originally_changed):
Specification.changed(self, originally_changed)
try:
del self._v_attrs
except AttributeError:
pass

def __contains__(self, interface):
"""Test whether an interface is in the specification
"""
Expand Down Expand Up @@ -625,9 +620,12 @@ def noLongerProvides(object, interface):


@_use_c_impl
class ClassProvidesBase(object):
# In C, this extends SpecificationBase, so its kind of weird here that it
# doesn't.
class ClassProvidesBase(SpecificationBase):

__slots__ = (
'_cls',
'_implements',
)

def __get__(self, inst, cls):
if cls is self._cls:
Expand Down Expand Up @@ -916,6 +914,8 @@ def _normalizeargs(sequence, output=None):

return output

_empty = Declaration()
# XXX: Declarations are mutable, allowing adjustments to their __bases__
# so having one as a singleton may not be a great idea.
_empty = Declaration() # type: Declaration

objectSpecificationDescriptor = ObjectSpecificationDescriptor()
100 changes: 74 additions & 26 deletions src/zope/interface/interface.py
Expand Up @@ -67,7 +67,9 @@ def __init__(self, __name__, __doc__=''):

self.__name__ = __name__
self.__doc__ = __doc__
self.__tagged_values = {}
# Tagged values are rare, especially on methods or attributes.
# Deferring the allocation can save substantial memory.
self.__tagged_values = None

def getName(self):
""" Returns the name of the object. """
Expand All @@ -79,26 +81,48 @@ def getDoc(self):

def getTaggedValue(self, tag):
""" Returns the value associated with 'tag'. """
if not self.__tagged_values:
raise KeyError(tag)
return self.__tagged_values[tag]

def queryTaggedValue(self, tag, default=None):
""" Returns the value associated with 'tag'. """
return self.__tagged_values.get(tag, default)
return self.__tagged_values.get(tag, default) if self.__tagged_values else default

def getTaggedValueTags(self):
""" Returns a list of all tags. """
return self.__tagged_values.keys()
return self.__tagged_values.keys() if self.__tagged_values else ()

def setTaggedValue(self, tag, value):
""" Associates 'value' with 'key'. """
if self.__tagged_values is None:
self.__tagged_values = {}
self.__tagged_values[tag] = value


@_use_c_impl
class SpecificationBase(object):

# This object is the base of the inheritance hierarchy for ClassProvides:
#
# ClassProvides < ClassProvidesBase, Declaration
# Declaration < Specification < SpecificationBase
# ClassProvidesBase < SpecificationBase
#
# In order to have compatible instance layouts, we need to declare
# the storage used by Specification and Declaration here (and
# those classes must have ``__slots__ = ()``); fortunately this is
# not a waste of space because those are the only two inheritance
# trees. These all translate into tp_members in C.
__slots__ = (
# Things used here.
'_implied',
# Things used in Specification.
'_dependents',
'_bases',
'_v_attrs',
'__iro__',
'__sro__',
'__weakref__',
)

def providedBy(self, ob):
Expand Down Expand Up @@ -128,6 +152,11 @@ class InterfaceBase(object):
"""Base class that wants to be replaced with a C base :)
"""

__slots__ = ()

def _call_conform(self, conform):
raise NotImplementedError

def __call__(self, obj, alternate=_marker):
"""Adapt an object to the interface
"""
Expand Down Expand Up @@ -173,53 +202,72 @@ class Specification(SpecificationBase):
Specifications are mutable. If you reassign their bases, their
relations with other specifications are adjusted accordingly.
"""
__slots__ = ()

# Copy some base class methods for speed
isOrExtends = SpecificationBase.isOrExtends
providedBy = SpecificationBase.providedBy

def __init__(self, bases=()):
# There are many leaf interfaces with no dependents,
# and a few with very many. It's a heavily left-skewed
# distribution. In a survey of Plone and Zope related packages
# that loaded 2245 InterfaceClass objects and 2235 ClassProvides
# instances, there were a total of 7000 Specification objects created.
# 4700 had 0 dependents, 1400 had 1, 382 had 2 and so on. Only one
# for <type> had 1664. So there's savings to be had deferring
# the creation of dependents.
self._dependents = None # type: weakref.WeakKeyDictionary
self._bases = ()
self._implied = {}
self.dependents = weakref.WeakKeyDictionary()
self._v_attrs = None
self.__iro__ = ()
self.__sro__ = ()

self.__bases__ = tuple(bases)

@property
def dependents(self):
if self._dependents is None:
self._dependents = weakref.WeakKeyDictionary()
return self._dependents

def subscribe(self, dependent):
self.dependents[dependent] = self.dependents.get(dependent, 0) + 1
self._dependents[dependent] = self.dependents.get(dependent, 0) + 1

def unsubscribe(self, dependent):
n = self.dependents.get(dependent, 0) - 1
try:
n = self._dependents[dependent]
except TypeError:
raise KeyError(dependent)
n -= 1
if not n:
del self.dependents[dependent]
elif n > 0:
self.dependents[dependent] = n
else:
raise KeyError(dependent)
assert n > 0
self.dependents[dependent] = n

def __setBases(self, bases):
# Remove ourselves as a dependent of our old bases
for b in self.__bases__:
b.unsubscribe(self)

# Register ourselves as a dependent of our bases
self.__dict__['__bases__'] = bases
self._bases = bases
for b in bases:
b.subscribe(self)

self.changed(self)

__bases__ = property(

lambda self: self.__dict__.get('__bases__', ()),
lambda self: self._bases,
__setBases,
)

def changed(self, originally_changed):
"""We, or something we depend on, have changed
"""
try:
del self._v_attrs
except AttributeError:
pass
self._v_attrs = None

implied = self._implied
implied.clear()
Expand All @@ -242,9 +290,13 @@ def changed(self, originally_changed):
implied[ancestor] = ()

# Now, advise our dependents of change:
for dependent in tuple(self.dependents.keys()):
for dependent in tuple(self._dependents.keys() if self._dependents else ()):
dependent.changed(originally_changed)

# Just in case something called get() at some point
# during that process and we have a cycle of some sort
# make sure we didn't cache incomplete results.
self._v_attrs = None

def interfaces(self):
"""Return an iterator for the interfaces in the specification.
Expand All @@ -256,7 +308,6 @@ def interfaces(self):
seen[interface] = 1
yield interface


def extends(self, interface, strict=True):
"""Does the specification extend the given interface?
Expand All @@ -274,9 +325,8 @@ def weakref(self, callback=None):
def get(self, name, default=None):
"""Query for an attribute description
"""
try:
attrs = self._v_attrs
except AttributeError:
attrs = self._v_attrs
if attrs is None:
attrs = self._v_attrs = {}
attr = attrs.get(name)
if attr is None:
Expand All @@ -286,10 +336,8 @@ def get(self, name, default=None):
attrs[name] = attr
break

if attr is None:
return default
else:
return attr
return default if attr is None else attr


class InterfaceClass(Element, InterfaceBase, Specification):
"""Prototype (scarecrow) Interfaces Implementation."""
Expand Down

0 comments on commit fec4a51

Please sign in to comment.