From 487664ecd47bae0bd1c23580a1dfac9704ca43a7 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Mon, 19 Nov 2018 07:53:09 -0600 Subject: [PATCH] Require CFFI on CPython for pure-Python operation. Fixes #77 --- CHANGES.rst | 8 ++ persistent/ring.py | 155 +++++++++------------------ persistent/tests/test_persistence.py | 3 +- persistent/tests/test_picklecache.py | 41 ++----- persistent/tests/test_ring.py | 26 +---- persistent/tests/test_timestamp.py | 25 +---- setup.py | 15 +-- tox.ini | 7 -- 8 files changed, 86 insertions(+), 194 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index dcf9a67..4269663 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -16,10 +16,18 @@ - The Python implementation raises ``AttributeError`` if a persistent class doesn't have a ``p_jar`` attribute. + See `issue 102 + `_. + - Allow sweeping cache without ``cache_size``. ``cache_size_bytes`` works with ``cache_size=0``, no need to set ``cache_size`` to a large value. +- Require ``CFFI`` on CPython for pure-Python operation. This drops + support for Jython (which was untested). See `issue 77 + `_. + + 4.4.3 (2018-10-22) ------------------ diff --git a/persistent/ring.py b/persistent/ring.py index 1bad105..e239149 100644 --- a/persistent/ring.py +++ b/persistent/ring.py @@ -13,11 +13,13 @@ # ############################################################################## -#pylint: disable=W0212,E0211,W0622,E0213,W0221,E0239 +# pylint:disable=inherit-non-class,no-self-argument,redefined-builtin,c-extension-no-member from zope.interface import Interface from zope.interface import implementer +from persistent import _ring + class IRing(Interface): """Conceptually, a doubly-linked list for efficiently keeping track of least- and most-recently used :class:`persistent.interfaces.IPersistent` objects. @@ -28,13 +30,13 @@ class IRing(Interface): explaining assumptions and performance requirements. """ - def __len__(): + def __len__(): # pylint:disable=no-method-argument """Return the number of persistent objects stored in the ring. Should be constant time. """ - def __contains__(object): + def __contains__(object): # pylint:disable=unexpected-special-method-signature """Answer whether the given persistent object is found in the ring. Must not rely on object equality or object hashing, but only @@ -83,7 +85,7 @@ def delete_all(indexes_and_values): Should be at least linear time (not quadratic). """ - def __iter__(): + def __iter__(): # pylint:disable=no-method-argument """Iterate over each persistent object in the ring, in the order of least recently used to most recently used. @@ -91,127 +93,72 @@ def __iter__(): undefined consequences. """ -from collections import deque -@implementer(IRing) -class _DequeRing(object): - """A ring backed by the :class:`collections.deque` class. +ffi = _ring.ffi +_FFI_RING = _ring.lib - Operations are a mix of constant and linear time. +_OGA = object.__getattribute__ +_OSA = object.__setattr__ - It is available on all platforms. + +@implementer(IRing) +class _CFFIRing(object): + """A ring backed by a C implementation. All operations are constant time. + + It is only available on platforms with ``cffi`` installed. """ - __slots__ = ('ring', 'ring_oids') + __slots__ = ('ring_home', 'ring_to_obj') def __init__(self): + node = self.ring_home = ffi.new("CPersistentRing*") + node.r_next = node + node.r_prev = node - self.ring = deque() - self.ring_oids = set() + # In order for the CFFI objects to stay alive, we must keep + # a strong reference to them, otherwise they get freed. We must + # also keep strong references to the objects so they can be deactivated + self.ring_to_obj = dict() def __len__(self): - return len(self.ring) + return len(self.ring_to_obj) def __contains__(self, pobj): - return pobj._p_oid in self.ring_oids + return getattr(pobj, '_Persistent__ring', self) in self.ring_to_obj def add(self, pobj): - self.ring.append(pobj) - self.ring_oids.add(pobj._p_oid) + node = ffi.new("CPersistentRing*") + _FFI_RING.ring_add(self.ring_home, node) + self.ring_to_obj[node] = pobj + _OSA(pobj, '_Persistent__ring', node) def delete(self, pobj): - # Note that we do not use self.ring.remove() because that - # uses equality semantics and we don't want to call the persistent - # object's __eq__ method (which might wake it up just after we - # tried to ghost it) - for i, o in enumerate(self.ring): - if o is pobj: - del self.ring[i] - self.ring_oids.discard(pobj._p_oid) - return 1 + its_node = getattr(pobj, '_Persistent__ring', None) + our_obj = self.ring_to_obj.pop(its_node, None) + if its_node is not None and our_obj is not None and its_node.r_next: + _FFI_RING.ring_del(its_node) + return 1 + return None def move_to_head(self, pobj): - self.delete(pobj) - self.add(pobj) + node = _OGA(pobj, '_Persistent__ring') + _FFI_RING.ring_move_to_head(self.ring_home, node) def delete_all(self, indexes_and_values): - for ix, value in reversed(indexes_and_values): - del self.ring[ix] - self.ring_oids.discard(value._p_oid) - - def __iter__(self): - return iter(self.ring) + for _, value in indexes_and_values: + self.delete(value) + def iteritems(self): + head = self.ring_home + here = head.r_next + while here != head: + yield here + here = here.r_next -try: - from persistent import _ring -except ImportError: # pragma: no cover - _CFFIRing = None -else: - ffi = _ring.ffi - _FFI_RING = _ring.lib - - _OGA = object.__getattribute__ - _OSA = object.__setattr__ - - #pylint: disable=E1101 - @implementer(IRing) - class _CFFIRing(object): - """A ring backed by a C implementation. All operations are constant time. - - It is only available on platforms with ``cffi`` installed. - """ - - __slots__ = ('ring_home', 'ring_to_obj') - - def __init__(self): - node = self.ring_home = ffi.new("CPersistentRing*") - node.r_next = node - node.r_prev = node - - # In order for the CFFI objects to stay alive, we must keep - # a strong reference to them, otherwise they get freed. We must - # also keep strong references to the objects so they can be deactivated - self.ring_to_obj = dict() - - def __len__(self): - return len(self.ring_to_obj) - - def __contains__(self, pobj): - return getattr(pobj, '_Persistent__ring', self) in self.ring_to_obj - - def add(self, pobj): - node = ffi.new("CPersistentRing*") - _FFI_RING.ring_add(self.ring_home, node) - self.ring_to_obj[node] = pobj - _OSA(pobj, '_Persistent__ring', node) - - def delete(self, pobj): - its_node = getattr(pobj, '_Persistent__ring', None) - our_obj = self.ring_to_obj.pop(its_node, None) - if its_node is not None and our_obj is not None and its_node.r_next: - _FFI_RING.ring_del(its_node) - return 1 - - def move_to_head(self, pobj): - node = _OGA(pobj, '_Persistent__ring') - _FFI_RING.ring_move_to_head(self.ring_home, node) - - def delete_all(self, indexes_and_values): - for _, value in indexes_and_values: - self.delete(value) - - def iteritems(self): - head = self.ring_home - here = head.r_next - while here != head: - yield here - here = here.r_next - - def __iter__(self): - ring_to_obj = self.ring_to_obj - for node in self.iteritems(): - yield ring_to_obj[node] + def __iter__(self): + ring_to_obj = self.ring_to_obj + for node in self.iteritems(): + yield ring_to_obj[node] # Export the best available implementation -Ring = _CFFIRing if _CFFIRing else _DequeRing +Ring = _CFFIRing diff --git a/persistent/tests/test_persistence.py b/persistent/tests/test_persistence.py index 392f246..b1e9526 100644 --- a/persistent/tests/test_persistence.py +++ b/persistent/tests/test_persistence.py @@ -22,7 +22,6 @@ _is_pypy3 = platform.python_implementation() == 'PyPy' and sys.version_info[0] > 2 -_is_jython = platform.python_implementation() == 'Jython' # pylint:disable=R0904,W0212,E1101 # pylint:disable=attribute-defined-outside-init,too-many-lines @@ -985,7 +984,7 @@ class Derived(Base): self.assertEqual(inst.baz, 'bam') self.assertEqual(inst.qux, 'spam') - if not _is_pypy3 and not _is_jython: + if not _is_pypy3: def test___setstate___interns_dict_keys(self): class Derived(self._getTargetClass()): pass diff --git a/persistent/tests/test_picklecache.py b/persistent/tests/test_picklecache.py index 55642b9..bd9ae92 100644 --- a/persistent/tests/test_picklecache.py +++ b/persistent/tests/test_picklecache.py @@ -12,9 +12,7 @@ # ############################################################################## import gc -import os import platform -import sys import unittest from persistent.interfaces import UPTODATE @@ -22,7 +20,6 @@ # pylint:disable=protected-access,too-many-lines,too-many-public-methods _is_pypy = platform.python_implementation() == 'PyPy' -_is_jython = 'java' in sys.platform _marker = object() @@ -32,22 +29,12 @@ def skipIfNoCExtension(o): persistent._cPickleCache is None, "The C extension is not available")(o) - -if _is_jython: # pragma: no cover - def with_deterministic_gc(f): - def test(self): - # pylint:disable=no-member - old_flags = gc.getMonitorGlobal() - gc.setMonitorGlobal(True) - try: - f(self, force_collect=True) - finally: - gc.setMonitorGlobal(old_flags) - return test -else: - def with_deterministic_gc(f): - return f - +def skipIfPurePython(o): + import persistent._compat + return unittest.skipIf( + persistent._compat.PURE_PYTHON, + "Cannot mix and match implementations" + )(o) class PickleCacheTests(unittest.TestCase): @@ -657,14 +644,6 @@ def _p_invalidate(cls): self.assertTrue(pclass.invalidated) - def test_ring_impl(self): - from .. import ring - - expected = (ring._CFFIRing - if _is_pypy or ring._CFFIRing is not None or os.environ.get('USING_CFFI') - else ring._DequeRing) - self.assertIs(ring.Ring, expected) - class PythonPickleCacheTests(PickleCacheTests): # Tests that depend on the implementation details of the @@ -1007,9 +986,8 @@ def test_reify_hit_single_ghost(self): self.assertTrue(items[0][1] is candidate) self.assertEqual(candidate._p_state, UPTODATE) - @with_deterministic_gc def test_cache_garbage_collection_bytes_also_deactivates_object(self, - force_collect=_is_pypy or _is_jython): + force_collect=_is_pypy): class MyPersistent(self._getDummyPersistentClass()): def _p_deactivate(self): @@ -1070,9 +1048,7 @@ def test_new_ghost_obj_already_in_cache(self): candidate._p_jar = None self.assertRaises(KeyError, cache.new_ghost, key, candidate) - @with_deterministic_gc - def test_cache_garbage_collection_bytes_with_cache_size_0( - self, force_collect=_is_pypy or _is_jython): + def test_cache_garbage_collection_bytes_with_cache_size_0(self): class MyPersistent(self._getDummyPersistentClass()): def _p_deactivate(self): @@ -1116,6 +1092,7 @@ def _p_deactivate(self): @skipIfNoCExtension +@skipIfPurePython class CPickleCacheTests(PickleCacheTests): def _getTargetClass(self): diff --git a/persistent/tests/test_ring.py b/persistent/tests/test_ring.py index 55464b1..0564f04 100644 --- a/persistent/tests/test_ring.py +++ b/persistent/tests/test_ring.py @@ -15,7 +15,7 @@ from .. import ring -#pylint: disable=R0904,W0212,E1101 +# pylint:disable=protected-access class DummyPersistent(object): _p_oid = None @@ -34,11 +34,11 @@ def __init__(self, oid=None): def __repr__(self): # pragma: no cover return "" % self._p_oid -class _Ring_Base(object): + +class CFFIRingTests(unittest.TestCase): def _getTargetClass(self): - """Return the type of the ring to test""" - raise NotImplementedError() + return ring._CFFIRing def _makeOne(self): return self._getTargetClass()() @@ -137,21 +137,3 @@ def test_delete_all(self): r.delete_all([(0, p1), (2, p3)]) self.assertEqual([p2], list(r)) self.assertEqual(1, len(r)) - -class DequeRingTests(unittest.TestCase, _Ring_Base): - - def _getTargetClass(self): - return ring._DequeRing - -_add_to_suite = [DequeRingTests] - -if ring._CFFIRing: - class CFFIRingTests(unittest.TestCase, _Ring_Base): - - def _getTargetClass(self): - return ring._CFFIRing - - _add_to_suite.append(CFFIRingTests) - -def test_suite(): - return unittest.TestSuite([unittest.makeSuite(x) for x in _add_to_suite]) diff --git a/persistent/tests/test_timestamp.py b/persistent/tests/test_timestamp.py index ff8b6a9..4c658a7 100644 --- a/persistent/tests/test_timestamp.py +++ b/persistent/tests/test_timestamp.py @@ -232,12 +232,6 @@ def _makePy(self, *args, **kwargs): from persistent.timestamp import pyTimeStamp return pyTimeStamp(*args, **kwargs) - @property - def _is_jython(self): - import platform - py_impl = getattr(platform, 'python_implementation', lambda: None) - return py_impl() == 'Jython' - def _make_C_and_Py(self, *args, **kwargs): return self._makeC(*args, **kwargs), self._makePy(*args, **kwargs) @@ -300,18 +294,7 @@ def test_py_hash_32_64_bit(self): MUT.c_long = c_int64 # call __hash__ directly to avoid interpreter truncation # in hash() on 32-bit platforms - if not self._is_jython: - self.assertEqual(py.__hash__(), bit_64_hash) - else: # pragma: no cover - # Jython 2.7's ctypes module doesn't properly - # implement the 'value' attribute by truncating. - # (It does for native calls, but not visibly to Python). - # Therefore we get back the full python long. The actual - # hash() calls are correct, though, because the JVM uses - # 32-bit ints for its hashCode methods. - self.assertEqual( - py.__hash__(), - 384009219096809580920179179233996861765753210540033) + self.assertEqual(py.__hash__(), bit_64_hash) finally: MUT._MAXINT = orig_maxint if orig_c_long is not None: @@ -320,7 +303,7 @@ def test_py_hash_32_64_bit(self): del MUT.c_long # These are *usually* aliases, but aren't required - # to be (and aren't under Jython 2.7). + # to be expected_hash = bit_32_hash if is_32_bit_hash else bit_64_hash self.assertEqual(py.__hash__(), expected_hash) @@ -328,9 +311,9 @@ def test_hash_equal_constants(self): # The simple constants make it easier to diagnose # a difference in algorithms import persistent.timestamp as MUT - # We get 32-bit hash values on 32-bit platforms, or on the JVM + # We get 32-bit hash values on 32-bit platforms, # OR on Windows (whether compiled in 64 or 32-bit mode) - is_32_bit = MUT._MAXINT == (2**31 - 1) or self._is_jython or sys.platform == 'win32' + is_32_bit = MUT._MAXINT == (2**31 - 1) or sys.platform == 'win32' c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x00\x00\x00') self.assertEqual(hash(c), 8) diff --git a/setup.py b/setup.py index ff6c8c8..0114080 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,7 @@ from setuptools import find_packages from setuptools import setup -version = '4.4.4.dev0' +version = '4.5.0.dev0' here = os.path.abspath(os.path.dirname(__file__)) @@ -33,12 +33,12 @@ def _read_file(filename): README = (_read_file('README.rst') + '\n\n' + _read_file('CHANGES.rst')) is_pypy = platform.python_implementation() == 'PyPy' -is_jython = 'java' in sys.platform -# Jython cannot build the C optimizations, while on PyPy they are +# On PyPy the C optimizations are # anti-optimizations (the C extension compatibility layer is known-slow, -# and defeats JIT opportunities). -if is_pypy or is_jython: +# and defeats JIT opportunities); PyPy 6.0 can compile them, but the +# tests fail and they actually crash the VM. +if is_pypy: # Note that all the lists we pass to setuptools must be distinct # objects, or bad things happen. See https://github.com/zopefoundation/persistent/issues/88 ext_modules = [] @@ -120,7 +120,6 @@ def _read_file(filename): extras_require={ 'test': [ 'zope.testrunner', - "cffi ; platform_python_implementation == 'CPython'", 'manuel', ], 'testing': (), @@ -131,6 +130,10 @@ def _read_file(filename): }, install_requires=[ 'zope.interface', + "cffi ; platform_python_implementation == 'CPython'", + ], + setup_requires=[ + "cffi ; platform_python_implementation == 'CPython'", ], entry_points={}, ) diff --git a/tox.ini b/tox.ini index de3d0ac..2869732 100644 --- a/tox.ini +++ b/tox.ini @@ -1,13 +1,9 @@ [tox] envlist = -# Jython 2.7rc2 does work, but unfortunately has an issue running -# with Tox 1.9.2 (http://bugs.jython.org/issue2325) -# py27,py27-pure,pypy,py33,py34,pypy3,jython,coverage,docs py27,py27-pure,py27-pure-cffi,pypy,py34,py35,py36,py37,pypy3,coverage,docs [testenv] deps = - cffi .[test] commands = zope-testrunner --test-path=. @@ -23,7 +19,6 @@ basepython = python2.7 setenv = PURE_PYTHON = 1 - USING_CFFI = 1 [testenv:coverage] usedevelop = true @@ -35,8 +30,6 @@ commands = deps = {[testenv]deps} coverage -setenv = - USING_CFFI = 1 [testenv:docs] basepython =