From 8ac3bd088d9b924cfb3170b77b41effd2de39d23 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Fri, 6 Mar 2020 18:38:50 -0600 Subject: [PATCH 01/12] Move Interface hashing and comparison to C; 2.5 to 15x speedup in micro benchmarks Included benchmark numbers: Current master, Python 3.8: ..................... contains (empty dict): Mean +- std dev: 198 ns +- 5 ns ..................... contains (populated dict): Mean +- std dev: 197 ns +- 6 ns ..................... contains (populated list): Mean +- std dev: 53.1 us +- 1.2 us This code: ..................... contains (empty dict): Mean +- std dev: 77.9 ns +- 2.3 ns ..................... contains (populated dict): Mean +- std dev: 78.4 ns +- 3.1 ns ..................... contains (populated list): Mean +- std dev: 3.69 us +- 0.08 us So anywhere from 2.5 to 15x faster. Not sure how that will translate to larger benchmarks, but I'm hopeful. It turns out that messing with ``__module__`` is nasty, tricky business, especially when you do it from C. Everytime you define a new subclass, the descriptors that you set get overridden by the type machinery (PyType_Ready). I'm using a data descriptor and a meta class right now to avoid that but I'm not super happy with that and would like to find a better way. (At least, maybe the data part of the descriptor isn't necessary?) It may be needed to move more code into C, I don't want a slowdown accessing ``__module__`` either; copying around the standard PyGetSet or PyMember descriptors isn't enough because they don't work on the class object (so ``classImplements(InterfaceClass, IInterface)`` fails). --- MANIFEST.in | 1 + benchmarks/micro.py | 42 ++++ .../_zope_interface_coptimizations.c | 212 +++++++++++++++++- src/zope/interface/interface.py | 161 +++++++------ src/zope/interface/tests/test_interface.py | 2 +- src/zope/interface/tests/test_sorting.py | 17 ++ 6 files changed, 361 insertions(+), 74 deletions(-) create mode 100644 benchmarks/micro.py diff --git a/MANIFEST.in b/MANIFEST.in index 1dcbd316..83a0eb8e 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -18,3 +18,4 @@ global-exclude coverage.xml global-exclude appveyor.yml prune docs/_build +prune benchmarks diff --git a/benchmarks/micro.py b/benchmarks/micro.py new file mode 100644 index 00000000..0494b560 --- /dev/null +++ b/benchmarks/micro.py @@ -0,0 +1,42 @@ +import pyperf + +from zope.interface import Interface +from zope.interface.interface import InterfaceClass + +ifaces = [ + InterfaceClass('I' + str(i), (Interface,), {}) + for i in range(100) +] + +INNER = 1000 + +def bench_in(loops, o): + t0 = pyperf.perf_counter() + for _ in range(loops): + for _ in range(INNER): + o.__contains__(Interface) + + return pyperf.perf_counter() - t0 + +runner = pyperf.Runner() + +runner.bench_time_func( + 'contains (empty dict)', + bench_in, + {}, + inner_loops=INNER +) + +runner.bench_time_func( + 'contains (populated dict)', + bench_in, + {k: k for k in ifaces}, + inner_loops=INNER +) + +runner.bench_time_func( + 'contains (populated list)', + bench_in, + ifaces, + inner_loops=INNER +) diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c index 180cdef9..73414746 100644 --- a/src/zope/interface/_zope_interface_coptimizations.c +++ b/src/zope/interface/_zope_interface_coptimizations.c @@ -33,6 +33,9 @@ #if PY_MAJOR_VERSION >= 3 #define PY3K +#define PyNative_FromString PyUnicode_FromString +#else +#define PyNative_FromString PyString_FromString #endif static PyObject *str__dict__, *str__implemented__, *strextends; @@ -45,6 +48,7 @@ static PyObject *str_uncached_subscriptions; static PyObject *str_registry, *strro, *str_generation, *strchanged; static PyObject *str__self__; + static PyTypeObject *Implements; static int imported_declarations = 0; @@ -709,6 +713,17 @@ __adapt__(PyObject *self, PyObject *obj) return Py_None; } +#ifndef PY3K +typedef long Py_hash_t; +#endif + +typedef struct { + Spec spec; + PyObject* __name__; + PyObject* __module__; + Py_hash_t _v_cached_hash; +} IB; + static struct PyMethodDef ib_methods[] = { {"__adapt__", (PyCFunction)__adapt__, METH_O, "Adapt an object to the reciever"}, @@ -776,13 +791,188 @@ ib_call(PyObject *self, PyObject *args, PyObject *kwargs) return NULL; } + +static int +IB_traverse(IB* self, visitproc visit, void* arg) +{ + Py_VISIT(self->__name__); + Py_VISIT(self->__module__); + return 0; +} + +static int +IB_clear(IB* self) +{ + Py_CLEAR(self->__name__); + Py_CLEAR(self->__module__); + return 0; +} + +static void +IB_dealloc(IB* self) +{ + IB_clear(self); + Py_TYPE(self)->tp_free(OBJECT(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} +}; + +static Py_hash_t +IB_hash(IB* self) +{ + PyObject* tuple; + if (!self->__module__) { + PyErr_SetString(PyExc_AttributeError, "__module__"); + return -1; + } + if (!self->__name__) { + PyErr_SetString(PyExc_AttributeError, "__name__"); + return -1; + } + + if (self->_v_cached_hash) { + return self->_v_cached_hash; + } + + tuple = PyTuple_Pack(2, self->__name__, self->__module__); + if (!tuple) { + return -1; + } + self->_v_cached_hash = PyObject_Hash(tuple); + Py_CLEAR(tuple); + return self->_v_cached_hash; +} + +static PyTypeObject InterfaceBaseType; + +static PyObject* +IB_richcompare(IB* self, PyObject* other, int op) +{ + PyObject* othername; + PyObject* othermod; + PyObject* oresult; + IB* otherib; + int result; + + otherib = NULL; + oresult = othername = othermod = NULL; + + if (OBJECT(self) == other) { + switch(op) { + case Py_EQ: + case Py_LE: + case Py_GE: + Py_RETURN_TRUE; + break; + case Py_NE: + Py_RETURN_FALSE; + } + } + + if (other == Py_None) { + switch(op) { + case Py_LT: + case Py_LE: + case Py_NE: + Py_RETURN_TRUE; + default: + Py_RETURN_FALSE; + } + } + + if (PyObject_TypeCheck(other, &InterfaceBaseType)) { + otherib = (IB*)other; + othername = otherib->__name__; + othermod = otherib->__module__; + } + else { + othername = PyObject_GetAttrString(other, "__name__"); + // TODO: Optimize this case. + if (othername == NULL) { + PyErr_Clear(); + othername = PyNative_FromString(""); + } + othermod = PyObject_GetAttrString(other, "__module__"); + if (othermod == NULL) { + PyErr_Clear(); + othermod = PyNative_FromString(""); + } + } +#if 0 +// This is the simple, straightforward version of what Python does. + PyObject* pt1 = PyTuple_Pack(2, self->__name__, self->__module__); + PyObject* pt2 = PyTuple_Pack(2, othername, othermod); + oresult = PyObject_RichCompare(pt1, pt2, op); +#endif + + // tuple comparison is decided by the first non-equal element. + result = PyObject_RichCompareBool(self->__name__, othername, Py_EQ); + if (result == 0) { + result = PyObject_RichCompareBool(self->__name__, othername, op); + } + else if (result == 1) { + result = PyObject_RichCompareBool(self->__module__, othermod, op); + } + if (result == -1) { + goto cleanup; + } + + oresult = result ? Py_True : Py_False; + Py_INCREF(oresult); + + +cleanup: + if (!otherib) { + Py_XDECREF(othername); + Py_XDECREF(othermod); + } + return oresult; + +} + +static PyObject* +IB_module_get(IB* self, void* context) +{ + return self->__module__; +} + +static int +IB_module_set(IB* self, PyObject* value, void* context) +{ + Py_XINCREF(value); + Py_XDECREF(self->__module__); + self->__module__ = value; + return 0; +} + +static int +IB_init(IB* self, PyObject* args, PyObject* kwargs) +{ + IB_clear(self); + self->__module__ = Py_None; + Py_INCREF(self->__module__); + self->__name__ = Py_None; + Py_INCREF(self->__name__); + 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) /* tp_name */ "_zope_interface_coptimizations." "InterfaceBase", - /* tp_basicsize */ 0, + /* tp_basicsize */ sizeof(IB), /* tp_itemsize */ 0, - /* tp_dealloc */ (destructor)0, + /* tp_dealloc */ (destructor)IB_dealloc, /* tp_print */ (printfunc)0, /* tp_getattr */ (getattrfunc)0, /* tp_setattr */ (setattrfunc)0, @@ -791,22 +981,30 @@ static PyTypeObject InterfaceBaseType = { /* tp_as_number */ 0, /* tp_as_sequence */ 0, /* tp_as_mapping */ 0, - /* tp_hash */ (hashfunc)0, + /* tp_hash */ (hashfunc)IB_hash, /* tp_call */ (ternaryfunc)ib_call, /* tp_str */ (reprfunc)0, /* tp_getattro */ (getattrofunc)0, /* tp_setattro */ (setattrofunc)0, /* tp_as_buffer */ 0, /* tp_flags */ Py_TPFLAGS_DEFAULT - | Py_TPFLAGS_BASETYPE , + | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, /* tp_doc */ "Interface base type providing __call__ and __adapt__", - /* tp_traverse */ (traverseproc)0, - /* tp_clear */ (inquiry)0, - /* tp_richcompare */ (richcmpfunc)0, + /* tp_traverse */ (traverseproc)IB_traverse, + /* tp_clear */ (inquiry)IB_clear, + /* tp_richcompare */ (richcmpfunc)IB_richcompare, /* tp_weaklistoffset */ (long)0, /* tp_iter */ (getiterfunc)0, /* tp_iternext */ (iternextfunc)0, /* tp_methods */ ib_methods, + /* tp_members */ IB_members, + /* tp_getset */ IB_getsets, + /* tp_base */ &SpecType, + /* tp_dict */ 0, + /* tp_descr_get */ 0, + /* tp_descr_set */ 0, + /* tp_dictoffset */ 0, + /* tp_init */ (initproc)IB_init, }; /* =================== End: __call__ and __adapt__ ==================== */ diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index a8eac019..8707b9ed 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -174,6 +174,10 @@ class InterfaceBase(object): __slots__ = () + def __init__(self): + self.__name__ = None + self.__module__ = None + def _call_conform(self, conform): raise NotImplementedError @@ -209,6 +213,67 @@ def __adapt__(self, obj): if adapter is not None: return adapter + 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__. + # I don't want to override __eq__ because I want the default + # __eq__, which is really fast. + """Make interfaces sortable + + TODO: It would ne nice if: + + More specific interfaces should sort before less specific ones. + Otherwise, sort on name and module. + + But this is too complicated, and we're going to punt on it + for now. + + For now, sort on interface and module name. + + None is treated as a pseudo interface that implies the loosest + contact possible, no contract. For that reason, all interfaces + sort before None. + + """ + 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) + + def __hash__(self): + try: + return self._v_cached_hash + except AttributeError: + self._v_cached_hash = hash((self.__name__, self.__module__)) + return self._v_cached_hash + + def __eq__(self, other): + c = self.__cmp(other) + return c == 0 + + def __ne__(self, other): + c = self.__cmp(other) + return c != 0 + + 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 adapter_hooks = _use_c_impl([], 'adapter_hooks') @@ -420,8 +485,33 @@ def get(self, name, default=None): return default if attr is None else attr +class _ModuleDescriptor(object): + def __init__(self, saved): + self._saved = saved + + def __get__(self, inst, kind): + if inst is None: + return self._saved + return inst.__ibmodule__ + + def __set__(self, inst, val): + inst.__ibmodule__ = val + +# The simple act of having *any* metaclass besides type +# makes our __module__ shenanigans work. Doing this at the class level, +# and manually copying it around doesn't work. +class _MC(type): + def __new__(cls, name, bases, attrs): + attrs['__module__'] = _ModuleDescriptor(attrs['__module__']) + return type.__new__(cls, name, bases, attrs) + +_InterfaceClassBase = _MC( + 'InterfaceClass', + (Element, InterfaceBase, Specification), + {'__module__': __name__}) + -class InterfaceClass(Element, InterfaceBase, Specification): +class InterfaceClass(_InterfaceClassBase): """Prototype (scarecrow) Interfaces Implementation.""" # We can't say this yet because we don't have enough @@ -449,6 +539,7 @@ def __init__(self, name, bases=(), attrs=None, __doc__=None, pass self.__module__ = __module__ + assert '__module__' not in self.__dict__ d = attrs.get('__doc__') if d is not None: @@ -471,6 +562,7 @@ def __init__(self, name, bases=(), attrs=None, __doc__=None, if not isinstance(base, InterfaceClass): raise TypeError('Expected base interfaces') + Specification.__init__(self, bases) # Make sure that all recorded attributes (and methods) are of type @@ -495,7 +587,7 @@ def __init__(self, name, bases=(), attrs=None, __doc__=None, self.__attrs = attrs - self.__identifier__ = "%s.%s" % (self.__module__, self.__name__) + self.__identifier__ = "%s.%s" % (__module__, name) def interfaces(self): """Return an iterator for the interfaces in the specification. @@ -607,7 +699,7 @@ def __repr__(self): # pragma: no cover return self._v_repr except AttributeError: name = self.__name__ - m = self.__module__ + m = self.__ibmodule__ if m: name = '%s.%s' % (m, name) r = "<%s %s>" % (self.__class__.__name__, name) @@ -636,69 +728,6 @@ def _call_conform(self, conform): def __reduce__(self): return self.__name__ - 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__. - # I don't want to override __eq__ because I want the default - # __eq__, which is really fast. - """Make interfaces sortable - - TODO: It would ne nice if: - - More specific interfaces should sort before less specific ones. - Otherwise, sort on name and module. - - But this is too complicated, and we're going to punt on it - for now. - - For now, sort on interface and module name. - - None is treated as a pseudo interface that implies the loosest - contact possible, no contract. For that reason, all interfaces - sort before None. - - """ - 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) - - def __hash__(self): - try: - return self._v_cached_hash - except AttributeError: - self._v_cached_hash = hash((self.__name__, self.__module__)) - return self._v_cached_hash - - def __eq__(self, other): - c = self.__cmp(other) - return c == 0 - - def __ne__(self, other): - c = self.__cmp(other) - return c != 0 - - 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 - - Interface = InterfaceClass("Interface", __module__='zope.interface') # Interface is the only member of its own SRO. Interface._calculate_sro = lambda: (Interface,) diff --git a/src/zope/interface/tests/test_interface.py b/src/zope/interface/tests/test_interface.py index da7eb8ce..737b2296 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -284,7 +284,6 @@ def test___call___w___conform___ob_no_provides_wo_alternate(self): self.assertRaises(TypeError, ib, adapted) - class InterfaceBaseTests(GenericInterfaceBaseTests, OptimizationTestMixin): # Tests that work with the C implementation @@ -1049,6 +1048,7 @@ def method2(self, a, b): self.assertTrue(ICurrent.implementedBy(Current)) self.assertFalse(IOther.implementedBy(Current)) + self.assertEqual(ICurrent, ICurrent) self.assertTrue(ICurrent in implementedBy(Current)) self.assertFalse(IOther in implementedBy(Current)) self.assertTrue(ICurrent in providedBy(current)) diff --git a/src/zope/interface/tests/test_sorting.py b/src/zope/interface/tests/test_sorting.py index 73613d0f..0e33f47f 100644 --- a/src/zope/interface/tests/test_sorting.py +++ b/src/zope/interface/tests/test_sorting.py @@ -45,3 +45,20 @@ def test_w_equal_names(self): l = [I1, m1_I1] l.sort() self.assertEqual(l, [m1_I1, I1]) + + def test_I1_I2(self): + self.assertLess(I1.__name__, I2.__name__) + self.assertEqual(I1.__module__, I2.__module__) + self.assertEqual(I1.__module__, __name__) + self.assertLess(I1, I2) + + def _makeI1(self): + class I1(Interface): + pass + return I1 + + def test_nested(self): + nested_I1 = self._makeI1() + self.assertEqual(I1, nested_I1) + self.assertEqual(nested_I1, I1) + self.assertEqual(hash(I1), hash(nested_I1)) From b9165b790c831b6d8a87a3f27d4f135143494ff8 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Tue, 10 Mar 2020 14:17:28 -0500 Subject: [PATCH 02/12] Fix doctest by making sure the default type repr can be used. --- src/zope/interface/interface.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index 8707b9ed..1ed1b6fd 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -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 as calculate_ro from zope.interface import ro @@ -485,8 +486,18 @@ def get(self, name, default=None): return default if attr is None else attr -class _ModuleDescriptor(object): +class _ModuleDescriptor(str): + # 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__ = ('_saved',) + def __init__(self, saved): + str.__init__(self) self._saved = saved def __get__(self, inst, kind): @@ -497,6 +508,9 @@ def __get__(self, inst, kind): def __set__(self, inst, val): inst.__ibmodule__ = val + def __str__(self): + return self._saved + # The simple act of having *any* metaclass besides type # makes our __module__ shenanigans work. Doing this at the class level, # and manually copying it around doesn't work. @@ -508,7 +522,7 @@ def __new__(cls, name, bases, attrs): _InterfaceClassBase = _MC( 'InterfaceClass', (Element, InterfaceBase, Specification), - {'__module__': __name__}) + {'__module__': __name__, '__qualname__': __name__ + 'InterfaceClass'}) class InterfaceClass(_InterfaceClassBase): From 01e0a7e36cecbd0a36fd49b08b1c21e0a5bba119 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Tue, 10 Mar 2020 17:47:21 -0500 Subject: [PATCH 03/12] Benchmarks looking up adapters from components. Current results (this branch vs master, 354faccebd5b612a2ac8e081a7e5d2f7fb1089c1): | Benchmark | 38-master | 38-faster | |-------------------------------------------|-----------|-------------------------------| | query adapter (no registrations) | 3.81 ms | 3.03 ms: 1.26x faster (-20%) | | query adapter (all trivial registrations) | 4.65 ms | 3.90 ms: 1.19x faster (-16%) | | contains (empty dict) | 163 ns | 76.1 ns: 2.14x faster (-53%) | | contains (populated dict) | 162 ns | 76.9 ns: 2.11x faster (-53%) | | contains (populated list) | 40.3 us | 3.09 us: 13.04x faster (-92%) | Also need benchmarks using inheritance. The 'implied' data structures are also hash/equality based. --- benchmarks/micro.py | 62 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/benchmarks/micro.py b/benchmarks/micro.py index 0494b560..9031261e 100644 --- a/benchmarks/micro.py +++ b/benchmarks/micro.py @@ -1,14 +1,33 @@ import pyperf from zope.interface import Interface +from zope.interface import classImplements from zope.interface.interface import InterfaceClass +from zope.interface.registry import Components +# Long, mostly similar names are a worst case for equality +# comparisons. ifaces = [ - InterfaceClass('I' + str(i), (Interface,), {}) + InterfaceClass('I' + ('0' * 20) + str(i), (Interface,), {}) for i in range(100) ] -INNER = 1000 +def make_implementer(iface): + c = type('Implementer' + iface.__name__, (object,), {}) + classImplements(c, iface) + return c + +implementers = [ + make_implementer(iface) + for iface in ifaces +] + +providers = [ + implementer() + for implementer in implementers +] + +INNER = 10 def bench_in(loops, o): t0 = pyperf.perf_counter() @@ -18,8 +37,47 @@ def bench_in(loops, o): return pyperf.perf_counter() - t0 +def bench_query_adapter(loops, components): + # One time through to prime the caches + for iface in ifaces: + for provider in providers: + components.queryAdapter(provider, iface) + + t0 = pyperf.perf_counter() + for _ in range(loops): + for iface in ifaces: + for provider in providers: + components.queryAdapter(provider, iface) + return pyperf.perf_counter() - t0 + runner = pyperf.Runner() +runner.bench_time_func( + 'query adapter (no registrations)', + bench_query_adapter, + Components(), + inner_loops=1 +) + +def populate_components(): + + def factory(o): + return 42 + + pop_components = Components() + for iface in ifaces: + for other_iface in ifaces: + pop_components.registerAdapter(factory, (iface,), other_iface, event=False) + + return pop_components + +runner.bench_time_func( + 'query adapter (all trivial registrations)', + bench_query_adapter, + populate_components(), + inner_loops=1 +) + runner.bench_time_func( 'contains (empty dict)', bench_in, From 5f4bb3f8ec7798b146c007041ff60aac2ca0566e Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 08:17:11 -0500 Subject: [PATCH 04/12] Clean up linter errors in test_interface.py so new/real problems are more obvious. --- src/zope/interface/tests/test_interface.py | 93 ++++++++++++---------- 1 file changed, 52 insertions(+), 41 deletions(-) diff --git a/src/zope/interface/tests/test_interface.py b/src/zope/interface/tests/test_interface.py index 737b2296..c6b21ad8 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -13,7 +13,14 @@ ############################################################################## """Test Interface implementation """ -# pylint:disable=protected-access +# Things we let slide because it's a test +# pylint:disable=protected-access,blacklisted-name,attribute-defined-outside-init +# pylint:disable=too-many-public-methods,too-many-lines,abstract-method +# pylint:disable=redefined-builtin,signature-differs,arguments-differ +# Things you get inheriting from Interface +# pylint:disable=inherit-non-class,no-self-argument,no-method-argument +# Things you get using methods of an Interface 'subclass' +# pylint:disable=no-value-for-parameter import unittest from zope.interface._compat import _skip_under_py3k @@ -99,7 +106,7 @@ def _getTargetClass(self): from zope.interface.interface import Element return Element - def _makeOne(self, name=None): + def _makeOne(self, name=None): if name is None: name = self.DEFAULT_NAME return self._getTargetClass()(name) @@ -166,7 +173,7 @@ def test_verifies(self): class GenericSpecificationBaseTests(unittest.TestCase): # Tests that work with both implementations def _getFallbackClass(self): - from zope.interface.interface import SpecificationBasePy + from zope.interface.interface import SpecificationBasePy # pylint:disable=no-name-in-module return SpecificationBasePy _getTargetClass = _getFallbackClass @@ -247,15 +254,17 @@ def _providedBy(obj): self.assertTrue(sb.providedBy(object())) -class GenericInterfaceBaseTests(unittest.TestCase): +class InterfaceBaseTestsMixin(object): # Tests for both C and Python implementation + def _getTargetClass(self): + raise NotImplementedError + def _getFallbackClass(self): + # pylint:disable=no-name-in-module from zope.interface.interface import InterfaceBasePy return InterfaceBasePy - _getTargetClass = _getFallbackClass - def _makeOne(self, object_should_provide): class IB(self._getTargetClass()): def _call_conform(self, conform): @@ -274,6 +283,7 @@ def __conform__(self, iface): def test___call___wo___conform___ob_no_provides_w_alternate(self): ib = self._makeOne(False) + __traceback_info__ = ib, self._getTargetClass() adapted = object() alternate = object() self.assertIs(ib(adapted, alternate), alternate) @@ -284,17 +294,20 @@ def test___call___w___conform___ob_no_provides_wo_alternate(self): self.assertRaises(TypeError, ib, adapted) -class InterfaceBaseTests(GenericInterfaceBaseTests, - OptimizationTestMixin): +class InterfaceBaseTests(InterfaceBaseTestsMixin, + OptimizationTestMixin, + unittest.TestCase): # Tests that work with the C implementation def _getTargetClass(self): from zope.interface.interface import InterfaceBase return InterfaceBase -class InterfaceBasePyTests(GenericInterfaceBaseTests): +class InterfaceBasePyTests(InterfaceBaseTestsMixin, unittest.TestCase): # Tests that only work with the Python implementation + _getTargetClass = InterfaceBaseTestsMixin._getFallbackClass + def test___call___w___conform___miss_ob_provides(self): ib = self._makeOne(True) class _Adapted(object): @@ -315,7 +328,6 @@ def test___adapt___ob_no_provides_uses_hooks(self): _missed = [] def _hook_miss(iface, obj): _missed.append((iface, obj)) - return None def _hook_hit(iface, obj): return obj with _Monkey(interface, adapter_hooks=[_hook_miss, _hook_hit]): @@ -667,8 +679,8 @@ def _bar(): base = self._makeOne('IBase', attrs=BASE_ATTRS) derived = self._makeOne('IDerived', bases=(base,), attrs=DERIVED_ATTRS) self.assertEqual(sorted(derived.namesAndDescriptions(all=False)), - [('baz', DERIVED_ATTRS['baz']), - ]) + [('baz', DERIVED_ATTRS['baz']), + ]) def test_namesAndDescriptions_w_all_True_no_bases(self): from zope.interface.interface import Attribute @@ -680,9 +692,9 @@ def _bar(): } one = self._makeOne(attrs=ATTRS) self.assertEqual(sorted(one.namesAndDescriptions(all=False)), - [('bar', ATTRS['bar']), - ('foo', ATTRS['foo']), - ]) + [('bar', ATTRS['bar']), + ('foo', ATTRS['foo']), + ]) def test_namesAndDescriptions_w_all_True_simple(self): from zope.interface.interface import Attribute @@ -697,10 +709,10 @@ def _bar(): base = self._makeOne('IBase', attrs=BASE_ATTRS) derived = self._makeOne('IDerived', bases=(base,), attrs=DERIVED_ATTRS) self.assertEqual(sorted(derived.namesAndDescriptions(all=True)), - [('bar', BASE_ATTRS['bar']), - ('baz', DERIVED_ATTRS['baz']), - ('foo', BASE_ATTRS['foo']), - ]) + [('bar', BASE_ATTRS['bar']), + ('baz', DERIVED_ATTRS['baz']), + ('foo', BASE_ATTRS['foo']), + ]) def test_namesAndDescriptions_w_all_True_bases_w_same_names(self): from zope.interface.interface import Attribute @@ -718,10 +730,10 @@ def _foo(): base = self._makeOne('IBase', attrs=BASE_ATTRS) derived = self._makeOne('IDerived', bases=(base,), attrs=DERIVED_ATTRS) self.assertEqual(sorted(derived.namesAndDescriptions(all=True)), - [('bar', BASE_ATTRS['bar']), - ('baz', DERIVED_ATTRS['baz']), - ('foo', DERIVED_ATTRS['foo']), - ]) + [('bar', BASE_ATTRS['bar']), + ('baz', DERIVED_ATTRS['baz']), + ('foo', DERIVED_ATTRS['foo']), + ]) def test_getDescriptionFor_miss(self): one = self._makeOne() @@ -902,13 +914,14 @@ def test___hash___normal(self): def test___hash___missing_required_attrs(self): class Derived(self._getTargetClass()): - def __init__(self): + def __init__(self): # pylint:disable=super-init-not-called pass # Don't call base class. derived = Derived() with self.assertRaises(AttributeError): hash(derived) def test_comparison_with_None(self): + # pylint:disable=singleton-comparison,misplaced-comparison-constant iface = self._makeOne() self.assertTrue(iface < None) self.assertTrue(iface <= None) @@ -925,6 +938,7 @@ def test_comparison_with_None(self): self.assertTrue(None > iface) def test_comparison_with_same_instance(self): + # pylint:disable=comparison-with-itself iface = self._makeOne() self.assertFalse(iface < iface) @@ -1762,7 +1776,7 @@ class HasInvariant(object): has_invariant.foo = 2 has_invariant.bar = 1 self._errorsEqual(has_invariant, 1, - ['Please, Boo MUST be greater than Foo!'], IInvariant) + ['Please, Boo MUST be greater than Foo!'], IInvariant) # and if we set foo to a positive number and boo to 0, we'll # get both errors! has_invariant.foo = 1 @@ -1781,27 +1795,25 @@ class HasInvariant(object): def test___doc___element(self): from zope.interface import Interface from zope.interface import Attribute - class I(Interface): + class IDocstring(Interface): "xxx" - self.assertEqual(I.__doc__, "xxx") - self.assertEqual(list(I), []) + self.assertEqual(IDocstring.__doc__, "xxx") + self.assertEqual(list(IDocstring), []) - class I(Interface): + class IDocstringAndAttribute(Interface): "xxx" __doc__ = Attribute('the doc') - self.assertEqual(I.__doc__, "") - self.assertEqual(list(I), ['__doc__']) + self.assertEqual(IDocstringAndAttribute.__doc__, "") + self.assertEqual(list(IDocstringAndAttribute), ['__doc__']) @_skip_under_py3k def testIssue228(self): # Test for http://collector.zope.org/Zope3-dev/228 # Old style classes don't have a '__class__' attribute # No old style classes in Python 3, so the test becomes moot. - import sys - from zope.interface import Interface class I(Interface): @@ -1834,10 +1846,10 @@ class Range(object): def __init__(self, min, max): self.min, self.max = min, max - IRange.validateInvariants(Range(1,2)) - IRange.validateInvariants(Range(1,1)) + IRange.validateInvariants(Range(1, 2)) + IRange.validateInvariants(Range(1, 1)) try: - IRange.validateInvariants(Range(2,1)) + IRange.validateInvariants(Range(2, 1)) except Invalid as e: self.assertEqual(str(e), 'max < min') @@ -2008,7 +2020,6 @@ class C(object): def test___call___w_adapter_hook(self): from zope.interface import Interface from zope.interface.interface import adapter_hooks - old_hooks = adapter_hooks[:] def _miss(iface, obj): pass @@ -2214,7 +2225,7 @@ def _func(foo='bar'): self.assertEqual(info['kwargs'], None) def test_w_optional_self(self): - # XXX This is a weird case, trying to cover the following code in + # This is a weird case, trying to cover the following code in # FUT:: # # nr = na-len(defaults) @@ -2254,7 +2265,7 @@ def _func(**kw): self.assertEqual(info['kwargs'], 'kw') def test_full_spectrum(self): - def _func(foo, bar='baz', *args, **kw): + def _func(foo, bar='baz', *args, **kw): # pylint:disable=keyword-arg-before-vararg "DOCSTRING" method = self._callFUT(_func) info = method.getSignatureInfo() @@ -2289,7 +2300,7 @@ def bar(self): def test_full_spectrum(self): class Foo(object): - def bar(self, foo, bar='baz', *args, **kw): + def bar(self, foo, bar='baz', *args, **kw): # pylint:disable=keyword-arg-before-vararg "DOCSTRING" method = self._callFUT(Foo.bar) info = method.getSignatureInfo() @@ -2345,7 +2356,7 @@ class _Monkey(object): # context-manager for replacing module names in the scope of a test. def __init__(self, module, **kw): self.module = module - self.to_restore = dict([(key, getattr(module, key)) for key in kw]) + self.to_restore = {key: getattr(module, key) for key in kw} for key, value in kw.items(): setattr(module, key, value) From 7afd59d869e3391ce27a4a07ad0931eb5e7910a1 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 08:28:23 -0500 Subject: [PATCH 05/12] Fix tests when zope.component is also importable. --- src/zope/interface/tests/__init__.py | 22 ++++++++++++++++++++++ src/zope/interface/tests/test_interface.py | 3 ++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/zope/interface/tests/__init__.py b/src/zope/interface/tests/__init__.py index c37dffc2..8b9ef258 100644 --- a/src/zope/interface/tests/__init__.py +++ b/src/zope/interface/tests/__init__.py @@ -31,3 +31,25 @@ def test_optimizations(self): self.assertIsNot(used, fallback) else: self.assertIs(used, fallback) + +# Be sure cleanup functionality is available; classes that use the adapter hook +# need to be sure to subclass ``CleanUp``. +# +# If zope.component is installed and imported when we run our tests +# (import chain: +# zope.testrunner->zope.security->zope.location->zope.component.api) +# it adds an adapter hook that uses its global site manager. That can cause +# leakage from one test to another unless its cleanup hooks are run. The symptoms can +# be odd, especially if one test used C objects and the next used the Python +# implementation. (For example, you can get strange TypeErrors or find inexplicable +# comparisons being done.) +try: + from zope.testing import cleanup +except ImportError: + class CleanUp(object): + def cleanUp(self): + pass + + setUp = tearDown = cleanUp +else: + CleanUp = cleanup.CleanUp diff --git a/src/zope/interface/tests/test_interface.py b/src/zope/interface/tests/test_interface.py index c6b21ad8..e7ae5475 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -25,6 +25,7 @@ from zope.interface._compat import _skip_under_py3k from zope.interface.tests import OptimizationTestMixin +from zope.interface.tests import CleanUp _marker = object() @@ -254,7 +255,7 @@ def _providedBy(obj): self.assertTrue(sb.providedBy(object())) -class InterfaceBaseTestsMixin(object): +class InterfaceBaseTestsMixin(CleanUp): # Tests for both C and Python implementation def _getTargetClass(self): From 413e716f9fb48338829435c4f243133589eb9fe6 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 12:19:19 -0500 Subject: [PATCH 06/12] Avoid use of a metaclass by implementeng __getattribute__. 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%) | +-------------------------------------------+------------+-------------------------------+ --- benchmarks/micro.py | 50 ++- .../_zope_interface_coptimizations.c | 63 ++-- src/zope/interface/declarations.py | 72 ++--- src/zope/interface/interface.py | 294 ++++++++++-------- src/zope/interface/tests/test_interface.py | 4 +- 5 files changed, 270 insertions(+), 213 deletions(-) diff --git a/benchmarks/micro.py b/benchmarks/micro.py index 9031261e..779298c1 100644 --- a/benchmarks/micro.py +++ b/benchmarks/micro.py @@ -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() @@ -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, diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c index 73414746..adc73511 100644 --- a/src/zope/interface/_zope_interface_coptimizations.c +++ b/src/zope/interface/_zope_interface_coptimizations.c @@ -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; @@ -97,7 +98,7 @@ import_declarations(void) } -static PyTypeObject SpecType; /* Forward */ +static PyTypeObject SpecificationBaseType; /* Forward */ static PyObject * implementedByFallback(PyObject *cls) @@ -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; @@ -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) ) @@ -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 @@ -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); @@ -438,7 +439,7 @@ static PyMemberDef Spec_members[] = { }; -static PyTypeObject SpecType = { +static PyTypeObject SpecificationBaseType = { PyVarObject_HEAD_INIT(NULL, 0) /* tp_name */ "_interface_coptimizations." "SpecificationBase", @@ -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, @@ -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; @@ -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} }; @@ -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; } @@ -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 @@ -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) @@ -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 @@ -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, @@ -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) @@ -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) diff --git a/src/zope/interface/declarations.py b/src/zope/interface/declarations.py index 1e9a2ea0..502e26e4 100644 --- a/src/zope/interface/declarations.py +++ b/src/zope/interface/declarations.py @@ -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 @@ -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 @@ -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__ diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index 1ed1b6fd..d17c8dc1 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -13,14 +13,13 @@ ############################################################################## """Interface object implementation """ - +# pylint:disable=protected-access import sys from types import MethodType from types import FunctionType 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 as calculate_ro from zope.interface import ro @@ -70,7 +69,7 @@ class Element(object): # #implements(IElement) - def __init__(self, __name__, __doc__=''): + def __init__(self, __name__, __doc__=''): # pylint:disable=redefined-builtin if not __doc__ and __name__.find(' ') >= 0: __doc__ = __name__ __name__ = None @@ -121,6 +120,9 @@ def setTaggedValue(self, tag, value): getDirectTaggedValueTags = getTaggedValueTags +SpecificationBasePy = object # filled by _use_c_impl. + + @_use_c_impl class SpecificationBase(object): # This object is the base of the inheritance hierarchy for ClassProvides: @@ -163,31 +165,126 @@ def implementedBy(self, cls): def isOrExtends(self, interface): """Is the interface the same as or extend the given interface """ - return interface in self._implied + return interface in self._implied # pylint:disable=no-member __call__ = isOrExtends +class NameAndModuleComparisonMixin(object): + # Internal use. Implement the basic sorting operators (but not (in)equality + # or hashing). Subclasses must provide ``__name__`` and ``__module__`` + # attributes. Subclasses will be mutually comparable; but because equality + # and hashing semantics are missing from this class, take care in how + # you define those two attributes: If you stick with the default equality + # and hashing (identity based) you should make sure that all possible ``__name__`` + # and ``__module__`` pairs are unique ACROSS ALL SUBCLASSES. (Actually, pretty + # much the same thing goes if you define equality and hashing to be based on + # those two attributes: they must still be consistent ACROSS ALL SUBCLASSES.) + + # pylint:disable=assigning-non-slot + __slots__ = () + def _compare(self, other): + """ + Compare *self* to *other* based on ``__name__`` and ``__module__``. + + Return 0 if they are equal, return 1 if *self* is + greater than *other*, and return -1 if *self* is less than + *other*. + + If *other* does not have ``__name__`` or ``__module__``, then + return ``NotImplemented``. + + + None is treated as a pseudo interface that implies the loosest + contact possible, no contract. For that reason, all interfaces + sort before None. + """ + if other is self: + return 0 + + if other is None: + return -1 + + n1 = (self.__name__, self.__module__) + try: + n2 = (other.__name__, other.__module__) + except AttributeError: + return NotImplemented + + # This spelling works under Python3, which doesn't have cmp(). + return (n1 > n2) - (n1 < n2) + + def __lt__(self, other): + c = self._compare(other) + if c is NotImplemented: + return c + return c < 0 + + def __le__(self, other): + c = self._compare(other) + if c is NotImplemented: + return c + return c <= 0 + + def __gt__(self, other): + c = self._compare(other) + if c is NotImplemented: + return c + return c > 0 + + def __ge__(self, other): + c = self._compare(other) + if c is NotImplemented: + return c + return c >= 0 + @_use_c_impl -class InterfaceBase(object): +class InterfaceBase(NameAndModuleComparisonMixin, SpecificationBasePy): """Base class that wants to be replaced with a C base :) """ - __slots__ = () + __slots__ = ( + '__name__', + '__ibmodule__', + ) - def __init__(self): - self.__name__ = None - self.__module__ = None + 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) + def __call__(self, obj, alternate=_marker): """Adapt an object to the interface """ try: conform = getattr(obj, '__conform__', None) - except: + # XXX: Do we really want to catch BaseException? Shouldn't + # things like MemoryError, KeyboardInterrupt, etc, get through? + except: # pylint:disable=bare-except conform = None if conform is not None: adapter = self._call_conform(conform) @@ -198,13 +295,12 @@ def __call__(self, obj, alternate=_marker): if adapter is not None: return adapter - elif alternate is not _marker: + if alternate is not _marker: return alternate - else: - raise TypeError("Could not adapt", obj, self) + raise TypeError("Could not adapt", obj, self) def __adapt__(self, obj): - """Adapt an object to the reciever + """Adapt an object to the receiver """ if self.providedBy(obj): return obj @@ -214,38 +310,10 @@ def __adapt__(self, obj): if adapter is not None: return adapter - 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__. - # I don't want to override __eq__ because I want the default - # __eq__, which is really fast. - """Make interfaces sortable - - TODO: It would ne nice if: - - More specific interfaces should sort before less specific ones. - Otherwise, sort on name and module. - - But this is too complicated, and we're going to punt on it - for now. - - For now, sort on interface and module name. - - None is treated as a pseudo interface that implies the loosest - contact possible, no contract. For that reason, all interfaces - sort before None. - - """ - 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) + return None def __hash__(self): + # pylint:disable=assigning-non-slot,attribute-defined-outside-init try: return self._v_cached_hash except AttributeError: @@ -253,28 +321,19 @@ def __hash__(self): return self._v_cached_hash def __eq__(self, other): - c = self.__cmp(other) + c = self._compare(other) + if c is NotImplemented: + return c return c == 0 def __ne__(self, other): - c = self.__cmp(other) - return c != 0 - - 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 + if other is self: + return False - def __ge__(self, other): - c = self.__cmp(other) - return c >= 0 + c = self._compare(other) + if c is NotImplemented: + return c + return c != 0 adapter_hooks = _use_c_impl([], 'adapter_hooks') @@ -486,56 +545,26 @@ def get(self, name, default=None): return default if attr is None else attr -class _ModuleDescriptor(str): - # 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__ = ('_saved',) - - def __init__(self, saved): - str.__init__(self) - self._saved = saved - - def __get__(self, inst, kind): - if inst is None: - return self._saved - return inst.__ibmodule__ - - def __set__(self, inst, val): - inst.__ibmodule__ = val - - def __str__(self): - return self._saved - -# The simple act of having *any* metaclass besides type -# makes our __module__ shenanigans work. Doing this at the class level, -# and manually copying it around doesn't work. -class _MC(type): - def __new__(cls, name, bases, attrs): - attrs['__module__'] = _ModuleDescriptor(attrs['__module__']) - return type.__new__(cls, name, bases, attrs) - -_InterfaceClassBase = _MC( - 'InterfaceClass', - (Element, InterfaceBase, Specification), - {'__module__': __name__, '__qualname__': __name__ + 'InterfaceClass'}) +class InterfaceClass(InterfaceBase, Element, Specification): + """ + Prototype (scarecrow) Interfaces Implementation. -class InterfaceClass(_InterfaceClassBase): - """Prototype (scarecrow) Interfaces Implementation.""" + Note that it is not possible to change the ``__name__`` or ``__module__`` + after an instance of this object has been constructed. + """ # We can't say this yet because we don't have enough # infrastructure in place. # #implements(IInterface) - def __init__(self, name, bases=(), attrs=None, __doc__=None, + 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 = {} @@ -552,8 +581,7 @@ def __init__(self, name, bases=(), attrs=None, __doc__=None, except (AttributeError, KeyError): # pragma: no cover pass - self.__module__ = __module__ - assert '__module__' not in self.__dict__ + self.__ibmodule__ = __module__ d = attrs.get('__doc__') if d is not None: @@ -572,36 +600,38 @@ def __init__(self, name, bases=(), attrs=None, __doc__=None, for key, val in tagged_data.items(): self.setTaggedValue(key, val) - for base in bases: - if not isinstance(base, InterfaceClass): - raise TypeError('Expected base interfaces') - - Specification.__init__(self, bases) + self.__attrs = self.__compute_attrs(attrs) + self.__identifier__ = "%s.%s" % (__module__, name) + + def __compute_attrs(self, attrs): # Make sure that all recorded attributes (and methods) are of type # `Attribute` and `Method` - for name, attr in list(attrs.items()): - if name in ('__locals__', '__qualname__', '__annotations__'): + def update_value(aname, aval): + if isinstance(aval, Attribute): + aval.interface = self + if not aval.__name__: + aval.__name__ = aname + elif isinstance(aval, FunctionType): + aval = fromFunction(aval, self, name=aname) + else: + raise InvalidInterface("Concrete attribute, " + aname) + return aval + + return { + aname: update_value(aname, aval) + for aname, aval in attrs.items() + if aname not in ( # __locals__: Python 3 sometimes adds this. + '__locals__', # __qualname__: PEP 3155 (Python 3.3+) + '__qualname__', # __annotations__: PEP 3107 (Python 3.0+) - del attrs[name] - continue - if isinstance(attr, Attribute): - attr.interface = self - if not attr.__name__: - attr.__name__ = name - elif isinstance(attr, FunctionType): - attrs[name] = fromFunction(attr, self, name=name) - elif attr is _decorator_non_return: - del attrs[name] - else: - raise InvalidInterface("Concrete attribute, " + name) - - self.__attrs = attrs - - self.__identifier__ = "%s.%s" % (__module__, name) + '__annotations__' + ) + and aval is not _decorator_non_return + } def interfaces(self): """Return an iterator for the interfaces in the specification. @@ -615,7 +645,7 @@ def isEqualOrExtendedBy(self, other): """Same interface or extends?""" return self == other or other.extends(self) - def names(self, all=False): + def names(self, all=False): # pylint:disable=redefined-builtin """Return the attribute names defined by the interface.""" if not all: return self.__attrs.keys() @@ -630,7 +660,7 @@ def names(self, all=False): def __iter__(self): return iter(self.names(all=True)) - def namesAndDescriptions(self, all=False): + def namesAndDescriptions(self, all=False): # pylint:disable=redefined-builtin """Return attribute names and descriptions defined by interface.""" if not all: return self.__attrs.items() @@ -670,8 +700,7 @@ def validateInvariants(self, obj, errors=None): except Invalid as e: if errors is None: raise - else: - errors.append(e) + errors.append(e) for base in self.__bases__: try: base.validateInvariants(obj, errors) @@ -717,7 +746,7 @@ def __repr__(self): # pragma: no cover if m: name = '%s.%s' % (m, name) r = "<%s %s>" % (self.__class__.__name__, name) - self._v_repr = r + self._v_repr = r # pylint:disable=attribute-defined-outside-init return r def _call_conform(self, conform): @@ -912,6 +941,7 @@ def _wire(): classImplements(Specification, ISpecification) # We import this here to deal with module dependencies. +# pylint:disable=wrong-import-position from zope.interface.declarations import implementedBy from zope.interface.declarations import providedBy from zope.interface.exceptions import InvalidInterface diff --git a/src/zope/interface/tests/test_interface.py b/src/zope/interface/tests/test_interface.py index e7ae5475..076fc444 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -536,7 +536,7 @@ def test_ctor_attrs_w___locals__(self): self.assertEqual(inst.__name__, 'ITesting') self.assertEqual(inst.__doc__, '') self.assertEqual(inst.__bases__, ()) - self.assertEqual(inst.names(), ATTRS.keys()) + self.assertEqual(list(inst.names()), []) def test_ctor_attrs_w___annotations__(self): ATTRS = {'__annotations__': {}} @@ -545,7 +545,7 @@ def test_ctor_attrs_w___annotations__(self): self.assertEqual(inst.__name__, 'ITesting') self.assertEqual(inst.__doc__, '') self.assertEqual(inst.__bases__, ()) - self.assertEqual(inst.names(), ATTRS.keys()) + self.assertEqual(list(inst.names()), []) def test_ctor_attrs_w__decorator_non_return(self): from zope.interface.interface import _decorator_non_return From d9f06470f9c45d0710c00e680806a3577b5617f1 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 13:41:59 -0500 Subject: [PATCH 07/12] Use a descriptor for __module__ 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 --- src/zope/interface/_compat.py | 1 + .../_zope_interface_coptimizations.c | 36 +++----- src/zope/interface/interface.py | 89 ++++++++++++++----- src/zope/interface/tests/test_registry.py | 5 +- 4 files changed, 83 insertions(+), 48 deletions(-) diff --git a/src/zope/interface/_compat.py b/src/zope/interface/_compat.py index a57951a5..3587463c 100644 --- a/src/zope/interface/_compat.py +++ b/src/zope/interface/_compat.py @@ -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 diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c index adc73511..5a4fc928 100644 --- a/src/zope/interface/_zope_interface_coptimizations.c +++ b/src/zope/interface/_zope_interface_coptimizations.c @@ -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} }; @@ -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; } @@ -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 diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index d17c8dc1..aa66c22d 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -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 as calculate_ro from zope.interface import ro @@ -238,6 +239,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 :) @@ -246,36 +297,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 @@ -559,12 +591,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 = {} @@ -581,8 +619,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): @@ -799,7 +839,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 '') + self._get_str_info() def __repr__(self): return "<%s.%s object at 0x%x %s>" % ( diff --git a/src/zope/interface/tests/test_registry.py b/src/zope/interface/tests/test_registry.py index db91c52d..2df4ae8d 100644 --- a/src/zope/interface/tests/test_registry.py +++ b/src/zope/interface/tests/test_registry.py @@ -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' From a9d90f4418315098686bcff9b978ab2572000df9 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 17:46:21 -0500 Subject: [PATCH 08/12] Move to a metaclass for handling __module__. This offers the absolute best performance at what seems like reasonable complexity. +-------------------------------------------------------------+----------------+-------------------------------+ | Benchmark | 38-master-full | 38-faster-meta | +=============================================================+================+===============================+ | read __module__ | 41.8 ns | 40.9 ns: 1.02x faster (-2%) | +-------------------------------------------------------------+----------------+-------------------------------+ | read __name__ | 41.8 ns | 39.9 ns: 1.05x faster (-5%) | +-------------------------------------------------------------+----------------+-------------------------------+ | read providedBy | 56.9 ns | 58.4 ns: 1.03x slower (+3%) | +-------------------------------------------------------------+----------------+-------------------------------+ | query adapter (no registrations) | 3.85 ms | 2.95 ms: 1.31x faster (-24%) | +-------------------------------------------------------------+----------------+-------------------------------+ | query adapter (all trivial registrations) | 4.62 ms | 3.63 ms: 1.27x faster (-21%) | +-------------------------------------------------------------+----------------+-------------------------------+ | query adapter (all trivial registrations, wide inheritance) | 51.8 us | 42.2 us: 1.23x faster (-19%) | +-------------------------------------------------------------+----------------+-------------------------------+ | query adapter (all trivial registrations, deep inheritance) | 52.0 us | 41.7 us: 1.25x faster (-20%) | +-------------------------------------------------------------+----------------+-------------------------------+ | sort interfaces | 234 us | 29.9 us: 7.84x faster (-87%) | +-------------------------------------------------------------+----------------+-------------------------------+ | sort mixed | 569 us | 340 us: 1.67x faster (-40%) | +-------------------------------------------------------------+----------------+-------------------------------+ | contains (empty dict) | 135 ns | 55.2 ns: 2.44x faster (-59%) | +-------------------------------------------------------------+----------------+-------------------------------+ | contains (populated dict: interfaces) | 137 ns | 56.1 ns: 2.45x faster (-59%) | +-------------------------------------------------------------+----------------+-------------------------------+ | contains (populated list: interfaces) | 39.7 us | 2.96 us: 13.42x faster (-93%) | +-------------------------------------------------------------+----------------+-------------------------------+ | contains (populated dict: implementedBy) | 137 ns | 55.2 ns: 2.48x faster (-60%) | +-------------------------------------------------------------+----------------+-------------------------------+ | contains (populated list: implementedBy) | 40.6 us | 24.1 us: 1.68x faster (-41%) | +-------------------------------------------------------------+----------------+-------------------------------+ Not significant (2): read __doc__; sort implementedBy --- benchmarks/micro.py | 104 +++++++++++++++++-- src/zope/interface/interface.py | 170 ++++++++++++++++++++------------ 2 files changed, 203 insertions(+), 71 deletions(-) diff --git a/benchmarks/micro.py b/benchmarks/micro.py index 779298c1..a9527dba 100644 --- a/benchmarks/micro.py +++ b/benchmarks/micro.py @@ -2,6 +2,7 @@ from zope.interface import Interface from zope.interface import classImplements +from zope.interface import implementedBy from zope.interface.interface import InterfaceClass from zope.interface.registry import Components @@ -12,6 +13,30 @@ for i in range(100) ] +class IWideInheritance(*ifaces): + """ + Inherits from 100 unrelated interfaces. + """ + +class WideInheritance(object): + pass +classImplements(WideInheritance, IWideInheritance) + +def make_deep_inheritance(): + children = [] + base = Interface + for iface in ifaces: + child = InterfaceClass('IDerived' + base.__name__, (iface, base,), {}) + base = child + children.append(child) + return children + +deep_ifaces = make_deep_inheritance() + +class DeepestInheritance(object): + pass +classImplements(DeepestInheritance, deep_ifaces[-1]) + def make_implementer(iface): c = type('Implementer' + iface.__name__, (object,), {}) classImplements(c, iface) @@ -37,7 +62,21 @@ def bench_in(loops, o): return pyperf.perf_counter() - t0 -def bench_query_adapter(loops, components): +def bench_sort(loops, objs): + import random + rand = random.Random(8675309) + + shuffled = list(objs) + rand.shuffle(shuffled) + + t0 = pyperf.perf_counter() + for _ in range(loops): + for _ in range(INNER): + sorted(shuffled) + + return pyperf.perf_counter() - t0 + +def bench_query_adapter(loops, components, objs=providers): # One time through to prime the caches for iface in ifaces: for provider in providers: @@ -46,10 +85,11 @@ def bench_query_adapter(loops, components): t0 = pyperf.perf_counter() for _ in range(loops): for iface in ifaces: - for provider in providers: + for provider in objs: components.queryAdapter(provider, iface) return pyperf.perf_counter() - t0 + def bench_getattr(loops, name, get=getattr): t0 = pyperf.perf_counter() for _ in range(loops): @@ -68,10 +108,6 @@ def bench_getattr(loops, name, get=getattr): 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, @@ -108,7 +144,6 @@ def bench_getattr(loops, name, get=getattr): ) def populate_components(): - def factory(o): return 42 @@ -126,6 +161,43 @@ def factory(o): inner_loops=1 ) +runner.bench_time_func( + 'query adapter (all trivial registrations, wide inheritance)', + bench_query_adapter, + populate_components(), + [WideInheritance()], + inner_loops=1 +) + +runner.bench_time_func( + 'query adapter (all trivial registrations, deep inheritance)', + bench_query_adapter, + populate_components(), + [DeepestInheritance()], + inner_loops=1 +) + +runner.bench_time_func( + 'sort interfaces', + bench_sort, + ifaces, + inner_loops=INNER, +) + +runner.bench_time_func( + 'sort implementedBy', + bench_sort, + [implementedBy(p) for p in implementers], + inner_loops=INNER, +) + +runner.bench_time_func( + 'sort mixed', + bench_sort, + [implementedBy(p) for p in implementers] + ifaces, + inner_loops=INNER, +) + runner.bench_time_func( 'contains (empty dict)', bench_in, @@ -134,15 +206,29 @@ def factory(o): ) runner.bench_time_func( - 'contains (populated dict)', + 'contains (populated dict: interfaces)', bench_in, {k: k for k in ifaces}, inner_loops=INNER ) runner.bench_time_func( - 'contains (populated list)', + 'contains (populated list: interfaces)', bench_in, ifaces, inner_loops=INNER ) + +runner.bench_time_func( + 'contains (populated dict: implementedBy)', + bench_in, + {implementedBy(p): 1 for p in implementers}, + inner_loops=INNER +) + +runner.bench_time_func( + 'contains (populated list: implementedBy)', + bench_in, + [implementedBy(p) for p in implementers], + inner_loops=INNER +) diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index aa66c22d..3818b89c 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -20,7 +20,6 @@ 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 as calculate_ro from zope.interface import ro @@ -239,56 +238,6 @@ 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 :) @@ -307,7 +256,10 @@ def __init__(self, name=None, module=None): def _call_conform(self, conform): raise NotImplementedError - __module__ = _ModuleDescriptor(__name__) + @property + def __module_property__(self): + # This is for _InterfaceMetaClass + return self.__ibmodule__ def __call__(self, obj, alternate=_marker): """Adapt an object to the interface @@ -578,7 +530,105 @@ def get(self, name, default=None): return default if attr is None else attr -class InterfaceClass(InterfaceBase, Element, Specification): +class _InterfaceMetaClass(type): + # Handling ``__module__`` on ``InterfaceClass`` is tricky. We need + # to be able to read it on a type and get the expected string. We + # also need to be able to set it on an instance and get the value + # we set. So far so good. But what gets tricky is that we'd like + # to store the value in the C structure (``__ibmodule__``) for + # direct access during equality, sorting, and hashing. "No + # problem, you think, I'll just use a property" (well, the C + # equivalents, ``PyMemberDef`` or ``PyGetSetDef``). + # + # Except there is a problem. When a subclass is created, the + # metaclass (``type``) always automatically puts the expected + # string in the class's dictionary under ``__module__``, thus + # overriding the property inherited from the superclass. Writing + # ``Subclass.__module__`` still works, but + # ``instance_of_subclass.__module__`` fails. + # + # There are multiple ways to workaround this: + # + # (1) Define ``__getattribute__`` to watch for ``__module__`` and return + # the C storage. + # + # This works, but slows down *all* attribute access (except, + # ironically, to ``__module__``) by about 25% (40ns becomes 50ns) + # (when implemented in C). Since that includes methods like + # ``providedBy``, that's probably not acceptable. + # + # All the other methods involve modifying subclasses. This can be + # done either on the fly in some cases, as instances are + # constructed, or by using a metaclass. These next few can be done on the fly. + # + # (2) Make ``__module__`` a descriptor in each subclass dictionary. + # It can't be a straight up ``@property`` descriptor, though, because accessing + # it on the class returns a ``property`` object, not the desired string. + # + # (3) Implement a data descriptor (``__get__`` and ``__set__``) that + # is both a string, and also does the redirect of ``__module__`` to ``__ibmodule__`` + # and does the correct thing with the ``instance`` argument to ``__get__`` is None + # (returns the class's value.) + # + # This works, preserves the ability to read and write + # ``__module__``, and eliminates any penalty accessing other + # attributes. But it slows down accessing ``__module__`` of instances by 200% + # (40ns to 124ns). + # + # (4) As in the last step, but make it a non-data descriptor (no ``__set__``). + # + # If you then *also* store a copy of ``__ibmodule__`` in + # ``__module__`` in the instances dict, reading works for both + # class and instance and is full speed for instances. But the cost + # is storage space, and you can't write to it anymore, not without + # things getting out of sync. + # + # (Actually, ``__module__`` was never meant to be writable. Doing + # so would break BTrees and normal dictionaries, as well as the + # repr, maybe more.) + # + # That leaves us with a metaclass. Here we can have our cake and + # eat it too: no extra storage, and C-speed access to the + # underlying storage. The only cost is that metaclasses tend to + # make people's heads hurt. (But still less than the descriptor-is-string, I think.) + + def __new__(cls, name, bases, attrs): + try: + # Figure out what module defined the interface. + # This is how cPython figures out the module of + # a class, but of course it does it in C. :-/ + __module__ = sys._getframe(1).f_globals['__name__'] + except (AttributeError, KeyError): # pragma: no cover + pass + # Get the C optimized __module__ accessor and give it + # to the new class. + moduledescr = InterfaceBase.__dict__['__module__'] + if isinstance(moduledescr, str): + # We're working with the Python implementation, + # not the C version + moduledescr = InterfaceBase.__dict__['__module_property__'] + attrs['__module__'] = moduledescr + kind = type.__new__(cls, name, bases, attrs) + kind.__module = __module__ + return kind + + @property + def __module__(cls): + return cls.__module + + def __repr__(cls): + return "" % ( + cls.__module, + cls.__name__, + ) + +_InterfaceClassBase = _InterfaceMetaClass( + 'InterfaceClass', + (InterfaceBase, Element, Specification), + {} +) + +class InterfaceClass(_InterfaceClassBase): """ Prototype (scarecrow) Interfaces Implementation. @@ -591,15 +641,11 @@ 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): + # We don't call our metaclass parent directly + # pylint:disable=non-parent-init-called + # pylint:disable=super-init-not-called if not all(isinstance(base, InterfaceClass) for base in bases): raise TypeError('Expected base interfaces') @@ -620,9 +666,9 @@ def __init__(self, name, bases=(), attrs=None, __doc__=None, # pylint:disable=r pass InterfaceBase.__init__(self, name, __module__) - assert '__module__' not in self.__dict__ - assert self.__module__ == __module__, (self.__module__, __module__, self.__ibmodule__) + assert self.__ibmodule__ is self.__module__ is __module__ + d = attrs.get('__doc__') if d is not None: if not isinstance(d, Attribute): From 1094bee45cc6d56f9f0e282dba6ca943577a1e7b Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 18:31:10 -0500 Subject: [PATCH 09/12] Several small tweaks to GC and deletion handling. Several places needed to, essentially, call super. --- .../interface/_zope_interface_coptimizations.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c index 5a4fc928..42debb34 100644 --- a/src/zope/interface/_zope_interface_coptimizations.c +++ b/src/zope/interface/_zope_interface_coptimizations.c @@ -330,6 +330,9 @@ Spec_clear(Spec* self) static void Spec_dealloc(Spec* self) { + /* PyType_GenericAlloc that you get when you don't + specify a tp_alloc always tracks the object. */ + PyObject_GC_UnTrack((PyObject *)self); if (self->weakreflist != NULL) { PyObject_ClearWeakRefs(OBJECT(self)); } @@ -562,7 +565,7 @@ CPB_traverse(CPB* self, visitproc visit, void* arg) { Py_VISIT(self->_cls); Py_VISIT(self->_implements); - return 0; + return Spec_traverse((Spec*)self, visit, arg); } static int @@ -570,14 +573,16 @@ CPB_clear(CPB* self) { Py_CLEAR(self->_cls); Py_CLEAR(self->_implements); + Spec_clear((Spec*)self); return 0; } static void CPB_dealloc(CPB* self) { + PyObject_GC_UnTrack((PyObject *)self); CPB_clear(self); - Py_TYPE(self)->tp_free(OBJECT(self)); + Spec_dealloc((Spec*)self); } static PyMemberDef CPB_members[] = { @@ -798,7 +803,7 @@ IB_traverse(IB* self, visitproc visit, void* arg) { Py_VISIT(self->__name__); Py_VISIT(self->__module__); - return 0; + return Spec_traverse((Spec*)self, visit, arg); } static int @@ -806,14 +811,15 @@ IB_clear(IB* self) { Py_CLEAR(self->__name__); Py_CLEAR(self->__module__); - return 0; + return Spec_clear((Spec*)self); } static void IB_dealloc(IB* self) { + PyObject_GC_UnTrack((PyObject *)self); IB_clear(self); - Py_TYPE(self)->tp_free(OBJECT(self)); + Spec_dealloc((Spec*)self); } static PyMemberDef IB_members[] = { From a2687a64253c76b0d7452d102f521e7f680b5306 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 12 Mar 2020 07:34:55 -0500 Subject: [PATCH 10/12] Add tests for comparing InterfaceClass/Implements objects to things without the required attributes. And fix the C handling of this case. --- CHANGES.rst | 17 +++ .../_zope_interface_coptimizations.c | 22 ++-- src/zope/interface/interface.py | 10 ++ src/zope/interface/tests/test_declarations.py | 11 +- src/zope/interface/tests/test_interface.py | 113 +++++++++++++++++- 5 files changed, 159 insertions(+), 14 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3d9bb5df..80c5c487 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -158,12 +158,18 @@ - Fix a potential interpreter crash in the low-level adapter registry lookup functions. See issue 11. +<<<<<<< HEAD - Adopt Python's standard `C3 resolution order `_ to compute the ``__iro__`` and ``__sro__`` of interfaces, with tweaks to support additional cases that are common in interfaces but disallowed for Python classes. Previously, an ad-hoc ordering that made no particular guarantees was used. +======= +- Use Python's standard C3 resolution order to compute the + ``__iro__`` and ``__sro__`` of interfaces. Previously, an ad-hoc + ordering that made no particular guarantees was used. +>>>>>>> Add tests for comparing InterfaceClass/Implements objects to things without the required attributes. This has many beneficial properties, including the fact that base interface and base classes tend to appear near the end of the @@ -210,6 +216,17 @@ hierarchy, ``Interface`` could be assigned too high a priority. See `issue 8 `_. +- Implement sorting, equality, and hashing in C for ``Interface`` + objects. In micro benchmarks, this makes those operations 40% to 80% + faster. This translates to a 20% speed up in querying adapters. + + Note that this changes certain implementation details. In + particular, ``InterfaceClass`` now has a non-default metaclass, and + it is enforced that ``__module__`` in instances of + ``InterfaceClass`` is read-only. + + See `PR 183 `_. + 4.7.2 (2020-03-10) ================== diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c index 42debb34..4e09614c 100644 --- a/src/zope/interface/_zope_interface_coptimizations.c +++ b/src/zope/interface/_zope_interface_coptimizations.c @@ -895,21 +895,23 @@ IB_richcompare(IB* self, PyObject* other, int op) } if (PyObject_TypeCheck(other, &InterfaceBaseType)) { + // This branch borrows references. No need to clean + // up if otherib is not null. otherib = (IB*)other; othername = otherib->__name__; othermod = otherib->__module__; } else { othername = PyObject_GetAttrString(other, "__name__"); - // TODO: Optimize this case. - if (othername == NULL) { - PyErr_Clear(); - othername = PyNative_FromString(""); + if (othername) { + othermod = PyObject_GetAttrString(other, "__module__"); } - othermod = PyObject_GetAttrString(other, "__module__"); - if (othermod == NULL) { - PyErr_Clear(); - othermod = PyNative_FromString(""); + if (!othername || !othermod) { + if (PyErr_Occurred() && PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); + oresult = Py_NotImplemented; + } + goto cleanup; } } #if 0 @@ -928,15 +930,17 @@ IB_richcompare(IB* self, PyObject* other, int op) result = PyObject_RichCompareBool(self->__module__, othermod, op); } // If either comparison failed, we have an error set. + // Leave oresult NULL so we raise it. if (result == -1) { goto cleanup; } oresult = result ? Py_True : Py_False; - Py_INCREF(oresult); cleanup: + Py_XINCREF(oresult); + if (!otherib) { Py_XDECREF(othername); Py_XDECREF(othermod); diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index 3818b89c..01af4226 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -169,6 +169,7 @@ def isOrExtends(self, interface): __call__ = isOrExtends + class NameAndModuleComparisonMixin(object): # Internal use. Implement the basic sorting operators (but not (in)equality # or hashing). Subclasses must provide ``__name__`` and ``__module__`` @@ -182,6 +183,7 @@ class NameAndModuleComparisonMixin(object): # pylint:disable=assigning-non-slot __slots__ = () + def _compare(self, other): """ Compare *self* to *other* based on ``__name__`` and ``__module__``. @@ -193,6 +195,14 @@ def _compare(self, other): If *other* does not have ``__name__`` or ``__module__``, then return ``NotImplemented``. + .. caution:: + This allows comparison to things well outside the type hierarchy, + perhaps not symmetrically. + + For example, ``class Foo(object)`` and ``class Foo(Interface)`` + in the same file would compare equal, depending on the order of + operands. Writing code like this by hand would be unusual, but it could + happen with dynamic creation of types and interfaces. None is treated as a pseudo interface that implies the loosest contact possible, no contract. For that reason, all interfaces diff --git a/src/zope/interface/tests/test_declarations.py b/src/zope/interface/tests/test_declarations.py index 0cdb3264..c391cdbb 100644 --- a/src/zope/interface/tests/test_declarations.py +++ b/src/zope/interface/tests/test_declarations.py @@ -17,6 +17,7 @@ from zope.interface._compat import _skip_under_py3k from zope.interface.tests import OptimizationTestMixin +from zope.interface.tests.test_interface import NameAndModuleComparisonTestsMixin class _Py3ClassAdvice(object): @@ -296,7 +297,8 @@ def test_get_always_default(self): self.assertIsNone(self._getEmpty().get('name')) self.assertEqual(self._getEmpty().get('name', 42), 42) -class TestImplements(unittest.TestCase): +class TestImplements(NameAndModuleComparisonTestsMixin, + unittest.TestCase): def _getTargetClass(self): from zope.interface.declarations import Implements @@ -305,6 +307,13 @@ def _getTargetClass(self): def _makeOne(self, *args, **kw): return self._getTargetClass()(*args, **kw) + def _makeOneToCompare(self): + from zope.interface.declarations import implementedBy + class A(object): + pass + + return implementedBy(A) + def test_ctor_no_bases(self): impl = self._makeOne() self.assertEqual(impl.inherit, None) diff --git a/src/zope/interface/tests/test_interface.py b/src/zope/interface/tests/test_interface.py index 076fc444..be3323d0 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -255,7 +255,112 @@ def _providedBy(obj): self.assertTrue(sb.providedBy(object())) -class InterfaceBaseTestsMixin(CleanUp): +class NameAndModuleComparisonTestsMixin(CleanUp): + + def _makeOneToCompare(self): + return self._makeOne('a', 'b') + + def __check_NotImplemented_comparison(self, name): + # Without the correct attributes of __name__ and __module__, + # comparison switches to the reverse direction. + + import operator + ib = self._makeOneToCompare() + op = getattr(operator, name) + meth = getattr(ib, '__%s__' % name) + + # If either the __name__ or __module__ attribute + # is missing from the other object, then we return + # NotImplemented. + class RaisesErrorOnMissing(object): + Exc = AttributeError + def __getattribute__(self, name): + try: + return object.__getattribute__(self, name) + except AttributeError: + exc = RaisesErrorOnMissing.Exc + raise exc(name) + + class RaisesErrorOnModule(RaisesErrorOnMissing): + def __init__(self): + self.__name__ = 'foo' + @property + def __module__(self): + raise AttributeError + + class RaisesErrorOnName(RaisesErrorOnMissing): + def __init__(self): + self.__module__ = 'foo' + + self.assertEqual(RaisesErrorOnModule().__name__, 'foo') + self.assertEqual(RaisesErrorOnName().__module__, 'foo') + with self.assertRaises(AttributeError): + getattr(RaisesErrorOnModule(), '__module__') + with self.assertRaises(AttributeError): + getattr(RaisesErrorOnName(), '__name__') + + for cls in RaisesErrorOnModule, RaisesErrorOnName: + self.assertIs(meth(cls()), NotImplemented) + + # If the other object has a comparison function, returning + # NotImplemented means Python calls it. + + class AllowsAnyComparison(RaisesErrorOnMissing): + def __eq__(self, other): + return True + __lt__ = __eq__ + __le__ = __eq__ + __gt__ = __eq__ + __ge__ = __eq__ + __ne__ = __eq__ + + self.assertTrue(op(ib, AllowsAnyComparison())) + self.assertIs(meth(AllowsAnyComparison()), NotImplemented) + + # If it doesn't have the comparison, Python raises a TypeError. + class AllowsNoComparison(object): + __eq__ = None + __lt__ = __eq__ + __le__ = __eq__ + __gt__ = __eq__ + __ge__ = __eq__ + __ne__ = __eq__ + + self.assertIs(meth(AllowsNoComparison()), NotImplemented) + with self.assertRaises(TypeError): + op(ib, AllowsNoComparison()) + + # Errors besides AttributeError are passed + class MyException(Exception): + pass + + RaisesErrorOnMissing.Exc = MyException + + with self.assertRaises(MyException): + getattr(RaisesErrorOnModule(), '__module__') + with self.assertRaises(MyException): + getattr(RaisesErrorOnName(), '__name__') + + for cls in RaisesErrorOnModule, RaisesErrorOnName: + with self.assertRaises(MyException): + op(ib, cls()) + with self.assertRaises(MyException): + meth(cls()) + + def test__lt__NotImplemented(self): + self.__check_NotImplemented_comparison('lt') + + def test__le__NotImplemented(self): + self.__check_NotImplemented_comparison('le') + + def test__gt__NotImplemented(self): + self.__check_NotImplemented_comparison('gt') + + def test__ge__NotImplemented(self): + self.__check_NotImplemented_comparison('ge') + + +class InterfaceBaseTestsMixin(NameAndModuleComparisonTestsMixin): # Tests for both C and Python implementation def _getTargetClass(self): @@ -266,13 +371,13 @@ def _getFallbackClass(self): from zope.interface.interface import InterfaceBasePy return InterfaceBasePy - def _makeOne(self, object_should_provide): + def _makeOne(self, object_should_provide=False, name=None, module=None): class IB(self._getTargetClass()): def _call_conform(self, conform): return conform(self) def providedBy(self, obj): return object_should_provide - return IB() + return IB(name, module) def test___call___w___conform___returning_value(self): ib = self._makeOne(False) @@ -280,7 +385,7 @@ def test___call___w___conform___returning_value(self): class _Adapted(object): def __conform__(self, iface): return conformed - self.assertTrue(ib(_Adapted()) is conformed) + self.assertIs(ib(_Adapted()), conformed) def test___call___wo___conform___ob_no_provides_w_alternate(self): ib = self._makeOne(False) From 6be183e34defe640bbad3d898fa3468bf292438a Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Mon, 16 Mar 2020 09:10:46 -0500 Subject: [PATCH 11/12] Add additional tests for assigning to Interface.__module__. --- CHANGES.rst | 6 ------ src/zope/interface/tests/test_interface.py | 7 +++++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 80c5c487..9ba0d8b6 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -158,18 +158,12 @@ - Fix a potential interpreter crash in the low-level adapter registry lookup functions. See issue 11. -<<<<<<< HEAD - Adopt Python's standard `C3 resolution order `_ to compute the ``__iro__`` and ``__sro__`` of interfaces, with tweaks to support additional cases that are common in interfaces but disallowed for Python classes. Previously, an ad-hoc ordering that made no particular guarantees was used. -======= -- Use Python's standard C3 resolution order to compute the - ``__iro__`` and ``__sro__`` of interfaces. Previously, an ad-hoc - ordering that made no particular guarantees was used. ->>>>>>> Add tests for comparing InterfaceClass/Implements objects to things without the required attributes. This has many beneficial properties, including the fact that base interface and base classes tend to appear near the end of the diff --git a/src/zope/interface/tests/test_interface.py b/src/zope/interface/tests/test_interface.py index be3323d0..7b7b7e8d 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -1121,6 +1121,13 @@ class ISpam(Interface): ISpam.__class__ = MyInterfaceClass self.assertEqual(ISpam(1), (1,)) + def test__module__is_readonly(self): + inst = self._makeOne() + with self.assertRaises((AttributeError, TypeError)): + # CPython 2.7 raises TypeError. Everything else + # raises AttributeError. + inst.__module__ = 'different.module' + class InterfaceTests(unittest.TestCase): From dc719e296d6f8ccf5d989414d0af6d645a93c940 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 18 Mar 2020 16:59:11 -0500 Subject: [PATCH 12/12] Remove untested except in the metaclass __new__. Reviewers weren't sure how it could be raised. --- src/zope/interface/interface.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index 01af4226..2d9a873f 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -603,13 +603,11 @@ class _InterfaceMetaClass(type): # make people's heads hurt. (But still less than the descriptor-is-string, I think.) def __new__(cls, name, bases, attrs): - try: - # Figure out what module defined the interface. - # This is how cPython figures out the module of - # a class, but of course it does it in C. :-/ - __module__ = sys._getframe(1).f_globals['__name__'] - except (AttributeError, KeyError): # pragma: no cover - pass + # Figure out what module defined the interface. + # This is copied from ``InterfaceClass.__init__``; + # reviewers aren't sure how AttributeError or KeyError + # could be raised. + __module__ = sys._getframe(1).f_globals['__name__'] # Get the C optimized __module__ accessor and give it # to the new class. moduledescr = InterfaceBase.__dict__['__module__']