Skip to content

Commit

Permalink
Use a descriptor for __module__
Browse files Browse the repository at this point in the history
This makes the rest of the attribute access fast again, but slows down
__module__.

+-------------------------------------------+------------+-------------------------------+
| Benchmark                                 | 38-master3 | 38-faster-descr               |
+===========================================+============+===============================+
| read __module__                           | 41.1 ns    | 123 ns: 2.99x slower (+199%)  |
+-------------------------------------------+------------+-------------------------------+
| read __name__                             | 41.3 ns    | 39.9 ns: 1.04x faster (-3%)   |
+-------------------------------------------+------------+-------------------------------+
| read __doc__                              | 41.8 ns    | 42.4 ns: 1.01x slower (+1%)   |
+-------------------------------------------+------------+-------------------------------+
| query adapter (no registrations)          | 3.85 ms    | 2.95 ms: 1.30x faster (-23%)  |
+-------------------------------------------+------------+-------------------------------+
| query adapter (all trivial registrations) | 4.59 ms    | 3.67 ms: 1.25x faster (-20%)  |
+-------------------------------------------+------------+-------------------------------+
| contains (empty dict)                     | 136 ns     | 54.8 ns: 2.48x faster (-60%)  |
+-------------------------------------------+------------+-------------------------------+
| contains (populated dict)                 | 137 ns     | 55.7 ns: 2.46x faster (-59%)  |
+-------------------------------------------+------------+-------------------------------+
| contains (populated list)                 | 40.2 us    | 2.86 us: 14.03x faster (-93%) |
+-------------------------------------------+------------+-------------------------------+

Not significant (1): read providedBy
  • Loading branch information
jamadden committed Mar 17, 2020
1 parent 3e54855 commit 5162b8d
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 48 deletions.
1 change: 1 addition & 0 deletions src/zope/interface/_compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,5 @@ def find_impl():
# name (for testing and documentation)
globs[name + 'Py'] = py_impl
globs[name + 'Fallback'] = py_impl

return c_impl
36 changes: 14 additions & 22 deletions src/zope/interface/_zope_interface_coptimizations.c
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,9 @@ IB_dealloc(IB* self)

static PyMemberDef IB_members[] = {
{"__name__", T_OBJECT_EX, offsetof(IB, __name__), 0, ""},
// The redundancy between __module__ and __ibmodule__ is because
// __module__ is often shadowed by subclasses.
{"__module__", T_OBJECT_EX, offsetof(IB, __module__), READONLY, ""},
{"__ibmodule__", T_OBJECT_EX, offsetof(IB, __module__), 0, ""},
{NULL}
};
Expand Down Expand Up @@ -936,32 +939,21 @@ IB_richcompare(IB* self, PyObject* other, int op)

}

static PyObject*
IB_getattro(IB* self, PyObject* name)
{
int cmpresult;
cmpresult = PyObject_RichCompareBool(name, str__module__, Py_EQ);
if (cmpresult == -1)
return NULL;

if (cmpresult) { // __module__
if (self->__module__) {
Py_INCREF(self->__module__);
return self->__module__;
}
}

// Wasn't __module__, *or* it was, but it was unset.
return PyObject_GenericGetAttr(OBJECT(self), name);
}

static int
IB_init(IB* self, PyObject* args, PyObject* kwargs)
{
static char *kwlist[] = {"__name__", "__module__", NULL};
PyObject* module = NULL;
PyObject* name = NULL;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OO:InterfaceBase.__init__", kwlist,
&name, &module)) {
return -1;
}
IB_clear(self);
self->__module__ = Py_None;
self->__module__ = module ? module : Py_None;
Py_INCREF(self->__module__);
self->__name__ = Py_None;
self->__name__ = name ? name : Py_None;
Py_INCREF(self->__name__);
return 0;
}
Expand All @@ -985,7 +977,7 @@ static PyTypeObject InterfaceBaseType = {
/* tp_hash */ (hashfunc)IB_hash,
/* tp_call */ (ternaryfunc)ib_call,
/* tp_str */ (reprfunc)0,
/* tp_getattro */ (getattrofunc)IB_getattro,
/* tp_getattro */ (getattrofunc)0,
/* tp_setattro */ (setattrofunc)0,
/* tp_as_buffer */ 0,
/* tp_flags */ Py_TPFLAGS_DEFAULT
Expand Down
89 changes: 65 additions & 24 deletions src/zope/interface/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import weakref

from zope.interface._compat import _use_c_impl
from zope.interface._compat import PYTHON3 as PY3
from zope.interface.exceptions import Invalid
from zope.interface.ro import ro

Expand Down Expand Up @@ -224,6 +225,56 @@ def __ge__(self, other):
return c >= 0


class _ModuleDescriptor(str):
# Descriptor for ``__module__``, used in InterfaceBase and subclasses.
#
# We store the module value in ``__ibmodule__`` and provide access
# to it under ``__module__`` through this descriptor. This is
# because we want to store ``__module__`` in the C structure (for
# speed of equality and sorting), but it's very hard to do
# that. Using PyMemberDef or PyGetSetDef (the C
# versions of properties) doesn't work without adding
# metaclasses: creating a new subclass puts a ``__module__``
# string in the class dict that overrides the descriptor that
# would access the C structure data.
#
# We must also preserve access to the *real* ``__module__`` of the
# class.
#
# Our solution is to watch for new subclasses and manually move
# this descriptor into them at creation time. We could use a
# metaclass, but this seems safer; using ``__getattribute__`` to
# alias the two imposed a 25% penalty on every attribute/method
# lookup, even when implemented in C.

# type.__repr__ accesses self.__dict__['__module__']
# and checks to see if it's a native string. If it's not,
# the repr just uses the __name__. So for things to work out nicely
# it's best for us to subclass str.
if PY3:
# Python 2 doesn't allow non-empty __slots__ for str
# subclasses.
__slots__ = ('_class_module',)

def __init__(self, class_module):
str.__init__(self)
self._class_module = class_module

def __get__(self, inst, kind):
if inst is None:
return self._class_module
return inst.__ibmodule__

def __set__(self, inst, val):
# Setting __module__ after construction is undefined. There are
# numerous things that cache based on it, either directly or indirectly.
# Nonetheless, it is allowed.
inst.__ibmodule__ = val

def __str__(self):
return self._class_module


@_use_c_impl
class InterfaceBase(NameAndModuleComparisonMixin, SpecificationBasePy):
"""Base class that wants to be replaced with a C base :)
Expand All @@ -232,36 +283,17 @@ class InterfaceBase(NameAndModuleComparisonMixin, SpecificationBasePy):
__slots__ = (
'__name__',
'__ibmodule__',
'_v_cached_hash',
)

def __init__(self, name=None, module=None):
# pylint:disable=assigning-non-slot
self.__name__ = name
# We store the module value in ``__ibmodule__`` and provide access
# to it under ``__module__`` through ``__getattribute__``. This is
# because we want to store __module__ in the C structure (for
# speed of equality and sorting), but it's very hard to do
# that any other way. Using PyMemberDef or PyGetSetDef (the C
# versions of properties) doesn't work without adding
# metaclasses: creating a new subclass puts a ``__module__``
# string in the class dict that overrides the descriptor that
# would access the C structure data.
#
# We could use a metaclass to override this behaviour, but it's probably
# safer to use ``__getattribute__``.
#
# Setting ``__module__`` after construction is undefined.
# There are numerous things that cache that value directly or
# indirectly (and long have).
self.__ibmodule__ = module

def _call_conform(self, conform):
raise NotImplementedError

def __getattribute__(self, name):
if name == '__module__':
return self.__ibmodule__
return object.__getattribute__(self, name)
__module__ = _ModuleDescriptor(__name__)

def __call__(self, obj, alternate=_marker):
"""Adapt an object to the interface
Expand Down Expand Up @@ -493,12 +525,18 @@ class InterfaceClass(InterfaceBase, Element, Specification):
#
#implements(IInterface)

def __new__(cls, *args, **kwargs):
if not isinstance(
cls.__dict__.get('__module__'),
_ModuleDescriptor):
cls.__module__ = _ModuleDescriptor(cls.__dict__['__module__'])
return super(InterfaceClass, cls).__new__(cls)

def __init__(self, name, bases=(), attrs=None, __doc__=None, # pylint:disable=redefined-builtin
__module__=None):
if not all(isinstance(base, InterfaceClass) for base in bases):
raise TypeError('Expected base interfaces')

InterfaceBase.__init__(self)
if attrs is None:
attrs = {}

Expand All @@ -515,8 +553,10 @@ def __init__(self, name, bases=(), attrs=None, __doc__=None, # pylint:disable=r
except (AttributeError, KeyError): # pragma: no cover
pass

self.__ibmodule__ = __module__
InterfaceBase.__init__(self, name, __module__)

assert '__module__' not in self.__dict__
assert self.__module__ == __module__, (self.__module__, __module__, self.__ibmodule__)
d = attrs.get('__doc__')
if d is not None:
if not isinstance(d, Attribute):
Expand Down Expand Up @@ -727,7 +767,8 @@ def __str__(self):
of = ''
if self.interface is not None:
of = self.interface.__module__ + '.' + self.interface.__name__ + '.'
return of + self.__name__ + self._get_str_info()
# self.__name__ may be None during construction (e.g., debugging)
return of + (self.__name__ or '<unknown>') + self._get_str_info()

def __repr__(self):
return "<%s.%s object at 0x%x %s>" % (
Expand Down
5 changes: 3 additions & 2 deletions src/zope/interface/tests/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -2343,9 +2343,10 @@ def _getTargetClass(self):
def _makeOne(self, component=None, factory=None):
from zope.interface.declarations import InterfaceClass

class IFoo(InterfaceClass):
class InterfaceClassSubclass(InterfaceClass):
pass
ifoo = IFoo('IFoo')

ifoo = InterfaceClassSubclass('IFoo')
class _Registry(object):
def __repr__(self):
return '_REGISTRY'
Expand Down

0 comments on commit 5162b8d

Please sign in to comment.