From 9e399071f0d033e36dd92cf2cc35d7c09505f9af Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Tue, 17 Mar 2020 08:28:12 -0500 Subject: Move the one-base optimization down a level, and enable using pre-calculated __sro__ for caching. In my local 'load the world' test, this went from ~7800 full C3 merges to about ~1100. Also take steps to avoid triggering false positive warnings about changed ROs when it's *just* Interface that moved. --- src/zope/interface/interface.py | 31 +++++++++--------- src/zope/interface/ro.py | 65 +++++++++++++++++++++++++++---------- src/zope/interface/tests/test_ro.py | 21 ++++++++++++ 3 files changed, 84 insertions(+), 33 deletions(-) diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index 34f54ba..a8eac01 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -21,8 +21,8 @@ import weakref from zope.interface._compat import _use_c_impl from zope.interface.exceptions import Invalid -from zope.interface.ro import ro -from zope.interface.ro import C3 +from zope.interface.ro import ro as calculate_ro +from zope.interface import ro __all__ = [ # Most of the public API from this module is directly exported @@ -322,29 +322,29 @@ class Specification(SpecificationBase): # # So we force the issue by mutating the resolution order. - # TODO: Caching. Perhaps make ro.C3 able to re-use the computed ``__sro__`` - # instead of re-doing it for the entire tree. - base_count = len(self._bases) + # Note that we let C3 use pre-computed __sro__ for our bases. + # This requires that by the time this method is invoked, our bases + # have settled their SROs. Thus, ``changed()`` must first + # update itself before telling its descendents of changes. + sro = calculate_ro(self, base_mros={ + b: b.__sro__ + for b in self.__bases__ + }) + root = self._ROOT + if root is not None and sro and sro[-1] is not root: + # In one dataset of 1823 Interface objects, 1117 ClassProvides objects, + # sro[-1] was root 4496 times, and only not root 118 times. So it's + # probably worth checking. - if base_count == 1: - # Fast path: One base makes it trivial to calculate - # the MRO. - sro = [self] - sro.extend(self.__bases__[0].__sro__) - else: - sro = ro(self) - if self._ROOT is not None: # Once we don't have to deal with old-style classes, # we can add a check and only do this if base_count > 1, # if we tweak the bootstrapping for ```` - root = self._ROOT sro = [ x for x in sro if x is not root ] sro.append(root) - assert sro[-1] is root, sro return sro @@ -705,6 +705,7 @@ Interface._calculate_sro = lambda: (Interface,) Interface.changed(Interface) assert Interface.__sro__ == (Interface,) Specification._ROOT = Interface +ro._ROOT = Interface class Attribute(Element): diff --git a/src/zope/interface/ro.py b/src/zope/interface/ro.py index dbffb53..7cb4f55 100644 --- a/src/zope/interface/ro.py +++ b/src/zope/interface/ro.py @@ -189,11 +189,25 @@ class _ClassBoolFromEnv(object): return val +class _StaticMRO(object): + # A previously resolved MRO, supplied by the caller. + # Used in place of calculating it. + + had_inconsistency = None # We don't know... + + def __init__(self, C, mro): + self.leaf = C + self.__mro = tuple(mro) + + def mro(self): + return list(self.__mro) + + class C3(object): # Holds the shared state during computation of an MRO. @staticmethod - def resolver(C, strict): + def resolver(C, strict, base_mros): strict = strict if strict is not None else C3.STRICT_RO factory = C3 if strict: @@ -201,7 +215,13 @@ class C3(object): elif C3.TRACK_BAD_IRO: factory = _TrackingC3 - return factory(C, {}) + memo = {} + base_mros = base_mros or {} + for base, mro in base_mros.items(): + assert base in C.__bases__ + memo[base] = _StaticMRO(base, mro) + + return factory(C, memo) __mro = None __legacy_ro = None @@ -229,6 +249,9 @@ class C3(object): self.bases_had_inconsistency = any(base.had_inconsistency for base in base_resolvers) + if len(C.__bases__) == 1: + self.__mro = [C] + memo[C.__bases__[0]].mro() + @property def had_inconsistency(self): return self.direct_inconsistency or self.bases_had_inconsistency @@ -520,10 +543,9 @@ class _ROComparison(object): left_lines = [str(x) for x in legacy_report] right_lines = [str(x) for x in c3_report] - # We have the same number of non-empty lines as we do items - # in the resolution order. - assert len(list(filter(None, left_lines))) == len(self.c3_ro) - assert len(list(filter(None, right_lines))) == len(self.c3_ro) + # We have the same number of lines in the report; this is not + # necessarily the same as the number of items in either RO. + assert len(left_lines) == len(right_lines) padding = ' ' * 2 max_left = max(len(x) for x in left_lines) @@ -547,20 +569,16 @@ class _ROComparison(object): return '\n'.join(lines) -def ro(C, strict=None, log_changed_ro=None, use_legacy_ro=None): +# Set to `Interface` once it is defined. This is used to +# avoid logging false positives about changed ROs. +_ROOT = None + +def ro(C, strict=None, base_mros=None, log_changed_ro=None, use_legacy_ro=None): """ ro(C) -> list Compute the precedence list (mro) according to C3. - As an implementation note, this always calculates the full MRO by - examining all the bases recursively. If there are special cases - that can reuse pre-calculated partial MROs, such as a - ``__bases__`` of length one, the caller is responsible for - optimizing that. (This is because this function doesn't know how - to get the complete MRO of a base; it only knows how to get their - ``__bases__``.) - :return: A fresh `list` object. .. versionchanged:: 5.0.0 @@ -568,7 +586,9 @@ def ro(C, strict=None, log_changed_ro=None, use_legacy_ro=None): keyword arguments. These are provisional and likely to be removed in the future. They are most useful for testing. """ - resolver = C3.resolver(C, strict) + # The ``base_mros`` argument is for internal optimization and + # not documented. + resolver = C3.resolver(C, strict, base_mros) mro = resolver.mro() log_changed = log_changed_ro if log_changed_ro is not None else resolver.LOG_CHANGED_IRO @@ -578,7 +598,16 @@ def ro(C, strict=None, log_changed_ro=None, use_legacy_ro=None): legacy_ro = resolver.legacy_ro assert isinstance(legacy_ro, list) assert isinstance(mro, list) - if legacy_ro != mro: + changed = legacy_ro != mro + if changed: + # Did only Interface move? The fix for issue #8 made that + # somewhat common. It's almost certainly not a problem, though, + # so allow ignoring it. + legacy_without_root = [x for x in legacy_ro if x is not _ROOT] + mro_without_root = [x for x in mro if x is not _ROOT] + changed = legacy_without_root != mro_without_root + + if changed: comparison = _ROComparison(resolver, mro, legacy_ro) _logger().warning( "Object %r has different legacy and C3 MROs:\n%s", @@ -602,4 +631,4 @@ def is_consistent(C): Check if the resolution order for *C*, as computed by :func:`ro`, is consistent according to C3. """ - return not C3.resolver(C, False).had_inconsistency + return not C3.resolver(C, False, None).had_inconsistency diff --git a/src/zope/interface/tests/test_ro.py b/src/zope/interface/tests/test_ro.py index 3a516b5..ce0a6f4 100644 --- a/src/zope/interface/tests/test_ro.py +++ b/src/zope/interface/tests/test_ro.py @@ -375,6 +375,27 @@ Object has different legacy and self.assertEqual(iro, legacy_iro) +class TestC3(unittest.TestCase): + def _makeOne(self, C, strict=False, base_mros=None): + from zope.interface.ro import C3 + return C3.resolver(C, strict, base_mros) + + def test_base_mros_given(self): + c3 = self._makeOne(type(self), base_mros={unittest.TestCase: unittest.TestCase.__mro__}) + memo = c3.memo + self.assertIn(unittest.TestCase, memo) + # We used the StaticMRO class + self.assertIsNone(memo[unittest.TestCase].had_inconsistency) + + def test_one_base_optimization(self): + c3 = self._makeOne(type(self)) + # Even though we didn't call .mro() yet, the MRO has been + # computed. + self.assertIsNotNone(c3._C3__mro) # pylint:disable=no-member + c3._merge = None + self.assertEqual(c3.mro(), list(type(self).__mro__)) + + class Test_ROComparison(unittest.TestCase): class MockC3(object): -- cgit v1.2.1 From 8ac3bd088d9b924cfb3170b77b41effd2de39d23 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Fri, 6 Mar 2020 18:38:50 -0600 Subject: 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 ++++ .../interface/_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 1dcbd31..83a0eb8 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 0000000..0494b56 --- /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 180cdef..7341474 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 a8eac01..8707b9e 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 @@ class InterfaceBase(object): 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 @@ class Specification(SpecificationBase): 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 @@ class InterfaceClass(Element, InterfaceBase, Specification): pass self.__module__ = __module__ + assert '__module__' not in self.__dict__ d = attrs.get('__doc__') if d is not None: @@ -471,6 +562,7 @@ class InterfaceClass(Element, InterfaceBase, Specification): 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 @@ class InterfaceClass(Element, InterfaceBase, Specification): 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 @@ class InterfaceClass(Element, InterfaceBase, Specification): 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 @@ class InterfaceClass(Element, InterfaceBase, Specification): 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 da7eb8c..737b229 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -284,7 +284,6 @@ class GenericInterfaceBaseTests(unittest.TestCase): self.assertRaises(TypeError, ib, adapted) - class InterfaceBaseTests(GenericInterfaceBaseTests, OptimizationTestMixin): # Tests that work with the C implementation @@ -1049,6 +1048,7 @@ class InterfaceTests(unittest.TestCase): 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 73613d0..0e33f47 100644 --- a/src/zope/interface/tests/test_sorting.py +++ b/src/zope/interface/tests/test_sorting.py @@ -45,3 +45,20 @@ class Test(unittest.TestCase): 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)) -- cgit v1.2.1 From b9165b790c831b6d8a87a3f27d4f135143494ff8 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Tue, 10 Mar 2020 14:17:28 -0500 Subject: 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 8707b9e..1ed1b6f 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -20,6 +20,7 @@ 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 @@ -485,8 +486,18 @@ class Specification(SpecificationBase): 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 @@ class _ModuleDescriptor(object): 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 @@ class _MC(type): _InterfaceClassBase = _MC( 'InterfaceClass', (Element, InterfaceBase, Specification), - {'__module__': __name__}) + {'__module__': __name__, '__qualname__': __name__ + 'InterfaceClass'}) class InterfaceClass(_InterfaceClassBase): -- cgit v1.2.1 From 01e0a7e36cecbd0a36fd49b08b1c21e0a5bba119 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Tue, 10 Mar 2020 17:47:21 -0500 Subject: 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 0494b56..9031261 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, -- cgit v1.2.1 From 5f4bb3f8ec7798b146c007041ff60aac2ca0566e Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 08:17:11 -0500 Subject: 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 737b229..c6b21ad 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 @@ class ElementTests(unittest.TestCase): 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 @@ class ElementTests(unittest.TestCase): 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 @@ class SpecificationBasePyTests(GenericSpecificationBaseTests): 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 @@ class GenericInterfaceBaseTests(unittest.TestCase): 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 @@ class GenericInterfaceBaseTests(unittest.TestCase): 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 @@ class InterfaceBasePyTests(GenericInterfaceBaseTests): _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 @@ class InterfaceClassTests(unittest.TestCase): 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 @@ class InterfaceClassTests(unittest.TestCase): } 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 @@ class InterfaceClassTests(unittest.TestCase): 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 @@ class InterfaceClassTests(unittest.TestCase): 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 @@ class InterfaceClassTests(unittest.TestCase): 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 @@ class InterfaceClassTests(unittest.TestCase): 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 InterfaceTests(unittest.TestCase): 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 InterfaceTests(unittest.TestCase): 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 InterfaceTests(unittest.TestCase): 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 InterfaceTests(unittest.TestCase): 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 @@ class Test_fromFunction(unittest.TestCase): 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 @@ class Test_fromFunction(unittest.TestCase): 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 @@ class Test_fromMethod(unittest.TestCase): 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) -- cgit v1.2.1 From 7afd59d869e3391ce27a4a07ad0931eb5e7910a1 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 08:28:23 -0500 Subject: 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 c37dffc..8b9ef25 100644 --- a/src/zope/interface/tests/__init__.py +++ b/src/zope/interface/tests/__init__.py @@ -31,3 +31,25 @@ class OptimizationTestMixin(object): 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 c6b21ad..e7ae547 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -25,6 +25,7 @@ import unittest 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 @@ class SpecificationBasePyTests(GenericSpecificationBaseTests): self.assertTrue(sb.providedBy(object())) -class InterfaceBaseTestsMixin(object): +class InterfaceBaseTestsMixin(CleanUp): # Tests for both C and Python implementation def _getTargetClass(self): -- cgit v1.2.1 From 413e716f9fb48338829435c4f243133589eb9fe6 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 12:19:19 -0500 Subject: 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 +++- .../interface/_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 9031261..779298c 100644 --- a/benchmarks/micro.py +++ b/benchmarks/micro.py @@ -27,7 +27,7 @@ providers = [ 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 7341474..adc7351 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 @@ cleanup: } 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 1e9a2ea..502e26e 100644 --- a/src/zope/interface/declarations.py +++ b/src/zope/interface/declarations.py @@ -36,6 +36,7 @@ from zope.interface.advice import addClassAdvisor 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 @@ class _ImmutableDeclaration(Declaration): # # 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 @@ class Implements(Declaration): 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 1ed1b6f..d17c8dc 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 @@ class Element(object): 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 @@ class SpecificationBase(object): 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 @@ class InterfaceBase(object): 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 @@ class InterfaceBase(object): 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 @@ class InterfaceBase(object): 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 @@ class Specification(SpecificationBase): 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 @@ class InterfaceClass(_InterfaceClassBase): 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 @@ class InterfaceClass(_InterfaceClassBase): 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 @@ class InterfaceClass(_InterfaceClassBase): """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 @@ class InterfaceClass(_InterfaceClassBase): 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 @@ class InterfaceClass(_InterfaceClassBase): 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 @@ class InterfaceClass(_InterfaceClassBase): 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 e7ae547..076fc44 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -536,7 +536,7 @@ class InterfaceClassTests(unittest.TestCase): 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 @@ class InterfaceClassTests(unittest.TestCase): 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 -- cgit v1.2.1 From d9f06470f9c45d0710c00e680806a3577b5617f1 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 13:41:59 -0500 Subject: 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 + .../interface/_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 a57951a..3587463 100644 --- a/src/zope/interface/_compat.py +++ b/src/zope/interface/_compat.py @@ -166,4 +166,5 @@ def _use_c_impl(py_impl, name=None, globs=None): # 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 adc7351..5a4fc92 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 @@ cleanup: } -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 d17c8dc..aa66c22 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -20,6 +20,7 @@ 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 @@ -238,6 +239,56 @@ class NameAndModuleComparisonMixin(object): 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 @@ class InterfaceClass(InterfaceBase, Element, Specification): 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 @@ class Attribute(Element): 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 db91c52..2df4ae8 100644 --- a/src/zope/interface/tests/test_registry.py +++ b/src/zope/interface/tests/test_registry.py @@ -2343,9 +2343,10 @@ class UtilityRegistrationTests(unittest.TestCase): 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' -- cgit v1.2.1 From a9d90f4418315098686bcff9b978ab2572000df9 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 17:46:21 -0500 Subject: 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 779298c..a9527db 100644 --- a/benchmarks/micro.py +++ b/benchmarks/micro.py @@ -2,6 +2,7 @@ import pyperf 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 @@ ifaces = [ 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 @@ runner.bench_time_func( ) def populate_components(): - def factory(o): return 42 @@ -126,6 +161,43 @@ runner.bench_time_func( 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 @@ runner.bench_time_func( ) 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 aa66c22..3818b89 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -20,7 +20,6 @@ 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 @@ -239,56 +238,6 @@ class NameAndModuleComparisonMixin(object): 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 @@ class InterfaceBase(NameAndModuleComparisonMixin, SpecificationBasePy): 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 @@ class Specification(SpecificationBase): 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 @@ class InterfaceClass(InterfaceBase, Element, Specification): 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): -- cgit v1.2.1 From 1094bee45cc6d56f9f0e282dba6ca943577a1e7b Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 11 Mar 2020 18:31:10 -0500 Subject: Several small tweaks to GC and deletion handling. Several places needed to, essentially, call super. --- src/zope/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 5a4fc92..42debb3 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[] = { -- cgit v1.2.1 From a2687a64253c76b0d7452d102f521e7f680b5306 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 12 Mar 2020 07:34:55 -0500 Subject: Add tests for comparing InterfaceClass/Implements objects to things without the required attributes. And fix the C handling of this case. --- CHANGES.rst | 17 ++++ .../interface/_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 3d9bb5d..80c5c48 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 42debb3..4e09614 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 3818b89..01af422 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -169,6 +169,7 @@ class SpecificationBase(object): __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 @@ class NameAndModuleComparisonMixin(object): 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 0cdb326..c391cdb 100644 --- a/src/zope/interface/tests/test_declarations.py +++ b/src/zope/interface/tests/test_declarations.py @@ -17,6 +17,7 @@ import unittest 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 @@ class TestImmutableDeclaration(unittest.TestCase): 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 @@ class TestImplements(unittest.TestCase): 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 076fc44..be3323d 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -255,7 +255,112 @@ class SpecificationBasePyTests(GenericSpecificationBaseTests): 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 @@ class InterfaceBaseTestsMixin(CleanUp): 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 @@ class InterfaceBaseTestsMixin(CleanUp): 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) -- cgit v1.2.1 From 6be183e34defe640bbad3d898fa3468bf292438a Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Mon, 16 Mar 2020 09:10:46 -0500 Subject: 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 80c5c48..9ba0d8b 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 be3323d..7b7b7e8 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -1121,6 +1121,13 @@ class InterfaceClassTests(unittest.TestCase): 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): -- cgit v1.2.1 From dc719e296d6f8ccf5d989414d0af6d645a93c940 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 18 Mar 2020 16:59:11 -0500 Subject: 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 01af422..2d9a873 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__'] -- cgit v1.2.1 From 42c5ad2d4f88610bb89d35152c1caa1c52dc6927 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 19 Mar 2020 06:54:33 -0500 Subject: Update comments and add a test for more coverage. --- .gitignore | 1 + benchmarks/.gitignore | 1 + src/zope/interface/declarations.py | 81 ++++++++++++++++----------- src/zope/interface/interface.py | 54 +++++++++++------- src/zope/interface/tests/test_declarations.py | 21 +++++++ tox.ini | 4 +- 6 files changed, 108 insertions(+), 54 deletions(-) create mode 100644 benchmarks/.gitignore diff --git a/.gitignore b/.gitignore index 35946f4..cbf117f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ *.egg-info *.pyc +*.pyo *.so __pycache__ .coverage diff --git a/benchmarks/.gitignore b/benchmarks/.gitignore new file mode 100644 index 0000000..a6c57f5 --- /dev/null +++ b/benchmarks/.gitignore @@ -0,0 +1 @@ +*.json diff --git a/src/zope/interface/declarations.py b/src/zope/interface/declarations.py index 502e26e..d85dbf4 100644 --- a/src/zope/interface/declarations.py +++ b/src/zope/interface/declarations.py @@ -46,6 +46,8 @@ __all__ = [ # re-exported from zope.interface directly. ] +# pylint:disable=too-many-lines + # Registry of class-implementation specifications BuiltinImplementationSpecifications = {} @@ -102,12 +104,14 @@ class Declaration(Specification): def __sub__(self, other): """Remove interfaces from a specification """ - return Declaration( - *[i for i in self.interfaces() - if not [j for j in other.interfaces() - if i.extends(j, 0)] + return Declaration(*[ + i for i in self.interfaces() + if not [ + j + for j in other.interfaces() + if i.extends(j, 0) # non-strict extends ] - ) + ]) def __add__(self, other): """Add two specifications or a specification and an interface @@ -229,7 +233,9 @@ class Implements(NameAndModuleComparisonMixin, declared = () # Weak cache of {class: } for super objects. - # Created on demand. + # Created on demand. These are rare, as of 5.0 anyway. Using a class + # level default doesn't take space in instances. Using _v_attrs would be + # another place to store this without taking space unless needed. _super_cache = None __name__ = '?' @@ -247,7 +253,10 @@ class Implements(NameAndModuleComparisonMixin, return inst def changed(self, originally_changed): - self._super_cache = None + try: + del self._super_cache + except AttributeError: + pass return super(Implements, self).changed(originally_changed) def __repr__(self): @@ -283,7 +292,7 @@ def _implementedBy_super(sup): # that excludes the classes being skipped over but # includes everything else. implemented_by_self = implementedBy(sup.__self_class__) - cache = implemented_by_self._super_cache + cache = implemented_by_self._super_cache # pylint:disable=protected-access if cache is None: cache = implemented_by_self._super_cache = weakref.WeakKeyDictionary() @@ -318,7 +327,7 @@ def _implementedBy_super(sup): @_use_c_impl -def implementedBy(cls): +def implementedBy(cls): # pylint:disable=too-many-return-statements,too-many-branches """Return the interfaces implemented for a class' instances The value returned is an `~zope.interface.interfaces.IDeclaration`. @@ -393,8 +402,7 @@ def implementedBy(cls): cls.__providedBy__ = objectSpecificationDescriptor if (isinstance(cls, DescriptorAwareMetaClasses) - and - '__provides__' not in cls.__dict__): + and '__provides__' not in cls.__dict__): # Make sure we get a __provides__ descriptor cls.__provides__ = ClassProvides( cls, @@ -499,9 +507,9 @@ def _classImplements_ordered(spec, before=(), after=()): def _implements_advice(cls): - interfaces, classImplements = cls.__dict__['__implements_advice_data__'] + interfaces, do_classImplements = cls.__dict__['__implements_advice_data__'] del cls.__implements_advice_data__ - classImplements(cls, *interfaces) + do_classImplements(cls, *interfaces) return cls @@ -533,6 +541,7 @@ class implementer(object): after the class has been created. """ + __slots__ = ('interfaces',) def __init__(self, *interfaces): self.interfaces = interfaces @@ -583,7 +592,7 @@ class implementer_only(object): if isinstance(ob, (FunctionType, MethodType)): # XXX Does this decorator make sense for anything but classes? # I don't think so. There can be no inheritance of interfaces - # on a method pr function.... + # on a method or function.... raise ValueError('The implementer_only decorator is not ' 'supported for methods or functions.') else: @@ -591,11 +600,11 @@ class implementer_only(object): classImplementsOnly(ob, *self.interfaces) return ob -def _implements(name, interfaces, classImplements): +def _implements(name, interfaces, do_classImplements): # This entire approach is invalid under Py3K. Don't even try to fix # the coverage for this block there. :( - frame = sys._getframe(2) - locals = frame.f_locals + frame = sys._getframe(2) # pylint:disable=protected-access + locals = frame.f_locals # pylint:disable=redefined-builtin # Try to make sure we were called from a class def. In 2.2.0 we can't # check for __module__ since it doesn't seem to be added to the locals @@ -606,7 +615,7 @@ def _implements(name, interfaces, classImplements): if '__implements_advice_data__' in locals: raise TypeError(name+" can be used only once in a class definition.") - locals['__implements_advice_data__'] = interfaces, classImplements + locals['__implements_advice_data__'] = interfaces, do_classImplements addClassAdvisor(_implements_advice, depth=3) def implements(*interfaces): @@ -713,7 +722,7 @@ ProvidesClass = Provides # This is a memory optimization to allow objects to share specifications. InstanceDeclarations = weakref.WeakValueDictionary() -def Provides(*interfaces): +def Provides(*interfaces): # pylint:disable=function-redefined """Cache instance declarations Instance declarations are shared among instances that have the same @@ -729,7 +738,7 @@ def Provides(*interfaces): Provides.__safe_for_unpickling__ = True -def directlyProvides(object, *interfaces): +def directlyProvides(object, *interfaces): # pylint:disable=redefined-builtin """Declare interfaces declared directly for an object The arguments after the object are one or more interfaces or interface @@ -765,7 +774,7 @@ def directlyProvides(object, *interfaces): object.__provides__ = Provides(cls, *interfaces) -def alsoProvides(object, *interfaces): +def alsoProvides(object, *interfaces): # pylint:disable=redefined-builtin """Declare interfaces declared directly for an object The arguments after the object are one or more interfaces or interface @@ -777,7 +786,7 @@ def alsoProvides(object, *interfaces): directlyProvides(object, directlyProvidedBy(object), *interfaces) -def noLongerProvides(object, interface): +def noLongerProvides(object, interface): # pylint:disable=redefined-builtin """ Removes a directly provided interface from an object. """ directlyProvides(object, directlyProvidedBy(object) - interface) @@ -794,6 +803,8 @@ class ClassProvidesBase(SpecificationBase): ) def __get__(self, inst, cls): + # member slots are set by subclass + # pylint:disable=no-member if cls is self._cls: # We only work if called on the class we were defined for @@ -815,6 +826,10 @@ class ClassProvides(Declaration, ClassProvidesBase): interfaces a bit quicker. """ + __slots__ = ( + '__args', + ) + def __init__(self, cls, metacls, *interfaces): self._cls = cls self._implements = implementedBy(cls) @@ -835,18 +850,18 @@ class ClassProvides(Declaration, ClassProvidesBase): __get__ = ClassProvidesBase.__get__ -def directlyProvidedBy(object): +def directlyProvidedBy(object): # pylint:disable=redefined-builtin """Return the interfaces directly provided by the given object The value returned is an `~zope.interface.interfaces.IDeclaration`. """ provides = getattr(object, "__provides__", None) - if (provides is None # no spec - or - # We might have gotten the implements spec, as an - # optimization. If so, it's like having only one base, that we - # lop off to exclude class-supplied declarations: - isinstance(provides, Implements) + if ( + provides is None # no spec + # We might have gotten the implements spec, as an + # optimization. If so, it's like having only one base, that we + # lop off to exclude class-supplied declarations: + or isinstance(provides, Implements) ): return _empty @@ -888,8 +903,8 @@ def classProvides(*interfaces): if PYTHON3: raise TypeError(_ADVICE_ERROR % 'provider') - frame = sys._getframe(1) - locals = frame.f_locals + frame = sys._getframe(1) # pylint:disable=protected-access + locals = frame.f_locals # pylint:disable=redefined-builtin # Try to make sure we were called from a class def if (locals is frame.f_globals) or ('__module__' not in locals): @@ -947,8 +962,8 @@ def moduleProvides(*interfaces): directlyProvides(sys.modules[__name__], I1) """ - frame = sys._getframe(1) - locals = frame.f_locals + frame = sys._getframe(1) # pylint:disable=protected-access + locals = frame.f_locals # pylint:disable=redefined-builtin # Try to make sure we were called from a class def if (locals is not frame.f_globals) or ('__name__' not in locals): diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index 2d9a873..e83b512 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -545,7 +545,7 @@ class _InterfaceMetaClass(type): # 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 + # to store the value in the C structure (``InterfaceBase.__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``). @@ -555,12 +555,12 @@ class _InterfaceMetaClass(type): # 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. + # ``Subclass().__module__`` fails. # - # There are multiple ways to workaround this: + # There are multiple ways to work around this: # - # (1) Define ``__getattribute__`` to watch for ``__module__`` and return - # the C storage. + # (1) Define ``InterfaceBase.__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) @@ -575,20 +575,27 @@ class _InterfaceMetaClass(type): # 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.) + # (3) Implement a data descriptor (``__get__`` and ``__set__``) + # that is both a subclass of 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.) (Why must it be a subclass of string? Because + # when it' s in the class's dict, it's defined on an *instance* of the + # metaclass; descriptors in an instance's dict aren't honored --- their + # ``__get__`` is never invoked --- so it must also *be* the value we want + # returned.) # # 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). + # attributes. But it slows down accessing ``__module__`` of + # instances by 200% (40ns to 124ns), requires editing class dicts on the fly + # (in InterfaceClass.__init__), thus slightly slowing down all interface creation, + # and is ugly. # # (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 + # ``__module__`` in the instance's 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. @@ -597,10 +604,16 @@ class _InterfaceMetaClass(type): # 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.) + # That leaves us with a metaclass. (Recall that a class is an + # instance of its metaclass, so properties/descriptors defined in + # the metaclass are used when accessing attributes on the + # instance/class. We'll use that to define ``__module__``.) Here + # we can have our cake and eat it too: no extra storage, and + # C-speed access to the underlying storage. The only substantial + # cost is that metaclasses tend to make people's heads hurt. (But + # still less than the descriptor-is-string, hopefully.) + + __slots__ = () def __new__(cls, name, bases, attrs): # Figure out what module defined the interface. @@ -630,12 +643,14 @@ class _InterfaceMetaClass(type): cls.__name__, ) + _InterfaceClassBase = _InterfaceMetaClass( 'InterfaceClass', (InterfaceBase, Element, Specification), - {} + {'__slots__': ()} ) + class InterfaceClass(_InterfaceClassBase): """ Prototype (scarecrow) Interfaces Implementation. @@ -674,8 +689,9 @@ class InterfaceClass(_InterfaceClassBase): pass InterfaceBase.__init__(self, name, __module__) - assert '__module__' not in self.__dict__ - assert self.__ibmodule__ is self.__module__ is __module__ + # These asserts assisted debugging the metaclass + # assert '__module__' not in self.__dict__ + # assert self.__ibmodule__ is self.__module__ is __module__ d = attrs.get('__doc__') if d is not None: diff --git a/src/zope/interface/tests/test_declarations.py b/src/zope/interface/tests/test_declarations.py index c391cdb..b0875c4 100644 --- a/src/zope/interface/tests/test_declarations.py +++ b/src/zope/interface/tests/test_declarations.py @@ -390,6 +390,27 @@ class TestImplements(NameAndModuleComparisonTestsMixin, self.assertTrue(proxy != implementedByB) self.assertTrue(implementedByB != proxy) + def test_changed_deletes_super_cache(self): + impl = self._makeOne() + self.assertIsNone(impl._super_cache) + self.assertNotIn('_super_cache', impl.__dict__) + + impl._super_cache = 42 + self.assertIn('_super_cache', impl.__dict__) + + impl.changed(None) + self.assertIsNone(impl._super_cache) + self.assertNotIn('_super_cache', impl.__dict__) + + def test_changed_does_not_add_super_cache(self): + impl = self._makeOne() + self.assertIsNone(impl._super_cache) + self.assertNotIn('_super_cache', impl.__dict__) + + impl.changed(None) + self.assertIsNone(impl._super_cache) + self.assertNotIn('_super_cache', impl.__dict__) + class Test_implementedByFallback(unittest.TestCase): diff --git a/tox.ini b/tox.ini index 2c60a50..be90fe3 100644 --- a/tox.ini +++ b/tox.ini @@ -43,7 +43,7 @@ commands = coverage report -i coverage html -i coverage xml -i -depends = py27,py27-pure,py35,py35-pure,py36,py37,py38,py38-cext,pypy,pypy3 +depends = py27,py27-pure,py35,py35-pure,py36,py37,py38,py38-cext,pypy,pypy3,docs parallel_show_output = true [testenv:docs] @@ -51,7 +51,7 @@ basepython = python3 commands = sphinx-build -b html -d docs/_build/doctrees docs docs/_build/html - sphinx-build -b doctest -d docs/_build/doctrees docs docs/_build/doctest + coverage run -p -m sphinx -b doctest -d docs/_build/doctrees docs docs/_build/doctest deps = Sphinx repoze.sphinx.autointerface -- cgit v1.2.1 From 04152ae7d2687bc7f100f4f504a342e84fc5c167 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 19 Mar 2020 07:36:50 -0500 Subject: Another comment update, referencing #163 --- src/zope/interface/interface.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index e83b512..e654b27 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -276,8 +276,13 @@ class InterfaceBase(NameAndModuleComparisonMixin, SpecificationBasePy): """ try: conform = getattr(obj, '__conform__', None) - # XXX: Do we really want to catch BaseException? Shouldn't - # things like MemoryError, KeyboardInterrupt, etc, get through? + # XXX: Ideally, we don't want to catch BaseException. Things + # like MemoryError, KeyboardInterrupt, and other BaseExceptions should + # get through. Here, and everywhere we have bare ``except:`` clauses. + # The bare ``except:`` clauses exist for compatibility with the C code in + # ``_zope_interface_coptimizations.c`` (``ib_call()`` in this case). Both + # implementations would need to change. But beware of things like proxies and + # Acquisition wrappers. See https://github.com/zopefoundation/zope.interface/issues/163 except: # pylint:disable=bare-except conform = None if conform is not None: -- cgit v1.2.1