Merge pull request #107 from zopefoundation/issue77

Require CFFI on CPython for pure-Python operation.

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
+  <https://github.com/zopefoundation/persistent/issues/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
+  <https://github.com/zopefoundation/persistent/issues/77>`_.
+
+
 4.4.3 (2018-10-22)
 ------------------
 

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,135 +85,80 @@ 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.
 
         Mutating the ring while an iteration is in progress has
         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

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

persistent/tests/test_picklecache.py

@@ -12,17 +12,14 @@
 #
 ##############################################################################
 import gc
-import os
 import platform
-import sys
 import unittest
 
 from persistent.interfaces import UPTODATE
 
 # 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):

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 "<Dummy %r>" % 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])