Skip to content

Commit

Permalink
Avoid use of a metaclass by implementeng __getattribute__.
Browse files Browse the repository at this point in the history
This is pretty, but it slows down all attribute access to interfaces.
By up to 25%. I'm not sure that's acceptable for things like
Interface.providedBy.

+-------------------------------------------+------------+-------------------------------+
| Benchmark                                 | 38-master3 | 38-faster3                    |
+===========================================+============+===============================+
| read __module__                           | 41.1 ns    | 44.3 ns: 1.08x slower (+8%)   |
+-------------------------------------------+------------+-------------------------------+
| read __name__                             | 41.3 ns    | 51.6 ns: 1.25x slower (+25%)  |
+-------------------------------------------+------------+-------------------------------+
| read __doc__                              | 41.8 ns    | 53.3 ns: 1.28x slower (+28%)  |
+-------------------------------------------+------------+-------------------------------+
| read providedBy                           | 56.7 ns    | 71.6 ns: 1.26x slower (+26%)  |
+-------------------------------------------+------------+-------------------------------+
| query adapter (no registrations)          | 3.85 ms    | 2.95 ms: 1.31x faster (-23%)  |
+-------------------------------------------+------------+-------------------------------+
| query adapter (all trivial registrations) | 4.59 ms    | 3.65 ms: 1.26x faster (-20%)  |
+-------------------------------------------+------------+-------------------------------+
| contains (empty dict)                     | 136 ns     | 55.4 ns: 2.45x faster (-59%)  |
+-------------------------------------------+------------+-------------------------------+
| contains (populated dict)                 | 137 ns     | 55.0 ns: 2.49x faster (-60%)  |
+-------------------------------------------+------------+-------------------------------+
| contains (populated list)                 | 40.2 us    | 2.95 us: 13.62x faster (-93%) |
+-------------------------------------------+------------+-------------------------------+
  • Loading branch information
jamadden committed Mar 11, 2020
1 parent f7f035e commit c575695
Show file tree
Hide file tree
Showing 5 changed files with 268 additions and 213 deletions.
50 changes: 49 additions & 1 deletion benchmarks/micro.py
Expand Up @@ -27,7 +27,7 @@ def make_implementer(iface):
for implementer in implementers
]

INNER = 10
INNER = 100

def bench_in(loops, o):
t0 = pyperf.perf_counter()
Expand All @@ -50,8 +50,56 @@ def bench_query_adapter(loops, components):
components.queryAdapter(provider, iface)
return pyperf.perf_counter() - t0

def bench_getattr(loops, name, get=getattr):
t0 = pyperf.perf_counter()
for _ in range(loops):
for _ in range(INNER):
get(Interface, name) # 1
get(Interface, name) # 2
get(Interface, name) # 3
get(Interface, name) # 4
get(Interface, name) # 5
get(Interface, name) # 6
get(Interface, name) # 7
get(Interface, name) # 8
get(Interface, name) # 9
get(Interface, name) # 10
return pyperf.perf_counter() - t0

runner = pyperf.Runner()

# TODO: Need benchmarks of adaptation, etc, using interface inheritance.
# TODO: Need benchmarks of sorting (e.g., putting in a BTree)
# TODO: Need those same benchmarks for implementedBy/Implements objects.

runner.bench_time_func(
'read __module__', # stored in C, accessed through __getattribute__
bench_getattr,
'__module__',
inner_loops=INNER * 10
)

runner.bench_time_func(
'read __name__', # stored in C, accessed through PyMemberDef
bench_getattr,
'__name__',
inner_loops=INNER * 10
)

runner.bench_time_func(
'read __doc__', # stored in Python instance dictionary directly
bench_getattr,
'__doc__',
inner_loops=INNER * 10
)

runner.bench_time_func(
'read providedBy', # from the class, wrapped into a method object.
bench_getattr,
'providedBy',
inner_loops=INNER * 10
)

runner.bench_time_func(
'query adapter (no registrations)',
bench_query_adapter,
Expand Down
63 changes: 33 additions & 30 deletions src/zope/interface/_zope_interface_coptimizations.c
Expand Up @@ -47,7 +47,8 @@ static PyObject *str_uncached_lookup, *str_uncached_lookupAll;
static PyObject *str_uncached_subscriptions;
static PyObject *str_registry, *strro, *str_generation, *strchanged;
static PyObject *str__self__;

static PyObject *str__module__;
static PyObject *str__name__;

static PyTypeObject *Implements;

Expand Down Expand Up @@ -97,7 +98,7 @@ import_declarations(void)
}


static PyTypeObject SpecType; /* Forward */
static PyTypeObject SpecificationBaseType; /* Forward */

static PyObject *
implementedByFallback(PyObject *cls)
Expand Down Expand Up @@ -177,7 +178,7 @@ getObjectSpecification(PyObject *ignored, PyObject *ob)
PyObject *cls, *result;

result = PyObject_GetAttr(ob, str__provides__);
if (result != NULL && PyObject_TypeCheck(result, &SpecType))
if (result != NULL && PyObject_TypeCheck(result, &SpecificationBaseType))
return result;


Expand Down Expand Up @@ -225,7 +226,7 @@ providedBy(PyObject *ignored, PyObject *ob)
because we may have a proxy, so we'll just try to get the
only attribute.
*/
if (PyObject_TypeCheck(result, &SpecType)
if (PyObject_TypeCheck(result, &SpecificationBaseType)
||
PyObject_HasAttr(result, strextends)
)
Expand Down Expand Up @@ -378,7 +379,7 @@ Spec_providedBy(PyObject *self, PyObject *ob)
if (decl == NULL)
return NULL;

if (PyObject_TypeCheck(decl, &SpecType))
if (PyObject_TypeCheck(decl, &SpecificationBaseType))
item = Spec_extends((Spec*)decl, self);
else
/* decl is probably a security proxy. We have to go the long way
Expand All @@ -405,7 +406,7 @@ Spec_implementedBy(PyObject *self, PyObject *cls)
if (decl == NULL)
return NULL;

if (PyObject_TypeCheck(decl, &SpecType))
if (PyObject_TypeCheck(decl, &SpecificationBaseType))
item = Spec_extends((Spec*)decl, self);
else
item = PyObject_CallFunctionObjArgs(decl, self, NULL);
Expand Down Expand Up @@ -438,7 +439,7 @@ static PyMemberDef Spec_members[] = {
};


static PyTypeObject SpecType = {
static PyTypeObject SpecificationBaseType = {
PyVarObject_HEAD_INIT(NULL, 0)
/* tp_name */ "_interface_coptimizations."
"SpecificationBase",
Expand Down Expand Up @@ -617,7 +618,7 @@ static PyTypeObject CPBType = {
/* tp_methods */ 0,
/* tp_members */ CPB_members,
/* tp_getset */ 0,
/* tp_base */ &SpecType,
/* tp_base */ &SpecificationBaseType,
/* tp_dict */ 0, /* internal use */
/* tp_descr_get */ (descrgetfunc)CPB_descr_get,
/* tp_descr_set */ 0,
Expand Down Expand Up @@ -654,7 +655,7 @@ __adapt__(PyObject *self, PyObject *obj)
if (decl == NULL)
return NULL;

if (PyObject_TypeCheck(decl, &SpecType))
if (PyObject_TypeCheck(decl, &SpecificationBaseType))
{
PyObject *implied;

Expand Down Expand Up @@ -817,7 +818,6 @@ IB_dealloc(IB* self)

static PyMemberDef IB_members[] = {
{"__name__", T_OBJECT_EX, offsetof(IB, __name__), 0, ""},
{"__module__", T_OBJECT_EX, offsetof(IB, __module__), 0, ""},
{"__ibmodule__", T_OBJECT_EX, offsetof(IB, __module__), 0, ""},
{NULL}
};
Expand Down Expand Up @@ -918,6 +918,7 @@ IB_richcompare(IB* self, PyObject* other, int op)
else if (result == 1) {
result = PyObject_RichCompareBool(self->__module__, othermod, op);
}
// If either comparison failed, we have an error set.
if (result == -1) {
goto cleanup;
}
Expand All @@ -936,18 +937,22 @@ IB_richcompare(IB* self, PyObject* other, int op)
}

static PyObject*
IB_module_get(IB* self, void* context)
IB_getattro(IB* self, PyObject* name)
{
return self->__module__;
}
int cmpresult;
cmpresult = PyObject_RichCompareBool(name, str__module__, Py_EQ);
if (cmpresult == -1)
return NULL;

static int
IB_module_set(IB* self, PyObject* value, void* context)
{
Py_XINCREF(value);
Py_XDECREF(self->__module__);
self->__module__ = value;
return 0;
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
Expand All @@ -961,10 +966,6 @@ IB_init(IB* self, PyObject* args, PyObject* kwargs)
return 0;
}

static PyGetSetDef IB_getsets[] = {
{"__module__", (getter)IB_module_get, (setter)IB_module_set, 0, NULL},
{NULL}
};

static PyTypeObject InterfaceBaseType = {
PyVarObject_HEAD_INIT(NULL, 0)
Expand All @@ -984,7 +985,7 @@ static PyTypeObject InterfaceBaseType = {
/* tp_hash */ (hashfunc)IB_hash,
/* tp_call */ (ternaryfunc)ib_call,
/* tp_str */ (reprfunc)0,
/* tp_getattro */ (getattrofunc)0,
/* tp_getattro */ (getattrofunc)IB_getattro,
/* tp_setattro */ (setattrofunc)0,
/* tp_as_buffer */ 0,
/* tp_flags */ Py_TPFLAGS_DEFAULT
Expand All @@ -998,8 +999,8 @@ static PyTypeObject InterfaceBaseType = {
/* tp_iternext */ (iternextfunc)0,
/* tp_methods */ ib_methods,
/* tp_members */ IB_members,
/* tp_getset */ IB_getsets,
/* tp_base */ &SpecType,
/* tp_getset */ 0,
/* tp_base */ &SpecificationBaseType,
/* tp_dict */ 0,
/* tp_descr_get */ 0,
/* tp_descr_set */ 0,
Expand Down Expand Up @@ -1958,14 +1959,16 @@ init(void)
DEFINE_STRING(ro);
DEFINE_STRING(changed);
DEFINE_STRING(__self__);
DEFINE_STRING(__name__);
DEFINE_STRING(__module__);
#undef DEFINE_STRING
adapter_hooks = PyList_New(0);
if (adapter_hooks == NULL)
return NULL;

/* Initialize types: */
SpecType.tp_new = PyBaseObject_Type.tp_new;
if (PyType_Ready(&SpecType) < 0)
SpecificationBaseType.tp_new = PyBaseObject_Type.tp_new;
if (PyType_Ready(&SpecificationBaseType) < 0)
return NULL;
OSDType.tp_new = PyBaseObject_Type.tp_new;
if (PyType_Ready(&OSDType) < 0)
Expand Down Expand Up @@ -1997,7 +2000,7 @@ init(void)
return NULL;

/* Add types: */
if (PyModule_AddObject(m, "SpecificationBase", OBJECT(&SpecType)) < 0)
if (PyModule_AddObject(m, "SpecificationBase", OBJECT(&SpecificationBaseType)) < 0)
return NULL;
if (PyModule_AddObject(m, "ObjectSpecificationDescriptor",
(PyObject *)&OSDType) < 0)
Expand Down
72 changes: 24 additions & 48 deletions src/zope/interface/declarations.py
Expand Up @@ -36,6 +36,7 @@ class implements (that instances of the class provides).
from zope.interface.interface import InterfaceClass
from zope.interface.interface import SpecificationBase
from zope.interface.interface import Specification
from zope.interface.interface import NameAndModuleComparisonMixin
from zope.interface._compat import CLASS_TYPES as DescriptorAwareMetaClasses
from zope.interface._compat import PYTHON3
from zope.interface._compat import _use_c_impl
Expand Down Expand Up @@ -197,7 +198,29 @@ def weakref(self, callback=None):
#
# These specify interfaces implemented by instances of classes

class Implements(Declaration):
class Implements(NameAndModuleComparisonMixin,
Declaration):
# Inherit from NameAndModuleComparisonMixin to be
# mutually comparable with InterfaceClass objects.
# (The two must be mutually comparable to be able to work in e.g., BTrees.)
# Instances of this class generally don't have a __module__ other than
# `zope.interface.declarations`, whereas they *do* have a __name__ that is the
# fully qualified name of the object they are representing.

# Note, though, that equality and hashing are still identity based. This
# accounts for things like nested objects that have the same name (typically
# only in tests) and is consistent with pickling. As far as comparisons to InterfaceClass
# goes, we'll never have equal name and module to those, so we're still consistent there.
# Instances of this class are essentially intended to be unique and are
# heavily cached (note how our __reduce__ handles this) so having identity
# based hash and eq should also work.

# We want equality and hashing to be based on identity. However, we can't actually
# implement __eq__/__ne__ to do this because sometimes we get wrapped in a proxy.
# We need to let the proxy types implement these methods so they can handle unwrapping
# and then rely on: (1) the interpreter automatically changing `implements == proxy` into
# `proxy == implements` (which will call proxy.__eq__ to do the unwrapping) and then
# (2) the default equality and hashing semantics being identity based.

# class whose specification should be used as additional base
inherit = None
Expand Down Expand Up @@ -233,53 +256,6 @@ def __repr__(self):
def __reduce__(self):
return implementedBy, (self.inherit, )

def __cmp(self, other):
# Yes, I did mean to name this __cmp, rather than __cmp__.
# It is a private method used by __lt__ and __gt__.
# This is based on, and compatible with, InterfaceClass.
# (The two must be mutually comparable to be able to work in e.g., BTrees.)
# Instances of this class generally don't have a __module__ other than
# `zope.interface.declarations`, whereas they *do* have a __name__ that is the
# fully qualified name of the object they are representing.

# Note, though, that equality and hashing are still identity based. This
# accounts for things like nested objects that have the same name (typically
# only in tests) and is consistent with pickling. As far as comparisons to InterfaceClass
# goes, we'll never have equal name and module to those, so we're still consistent there.
# Instances of this class are essentially intended to be unique and are
# heavily cached (note how our __reduce__ handles this) so having identity
# based hash and eq should also work.
if other is None:
return -1

n1 = (self.__name__, self.__module__)
n2 = (getattr(other, '__name__', ''), getattr(other, '__module__', ''))

# This spelling works under Python3, which doesn't have cmp().
return (n1 > n2) - (n1 < n2)

# We want equality and hashing to be based on identity. However, we can't actually
# implement __eq__/__ne__ to do this because sometimes we get wrapped in a proxy.
# We need to let the proxy types implement these methods so they can handle unwrapping
# and then rely on: (1) the interpreter automatically changing `implements == proxy` into
# `proxy == implements` (which will call proxy.__eq__ to do the unwrapping) and then
# (2) the default equality and hashing semantics being identity based.

def __lt__(self, other):
c = self.__cmp(other)
return c < 0

def __le__(self, other):
c = self.__cmp(other)
return c <= 0

def __gt__(self, other):
c = self.__cmp(other)
return c > 0

def __ge__(self, other):
c = self.__cmp(other)
return c >= 0

def _implements_name(ob):
# Return the __name__ attribute to be used by its __implemented__
Expand Down

0 comments on commit c575695

Please sign in to comment.