WIP: Resurrect old Python ring implementation and make the CFFI implementation optional

CHANGES.rst

@@ -4,6 +4,12 @@ Change log
 6.6 (unreleased)
 ----------------
 
+- Remove CFFI dependency because it is too troublesome.
+  Originally added in `#107
+  <https://github.com/zopefoundation/persistent/pull/107>`_ it caused problems
+  whenever a new Python version came out because the CFFI project is slow
+  releasing compatible versions.
+
 
 6.5 (2025-11-18)
 ----------------

pyproject.toml

@@ -4,8 +4,6 @@
 requires = [
     "setuptools",
     "wheel",
-    "cffi; platform_python_implementation == 'CPython'",
-    "pycparser",
 ]
 build-backend = "setuptools.build_meta"
 
@@ -43,7 +41,6 @@ classifiers = [
 dependencies = [
   "zope.deferredimport",
   "zope.interface",
-  "cffi ; platform_python_implementation == 'CPython'",
 ]
 
 [project.optional-dependencies]
@@ -55,6 +52,7 @@ docs = [
 test = [
   "manuel",
   "zope.testrunner >= 6.4",
+  "cffi ; platform_python_implementation == 'CPython'",
 ]
 
 [project.urls]

setup.py

@@ -60,8 +60,7 @@
     ),
 ]
 
-is_pypy = platform.python_implementation() == 'PyPy'
-if is_pypy:
+if platform.python_implementation() == 'PyPy':
     # Header installation doesn't work on PyPy:
     # https://github.com/zopefoundation/persistent/issues/135
     headers = []
@@ -71,13 +70,5 @@
         'src/persistent/ring.h',
     ]
 
-# setup_requires must be specified in the setup call, when building CFFI
-# modules it's not sufficient to have the requirements in a pyproject.toml
-# [build-system] section.
 setup(ext_modules=ext_modules,
-      cffi_modules=['src/persistent/_ring_build.py:ffi'],
-      headers=headers,
-      setup_requires=[
-          "cffi ; platform_python_implementation == 'CPython'",
-          "pycparser",
-      ])
+      headers=headers)

src/persistent/ring.py

@@ -12,10 +12,14 @@
 #
 ##############################################################################
 
+from collections import deque
+
 from zope.interface import Interface
 from zope.interface import implementer
 
-from persistent import _ring
+
+_OGA = object.__getattribute__
+_OSA = object.__setattr__
 
 
 class IRing(Interface):
@@ -80,109 +84,189 @@ def __iter__():
         """
 
 
-ffi = _ring.ffi
-_FFI_RING = _ring.lib
-
-_OGA = object.__getattribute__
-_OSA = object.__setattr__
-
-_handles = set()
-
-
 @implementer(IRing)
-class _CFFIRing:
-    """A ring backed by a C implementation. All operations are constant time.
+class _DequeRing:
+    """A ring backed by the :class:`collections.deque` class.
+
+    Operations are a mix of constant and linear time.
 
-    It is only available on platforms with ``cffi`` installed.
+    It is available on all platforms.
     """
 
-    __slots__ = ('ring_home', 'ring_to_obj', 'cleanup_func')
+    __slots__ = ('ring', 'ring_oids', 'cleanup_func')
 
     def __init__(self, cleanup_func=None):
-        node = self.ring_home = ffi.new("CPersistentRing*")
-        node.r_next = node
-        node.r_prev = node
 
-        self.cleanup_func = cleanup_func
+        self.ring = deque()
+        self.ring_oids = set()
 
-        # The Persistent objects themselves are responsible for keeping
-        # the CFFI nodes alive, but we need to be able to detect whether
-        # or not any given object is in our ring, plus know how many there are.
-        # In addition, once an object enters the ring, it must be kept alive
-        # so that it can be deactivated.
-        # Note that because this is a strong reference to the persistent
-        # object, its cleanup function --- triggered by the ``ffi.gc`` object
-        # it owns --- will never be fired while it is in this dict.
-        self.ring_to_obj = {}
-
-    def ring_node_for(self, persistent_object, create=True):
-        ring_data = _OGA(persistent_object, '_Persistent__ring')
-        if ring_data is None:
-            if not create:
-                return None
-
-            if self.cleanup_func:
-                node = ffi.new('CPersistentRingCFFI*')
-                node.pobj_id = ffi.cast('uintptr_t', id(persistent_object))
-                gc_ptr = ffi.gc(node, self.cleanup_func)
-            else:
-                node = ffi.new("CPersistentRing*")
-                gc_ptr = None
-            ring_data = (
-                node,
-                gc_ptr,
-            )
-            _OSA(persistent_object, '_Persistent__ring', ring_data)
-
-        return ring_data[0]
+        self.cleanup_func = cleanup_func
 
     def __len__(self):
-        return len(self.ring_to_obj)
+        return len(self.ring)
 
     def __contains__(self, pobj):
-        node = self.ring_node_for(pobj, False)
-        return node and node in self.ring_to_obj
+        return pobj._p_oid in self.ring_oids
 
     def add(self, pobj):
-        node = self.ring_node_for(pobj)
-        _FFI_RING.cffi_ring_add(self.ring_home, node)
-        self.ring_to_obj[node] = pobj
+        self.ring.append(pobj)
+        self.ring_oids.add(pobj._p_oid)
 
     def delete(self, pobj):
-        its_node = self.ring_node_for(pobj, False)
-        our_obj = self.ring_to_obj.pop(its_node, self)
-        if its_node is not None and our_obj is not self and its_node.r_next:
-            _FFI_RING.cffi_ring_del(its_node)
-            return 1
-        return None
-
-    def delete_node(self, node):
-        # Minimal sanity checking, assumes we're called from iter.
-        self.ring_to_obj.pop(node)
-        _FFI_RING.cffi_ring_del(node)
+        # 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
 
     def move_to_head(self, pobj):
-        node = self.ring_node_for(pobj, False)
-        _FFI_RING.cffi_ring_move_to_head(self.ring_home, node)
-
-    def iteritems(self):
-        head = self.ring_home
-        here = head.r_next
-        ring_to_obj = self.ring_to_obj
-        while here != head:
-            # We allow mutation during iteration, which means
-            # we must get the next ``here`` value before
-            # yielding, just in case the current value is
-            # removed.
-            current = here
-            here = here.r_next
-            pobj = ring_to_obj[current]
-            yield current, pobj
+        self.delete(pobj)
+        self.add(pobj)
+
+    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):
-        for _, v in self.iteritems():
-            yield v
+        return iter(self.ring)
+
+#    def iteritems(self):
+#        return [(obj._p_oid, obj) for obj in self.ring]
+#
+#    def delete_node(self, node):
+#        pass
+#
+#    def ring_node_for(self, persistent_object, create=True):
+#        ring_data = _OGA(persistent_object, '_Persistent__ring')
+#        if ring_data is None:
+#            if not create:
+#                return None
+#
+#            node =
+#            gc_ptr = None
+#            _data = (
+#            node,
+#            gc_ptr,
+#            )
+#            _OSA(persistent_object, '_Persistent__ring', ring_data)
+#
+#        return ring_data[0]
+
+
+try:
+    from persistent import _ring
+except ImportError:  # pragma: no cover
+    _CFFIRing = None
+else:
+    ffi = _ring.ffi
+    _FFI_RING = _ring.lib
+
+    _handles = set()
+
+    @implementer(IRing)
+    class _CFFIRing:
+        """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', 'cleanup_func')
+
+        def __init__(self, cleanup_func=None):
+            node = self.ring_home = ffi.new("CPersistentRing*")
+            node.r_next = node
+            node.r_prev = node
+
+            self.cleanup_func = cleanup_func
+
+            # The Persistent objects themselves are responsible for keeping
+            # the CFFI nodes alive, but we need to be able to detect whether
+            # or not any given object is in our ring, plus know how many
+            # there are.
+            # In addition, once an object enters the ring, it must be kept
+            # alive so that it can be deactivated.
+            # Note that because this is a strong reference to the persistent
+            # object, its cleanup function --- triggered by the ``ffi.gc``
+            # object it owns --- will never be fired while it is in this dict.
+            self.ring_to_obj = {}
+
+        def ring_node_for(self, persistent_object, create=True):
+            ring_data = _OGA(persistent_object, '_Persistent__ring')
+            if ring_data is None:
+                if not create:
+                    return None
+
+                if self.cleanup_func:
+                    node = ffi.new('CPersistentRingCFFI*')
+                    node.pobj_id = ffi.cast('uintptr_t', id(persistent_object))
+                    gc_ptr = ffi.gc(node, self.cleanup_func)
+                else:
+                    node = ffi.new("CPersistentRing*")
+                    gc_ptr = None
+                ring_data = (
+                    node,
+                    gc_ptr,
+                )
+                _OSA(persistent_object, '_Persistent__ring', ring_data)
+
+            return ring_data[0]
+
+        def __len__(self):
+            return len(self.ring_to_obj)
+
+        def __contains__(self, pobj):
+            node = self.ring_node_for(pobj, False)
+            return node and node in self.ring_to_obj
+
+        def add(self, pobj):
+            node = self.ring_node_for(pobj)
+            _FFI_RING.cffi_ring_add(self.ring_home, node)
+            self.ring_to_obj[node] = pobj
+
+        def delete(self, pobj):
+            its_node = self.ring_node_for(pobj, False)
+            our_obj = self.ring_to_obj.pop(its_node, self)
+            if its_node is not None and \
+               our_obj is not self and \
+               its_node.r_next:
+                _FFI_RING.cffi_ring_del(its_node)
+                return 1
+            return None
+
+        def delete_node(self, node):
+            # Minimal sanity checking, assumes we're called from iter.
+            self.ring_to_obj.pop(node)
+            _FFI_RING.cffi_ring_del(node)
+
+        def move_to_head(self, pobj):
+            node = self.ring_node_for(pobj, False)
+            _FFI_RING.cffi_ring_move_to_head(self.ring_home, node)
+
+        def iteritems(self):
+            head = self.ring_home
+            here = head.r_next
+            ring_to_obj = self.ring_to_obj
+            while here != head:
+                # We allow mutation during iteration, which means
+                # we must get the next ``here`` value before
+                # yielding, just in case the current value is
+                # removed.
+                current = here
+                here = here.r_next
+                pobj = ring_to_obj[current]
+                yield current, pobj
+
+        def __iter__(self):
+            for _, v in self.iteritems():
+                yield v
 
 
 # Export the best available implementation
-Ring = _CFFIRing
+Ring = _CFFIRing if _CFFIRing else _DequeRing

src/persistent/tests/test_ring.py

@@ -33,10 +33,11 @@ def __repr__(self):  # pragma: no cover
         return f"<Dummy {self._p_oid!r} at 0x{id(self):x}>"
 
 
-class CFFIRingTests(unittest.TestCase):
+class _RingBase:
 
     def _getTargetClass(self):
-        return ring._CFFIRing
+        """Return the type of the ring to test"""
+        raise NotImplementedError()
 
     def _makeOne(self):
         return self._getTargetClass()()
@@ -124,3 +125,16 @@ def test_move_to_head(self):
 
         r.move_to_head(p3)
         self.assertEqual([p2, p1, p3], list(r))
+
+
+class DequeRingTests(unittest.TestCase, _RingBase):
+
+    def _getTargetClass(self):
+        return ring._DequeRing
+
+
+@unittest.skipUnless(ring._CFFIRing, 'CFFI not available')
+class CFFIRingTests(unittest.TestCase, _RingBase):
+
+    def _getTargetClass(self):
+        return ring._CFFIRing

tox.ini

@@ -57,8 +57,6 @@ skip_install = true
 deps =
     setuptools
     wheel
-    cffi; platform_python_implementation == 'CPython'
-    pycparser
     twine
     build
     check-manifest