Remove checking of PURE_PYTHON at build time #151
Conversation
|
Python 3.6 wheels for Mac OS fail because they still use 3.6.0 instead of 3.6.2. I wonder if coverage numbers dropping indicate that unittest -m discover doesn't pick up some of the tests that setup.py test used to?
Adding --- SETUP-PY-TEST.txt 2020-01-21 10:12:23.393695118 +0200
+++ DISCOVER.txt 2020-01-21 10:13:33.035477287 +0200
@@ -1,7 +1,5 @@
-warning: no previously-included files matching '*.dll' found anywhere in distribution
-warning: no previously-included files matching '*.pyo' found anywhere in distribution
-warning: no previously-included files matching 'coverage.xml' found anywhere in distribution
-warning: no previously-included files matching 'appveyor.yml' found anywhere in distribution
+test_import (zope.interface.common.tests.test_import_interfaces.TestInterfaceImport) ... ok
+test_interfaces (zope.interface.common.tests.test_idatetime.TestDateTimeInterfaces) ... ok
test_ObjectSpecification (zope.interface.tests.test_odd_declarations.Test) ... ok
test_classImplements (zope.interface.tests.test_odd_declarations.Test) ... ok
test_classImplementsOnly (zope.interface.tests.test_odd_declarations.Test) ... ok
@@ -875,6 +873,6 @@
test_asStructuredText_with_method_with_docstring (zope.interface.tests.test_document.Test_asStructuredText) ... ok
----------------------------------------------------------------------
-Ran 870 tests in 0.089s
+Ran 872 tests in 0.095s
OK (skipped=3)Coveralls shows me the difference is due to three tiny test files no longer being even imported: I can't find any import statements for any of them. I guess setup.py test was just going along importing every file in that package, including the unused ones? Suggestion: let's remove these unused files altogether. |
jamadden
force-pushed
the
no-pure-python-build
branch
from
edc67df to
d165e3d
Compare
Turns out that the C extensions build and work fine with PyPy, but don't use them by default. Let them be forced, though. Tests needed some refactoring to account for the various permutations.
And tox. It wasn't properly working to report coverage before.
jamadden
force-pushed
the
no-pure-python-build
branch
from
2a06112 to
35f349b
Compare
|
The 0.9% test coverage decrease is on decorator lines, reporting branches not taken. Coverage has an issue reporting decorators when the Python version that runs |
| We don't have that ability with the standard API, | ||
| so we decref here to mimic it. That should be fine if the | ||
| attribute was really in the dictionary, but it could be an | ||
| issue if it was a new object from a descriptor. |
There was a problem hiding this comment.
Is it possible to add an assertion that the refcount is > 1? I.e. make it crash here instead of unpredictably at some point later?
There was a problem hiding this comment.
I agree, it scares me too. I think it can be cleaned up nicely, but the tl;dr is that I'd like to deal with that in a follow-up PR.
There are only four uses of inst_attr. I think it should be possible to eliminate all of them in favor of inlined fields in the structures, instead of putting them in the __dict__ directly, if we use tp_members or tp_getset. That should preserve the suggested speed advantages of inst_attr, while also being mostly compatible:
- Attributes could be read/write accessible to C and Python;
- The attributes would still be listed in
dir; - These are private types so we don't have to worry about extensions at the C level;
- If anyone is accessing these attributes from C, they should be using
PyObject_GetAttr(if they're trying to pull the same trick with the undocumented private API_PyObject_GetDictPtr, they'll lose…but eh, undocumented private API; I didn't find any uses of that in other zopefoundation projects, notably not zope.security or zope.proxy)
The major difference is they won't show up in vars() or __dict__. We'd move them to __slots__ in the Python implementation to mimic that. We may be able to stop most instances of Specification(Base) and especially ClassProvides(Base) from even having dicts; given how common those are that could be a nice memory win.
If that doesn't work out, I would propose adding a companion macro, something like inst_attr_decref that's required to be called. On PyPy it would expand to a real DECREF, on CPython a no-op (I'd also want to add a branch for Python 3 to use the supported API to get an object's dict).
Thoughts?
There was a problem hiding this comment.
I'm a little less scared now.
I'm not quite sure whether it is possible to trigger a refcounting problem on pypy by writing pure python code that invokes zope.interface's C optimizations somehow? If this is an internal function that works on instances of internal classes, then everything should be fine, no?
There was a problem hiding this comment.
I actually think it would be quite hard to trigger some sort of refcounting-type issue here. IIUC, it should be pretty much impossible.
By definition, the object we're accessing is a strongly held attribute of another strongly held object (i.e., in CPython it's in the object's __dict__; it PyPy it's possibly not in a literal dict, but part of the hidden class backing the object implementation). According to this PyPy blog post, as long as a particular object exists in the Python/PyPy world, the PyObject* that points to it will never go invalid, even if the refcount is 0:
We now need a way to convert between W_Root and PyObject* and vice-versa; also, we need to to ensure that the lifetime of the two entities are in sync. In particular:
- as long as the W_Root is kept alive by the GC, we want the PyObject* to live ((even if its refcount drops to 0**;
- as long as the PyObject* has a refcount greater than 0, we want to make sure that the GC does not collect the W_Root.
| from zope.interface._zope_interface_coptimizations import adapter_hooks | ||
| except ImportError: | ||
| pass | ||
| adapter_hooks = _use_c_impl([], 'adapter_hooks') |
There was a problem hiding this comment.
I suppose the extra adapter_hooksFallback and adapter_hooksPy lists can't hurt anyone.
There was a problem hiding this comment.
Maybe it would be good for these modules to define __all__?
Another good followup PR would be to standardize the convention used for defining the Python fallback so we only have to write one extra attribute (and which is consistent).
Also, a good followup PR might be to make Sphinx's conf.py define PURE_PYTHON so that the objects that are documented could be written with their preferred name, instead of like .. autofunction:: implementedByFallback. (Ok, I only actually see that once.)
| return False | ||
| return not _c_optimizations_ignored() | ||
|
|
||
| def _use_c_impl(py_impl, name=None, globs=None): |
There was a problem hiding this comment.
PEP-8: two blank lines between functions.
There was a problem hiding this comment.
And I was being so careful about function spacing, too. I even corrected it several other places! 😉
Hm, I got the impression that if you measure on several versions including 3.8 and combine that into one data file, then coverage report should see 100% coverage. But also I don't trust coveralls.io any more. I'm pretty sure they have their own combining logic, and I don't think it's working correctly. But coveralls.io is a big black box that doesn't respond to queries and bug reports (and, hey, it's a completely free service, we're not entitled to responses), so it's hard to be sure. |
Also add a line of missing whitespace.
|
I have the first followup PR ready, turns out using |
5.1.0 (2020-04-08) ================== - Make ``@implementer(*iface)`` and ``classImplements(cls, *iface)`` ignore redundant interfaces. If the class already implements an interface through inheritance, it is no longer redeclared specifically for *cls*. This solves many instances of inconsistent resolution orders, while still allowing the interface to be declared for readability and maintenance purposes. See `issue 199 <https://github.com/zopefoundation/zope.interface/issues/199>`_. - Remove all bare ``except:`` statements. Previously, when accessing special attributes such as ``__provides__``, ``__providedBy__``, ``__class__`` and ``__conform__``, this package wrapped such access in a bare ``except:`` statement, meaning that many errors could pass silently; typically this would result in a fallback path being taken and sometimes (like with ``providedBy()``) the result would be non-sensical. This is especially true when those attributes are implemented with descriptors. Now, only ``AttributeError`` is caught. This makes errors more obvious. Obviously, this means that some exceptions will be propagated differently than before. In particular, ``RuntimeError`` raised by Acquisition in the case of circular containment will now be propagated. Previously, when adapting such a broken object, a ``TypeError`` would be the common result, but now it will be a more informative ``RuntimeError``. In addition, ZODB errors like ``POSKeyError`` could now be propagated where previously they would ignored by this package. See `issue 200 <https://github.com/zopefoundation/zope.interface/issues/200>`_. - Require that the second argument (*bases*) to ``InterfaceClass`` is a tuple. This only matters when directly using ``InterfaceClass`` to create new interfaces dynamically. Previously, an individual interface was allowed, but did not work correctly. Now it is consistent with ``type`` and requires a tuple. - Let interfaces define custom ``__adapt__`` methods. This implements the other side of the :pep:`246` adaptation protocol: objects being adapted could already implement ``__conform__`` if they know about the interface, and now interfaces can implement ``__adapt__`` if they know about particular objects. There is no performance penalty for interfaces that do not supply custom ``__adapt__`` methods. This includes the ability to add new methods, or override existing interface methods using the new ``@interfacemethod`` decorator. See `issue 3 <https://github.com/zopefoundation/zope.interface/issues/3>`_. - Make the internal singleton object returned by APIs like ``implementedBy`` and ``directlyProvidedBy`` for objects that implement or provide no interfaces more immutable. Previously an internal cache could be mutated. See `issue 204 <https://github.com/zopefoundation/zope.interface/issues/204>`_. 5.0.2 (2020-03-30) ================== - Ensure that objects that implement no interfaces (such as direct subclasses of ``object``) still include ``Interface`` itself in their ``__iro___`` and ``__sro___``. This fixes adapter registry lookups for such objects when the adapter is registered for ``Interface``. See `issue 197 <https://github.com/zopefoundation/zope.interface/issues/197>`_. 5.0.1 (2020-03-21) ================== - Ensure the resolution order for ``InterfaceClass`` is consistent. See `issue 192 <https://github.com/zopefoundation/zope.interface/issues/192>`_. - Ensure the resolution order for ``collections.OrderedDict`` is consistent on CPython 2. (It was already consistent on Python 3 and PyPy). - Fix the handling of the ``ZOPE_INTERFACE_STRICT_IRO`` environment variable. Previously, ``ZOPE_INTERFACE_STRICT_RO`` was read, in contrast with the documentation. See `issue 194 <https://github.com/zopefoundation/zope.interface/issues/194>`_. 5.0.0 (2020-03-19) ================== - Make an internal singleton object returned by APIs like ``implementedBy`` and ``directlyProvidedBy`` immutable. Previously, it was fully mutable and allowed changing its ``__bases___``. That could potentially lead to wrong results in pathological corner cases. See `issue 158 <https://github.com/zopefoundation/zope.interface/issues/158>`_. - Support the ``PURE_PYTHON`` environment variable at runtime instead of just at wheel build time. A value of 0 forces the C extensions to be used (even on PyPy) failing if they aren't present. Any other value forces the Python implementation to be used, ignoring the C extensions. See `PR 151 <https://github.com/zopefoundation/zope.interface/pull/151>`_. - Cache the result of ``__hash__`` method in ``InterfaceClass`` as a speed optimization. The method is called very often (i.e several hundred thousand times during Plone 5.2 startup). Because the hash value never changes it can be cached. This improves test performance from 0.614s down to 0.575s (1.07x faster). In a real world Plone case a reindex index came down from 402s to 320s (1.26x faster). See `PR 156 <https://github.com/zopefoundation/zope.interface/pull/156>`_. - Change the C classes ``SpecificationBase`` and its subclass ``ClassProvidesBase`` to store implementation attributes in their structures instead of their instance dictionaries. This eliminates the use of an undocumented private C API function, and helps make some instances require less memory. See `PR 154 <https://github.com/zopefoundation/zope.interface/pull/154>`_. - Reduce memory usage in other ways based on observations of usage patterns in Zope (3) and Plone code bases. - Specifications with no dependents are common (more than 50%) so avoid allocating a ``WeakKeyDictionary`` unless we need it. - Likewise, tagged values are relatively rare, so don't allocate a dictionary to hold them until they are used. - Use ``__slots___`` or the C equivalent ``tp_members`` in more common places. Note that this removes the ability to set arbitrary instance variables on certain objects. See `PR 155 <https://github.com/zopefoundation/zope.interface/pull/155>`_. The changes in this release resulted in a 7% memory reduction after loading about 6,000 modules that define about 2,200 interfaces. .. caution:: Details of many private attributes have changed, and external use of those private attributes may break. In particular, the lifetime and default value of ``_v_attrs`` has changed. - Remove support for hashing uninitialized interfaces. This could only be done by subclassing ``InterfaceClass``. This has generated a warning since it was first added in 2011 (3.6.5). Please call the ``InterfaceClass`` constructor or otherwise set the appropriate fields in your subclass before attempting to hash or sort it. See `issue 157 <https://github.com/zopefoundation/zope.interface/issues/157>`_. - Remove unneeded override of the ``__hash__`` method from ``zope.interface.declarations.Implements``. Watching a reindex index process in ZCatalog with on a Py-Spy after 10k samples the time for ``.adapter._lookup`` was reduced from 27.5s to 18.8s (~1.5x faster). Overall reindex index time shrunk from 369s to 293s (1.26x faster). See `PR 161 <https://github.com/zopefoundation/zope.interface/pull/161>`_. - Make the Python implementation closer to the C implementation by ignoring all exceptions, not just ``AttributeError``, during (parts of) interface adaptation. See `issue 163 <https://github.com/zopefoundation/zope.interface/issues/163>`_. - Micro-optimization in ``.adapter._lookup`` , ``.adapter._lookupAll`` and ``.adapter._subscriptions``: By loading ``components.get`` into a local variable before entering the loop a bytcode "LOAD_FAST 0 (components)" in the loop can be eliminated. In Plone, while running all tests, average speedup of the "owntime" of ``_lookup`` is ~5x. See `PR 167 <https://github.com/zopefoundation/zope.interface/pull/167>`_. - Add ``__all__`` declarations to all modules. This helps tools that do auto-completion and documentation and results in less cluttered results. Wildcard ("*") are not recommended and may be affected. See `issue 153 <https://github.com/zopefoundation/zope.interface/issues/153>`_. - Fix ``verifyClass`` and ``verifyObject`` for builtin types like ``dict`` that have methods taking an optional, unnamed argument with no default value like ``dict.pop``. On PyPy3, the verification is strict, but on PyPy2 (as on all versions of CPython) those methods cannot be verified and are ignored. See `issue 118 <https://github.com/zopefoundation/zope.interface/issues/118>`_. - Update the common interfaces ``IEnumerableMapping``, ``IExtendedReadMapping``, ``IExtendedWriteMapping``, ``IReadSequence`` and ``IUniqueMemberWriteSequence`` to no longer require methods that were removed from Python 3 on Python 3, such as ``__setslice___``. Now, ``dict``, ``list`` and ``tuple`` properly verify as ``IFullMapping``, ``ISequence`` and ``IReadSequence,`` respectively on all versions of Python. - Add human-readable ``__str___`` and ``__repr___`` to ``Attribute`` and ``Method``. These contain the name of the defining interface and the attribute. For methods, it also includes the signature. - Change the error strings raised by ``verifyObject`` and ``verifyClass``. They now include more human-readable information and exclude extraneous lines and spaces. See `issue 170 <https://github.com/zopefoundation/zope.interface/issues/170>`_. .. caution:: This will break consumers (such as doctests) that depended on the exact error messages. - Make ``verifyObject`` and ``verifyClass`` report all errors, if the candidate object has multiple detectable violations. Previously they reported only the first error. See `issue <https://github.com/zopefoundation/zope.interface/issues/171>`_. Like the above, this will break consumers depending on the exact output of error messages if more than one error is present. - Add ``zope.interface.common.collections``, ``zope.interface.common.numbers``, and ``zope.interface.common.io``. These modules define interfaces based on the ABCs defined in the standard library ``collections.abc``, ``numbers`` and ``io`` modules, respectively. Importing these modules will make the standard library concrete classes that are registered with those ABCs declare the appropriate interface. See `issue 138 <https://github.com/zopefoundation/zope.interface/issues/138>`_. - Add ``zope.interface.common.builtins``. This module defines interfaces of common builtin types, such as ``ITextString`` and ``IByteString``, ``IDict``, etc. These interfaces extend the appropriate interfaces from ``collections`` and ``numbers``, and the standard library classes implement them after importing this module. This is intended as a replacement for third-party packages like `dolmen.builtins <https://pypi.org/project/dolmen.builtins/>`_. See `issue 138 <https://github.com/zopefoundation/zope.interface/issues/138>`_. - Make ``providedBy()`` and ``implementedBy()`` respect ``super`` objects. For instance, if class ``Derived`` implements ``IDerived`` and extends ``Base`` which in turn implements ``IBase``, then ``providedBy(super(Derived, derived))`` will return ``[IBase]``. Previously it would have returned ``[IDerived]`` (in general, it would previously have returned whatever would have been returned without ``super``). Along with this change, adapter registries will unpack ``super`` objects into their ``__self___`` before passing it to the factory. Together, this means that ``component.getAdapter(super(Derived, self), ITarget)`` is now meaningful. See `issue 11 <https://github.com/zopefoundation/zope.interface/issues/11>`_. - Fix a potential interpreter crash in the low-level adapter registry lookup functions. See issue 11. - Adopt Python's standard `C3 resolution order <https://www.python.org/download/releases/2.3/mro/>`_ 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. This has many beneficial properties, including the fact that base interface and base classes tend to appear near the end of the resolution order instead of the beginning. The resolution order in general should be more predictable and consistent. .. caution:: In some cases, especially with complex interface inheritance trees or when manually providing or implementing interfaces, the resulting IRO may be quite different. This may affect adapter lookup. The C3 order enforces some constraints in order to be able to guarantee a sensible ordering. Older versions of zope.interface did not impose similar constraints, so it was possible to create interfaces and declarations that are inconsistent with the C3 constraints. In that event, zope.interface will still produce a resolution order equal to the old order, but it won't be guaranteed to be fully C3 compliant. In the future, strict enforcement of C3 order may be the default. A set of environment variables and module constants allows controlling several aspects of this new behaviour. It is possible to request warnings about inconsistent resolution orders encountered, and even to forbid them. Differences between the C3 resolution order and the previous order can be logged, and, in extreme cases, the previous order can still be used (this ability will be removed in the future). For details, see the documentation for ``zope.interface.ro``. - Make inherited tagged values in interfaces respect the resolution order (``__iro__``), as method and attribute lookup does. Previously tagged values could give inconsistent results. See `issue 190 <https://github.com/zopefoundation/zope.interface/issues/190>`_. - Add ``getDirectTaggedValue`` (and related methods) to interfaces to allow accessing tagged values irrespective of inheritance. See `issue 190 <https://github.com/zopefoundation/zope.interface/issues/190>`_. - Ensure that ``Interface`` is always the last item in the ``__iro__`` and ``__sro__``. This is usually the case, but if classes that do not implement any interfaces are part of a class inheritance hierarchy, ``Interface`` could be assigned too high a priority. See `issue 8 <https://github.com/zopefoundation/zope.interface/issues/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 <https://github.com/zopefoundation/zope.interface/pull/183>`_.
Avoid poisoning caches.
Also stop using deprecated
setup.py testcommand. (zope.testrunner includes zope.interface so we don't use that here, butpython -m unittest discoverhandles things fine AFAICS.)