Checkpoint on a CFFI version of the cache MRU ring. Currently fails d…

…ue to lifetime issues.

persistent/persistence.py

@@ -27,6 +27,8 @@
 from persistent._compat import copy_reg
 from persistent._compat import intern
 
+from . import ring
+
 _INITIAL_SERIAL = _ZERO
 
 
@@ -54,7 +56,7 @@
 class Persistent(object):
     """ Pure Python implmentation of Persistent base class
     """
-    __slots__ = ('__jar', '__oid', '__serial', '__flags', '__size')
+    __slots__ = ('__jar', '__oid', '__serial', '__flags', '__size', '__ring', '__ring_handle')
 
     def __new__(cls, *args, **kw):
         inst = super(Persistent, cls).__new__(cls)
@@ -67,6 +69,8 @@ def __new__(cls, *args, **kw):
         _OSA(inst, '_Persistent__serial', None)
         _OSA(inst, '_Persistent__flags', None)
         _OSA(inst, '_Persistent__size', 0)
+        _OSA(inst, '_Persistent__ring', None)
+        _OSA(inst, '_Persistent__ring_handle', None)
         return inst
 
     # _p_jar:  see IPersistent.
@@ -483,24 +487,29 @@ def _p_accessed(self):
         jar = oga(self, '_Persistent__jar')
         if jar is None:
             return
+        myring = oga(self, '_Persistent__ring')
+        if ring is None:
+            return
         oid = oga(self, '_Persistent__oid')
         if oid is None:
             return
         flags = oga(self, '_Persistent__flags')
         if flags is None: # ghost
             return
 
+
         # The KeyError arises in ZODB: ZODB.serialize.ObjectWriter
         # can assign a jar and an oid to newly seen persistent objects,
         # but because they are newly created, they aren't in the
         # pickle cache yet. There doesn't seem to be a way to distinguish
         # that at this level, all we can do is catch it.
         # The AttributeError arises in ZODB test cases
         try:
-            jar._cache.mru(oid)
+            ring.move_to_head(jar._cache.ring_home, myring)
         except (AttributeError,KeyError):
             pass
 
+
     def _p_is_in_cache(self):
         oid = self.__oid
         if not oid:
@@ -511,6 +520,10 @@ def _p_is_in_cache(self):
         if cache is not None:
             return cache.get(oid) is self
 
+    def __del__(self):
+        if self._p_is_in_cache():
+            ring.del_(self._Persistent__ring)
+
 def _estimated_size_in_24_bits(value):
     if value > 1073741696:
         return 16777215

persistent/picklecache.py

@@ -71,6 +71,8 @@ def locked(self, *args, **kwargs):
 
 from collections import deque
 
+from . import ring
+
 @implementer(IPickleCache)
 class PickleCache(object):
 
@@ -100,11 +102,7 @@ def __init__(self, jar, target_size=0, cache_size_bytes=0):
         self.non_ghost_count = 0
         self.persistent_classes = {}
         self.data = weakref.WeakValueDictionary()
-        # oldest is on the left, newest on the right so that default
-        # iteration order is maintained from oldest to newest.
-        # Note that the remove() method is verboten: it uses equality
-        # comparisons, but we must use identity comparisons
-        self.ring = deque()
+        self.ring_home = ring.CPersistentRingHead()
         self.cache_size_bytes = cache_size_bytes
 
     # IPickleCache API
@@ -164,7 +162,11 @@ def __setitem__(self, oid, value):
         else:
             self.data[oid] = value
             _gc_monitor(value)
-            self.mru(oid)
+            node = ring.CPersistentRing(value)
+            value._Persistent__ring = node
+            if _OGA(value, '_p_state') != GHOST:
+                ring.add(self.ring_home, node)
+                self.non_ghost_count += 1
 
     def __delitem__(self, oid):
         """ See IPickleCache.
@@ -196,21 +198,18 @@ def mru(self, oid):
             return False # marker return for tests
 
         value = self.data[oid]
-        was_in_ring = self._remove_from_ring(value)
-        if was_in_ring:
-            # Compensate for decrementing the count; by
-            # definition it should already have been not-a-ghost
-            # so we can avoid a trip through Persistent.__getattribute__
-            self.ring.append(value)
-            self.non_ghost_count += 1
-        elif _OGA(value, '_p_state') != GHOST:
-            self.ring.append(value)
+        id_value = id(value)
+
+        was_in_ring = bool(value._Persistent__ring.r_next)
+        ring.move_to_head(self.ring_home, value._Persistent__ring)
+        if not was_in_ring and  _OGA(value, '_p_state') != GHOST:
             self.non_ghost_count += 1
 
     def ringlen(self):
         """ See IPickleCache.
         """
-        return len(self.ring)
+        return ring.ringlen(self.ring_home)
+
 
     def items(self):
         """ See IPickleCache.
@@ -220,7 +219,11 @@ def items(self):
     def lru_items(self):
         """ See IPickleCache.
         """
-        return [(x._p_oid, x) for x in self.ring]
+        result = []
+        for item in ring.iteritems(self.ring_home):
+            obj = ring.get_object(item)
+            result.append((obj._p_oid, obj))
+        return result
 
     def klass_items(self):
         """ See IPickleCache.
@@ -346,17 +349,16 @@ def update_object_size_estimation(self, oid, new_size):
     @_sweeping_ring
     def _sweep(self, target, target_size_bytes=0):
         # lock
-        # We can't mutate while we're iterating, so store ejections by index here
-        # (deleting by index is potentially more efficient then by value because it
-        # can use the rotate() method and not be O(n)). Also note that we do not use
-        # self._remove_from_ring because we need to decrement the non_ghost_count
-        # as we traverse the ring to be sure to meet our target
-        to_eject = []
-        i = -1 # Using a manual numeric counter instead of enumerate() is much faster on PyPy
-        for value in self.ring:
+        ejected = 0
+        for here in ring.iteritems(self.ring_home):
+            value = ring.get_object(here)
+            if value is None:
+                continue
+            here = here.r_next
+
             if self.non_ghost_count <= target and (self.total_estimated_size <= target_size_bytes or not target_size_bytes):
                 break
-            i += 1
+
             if value._p_state == UPTODATE:
                 # The C implementation will only evict things that are specifically
                 # in the up-to-date state
@@ -379,16 +381,14 @@ def _sweep(self, target, target_size_bytes=0):
                     # they don't cooperate (without this check a bunch of test_picklecache
                     # breaks)
                     or not isinstance(value, _SWEEPABLE_TYPES)):
-                    to_eject.append(i)
+                    self._remove_from_ring(value)
                     self.non_ghost_count -= 1
+                    ejected += 1
 
-        for i in reversed(to_eject):
-            del self.ring[i]
-
-        if to_eject and _SWEEP_NEEDS_GC:
+        if ejected and _SWEEP_NEEDS_GC:
             # See comments on _SWEEP_NEEDS_GC
             gc.collect()
-        return len(to_eject)
+        return ejected
 
     @_sweeping_ring
     def _invalidate(self, oid):
@@ -408,21 +408,5 @@ def _invalidate(self, oid):
                 pass
 
     def _remove_from_ring(self, value):
-        """
-        Removes the previously non-ghost `value` from the ring, decrementing
-        the `non_ghost_count` if it's found. The value may be a ghost when
-        this method is called.
-
-        :return: A true value if the object was found in the ring.
-        """
-        # 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)
-        i = 0 # Using a manual numeric counter instead of enumerate() is much faster on PyPy
-        for o in self.ring:
-            if o is value:
-                del self.ring[i]
-                self.non_ghost_count -= 1
-                return 1
-            i += 1
+        if value._Persistent__ring.r_next:
+            ring.del_(value._Persistent__ring)

persistent/ring.py

@@ -0,0 +1,81 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+"""
+
+
+
+"""
+from cffi import FFI
+import pkg_resources
+import os
+
+ffi = FFI()
+
+ffi.cdef("""
+typedef struct CPersistentRingEx_struct
+{
+    struct CPersistentRingEx_struct *r_prev;
+    struct CPersistentRingEx_struct *r_next;
+    void* object;
+} CPersistentRingEx;
+""")
+
+ffi.cdef(pkg_resources.resource_string('persistent', 'ring.h'))
+
+ring = ffi.verify("""
+typedef struct CPersistentRingEx_struct
+{
+    struct CPersistentRingEx_struct *r_prev;
+    struct CPersistentRingEx_struct *r_next;
+    void* object;
+} CPersistentRingEx;
+#include "ring.c"
+""", include_dirs=[os.path.dirname(os.path.abspath(__file__))])
+
+
+class CPersistentRing(object):
+
+    def __init__(self, obj=None):
+        self.handle = None
+        self.node = ffi.new("CPersistentRingEx*")
+        if obj is not None:
+            self.handle = self.node.object = ffi.new_handle(obj)
+            self._object = obj # Circular reference
+
+    def __getattr__(self, name):
+        return getattr(self.node, name)
+
+def CPersistentRingHead():
+    head = CPersistentRing()
+    head.node.r_next = head.node
+    head.node.r_prev = head.node
+    return head
+
+def _c(node):
+    return ffi.cast("CPersistentRing*", node.node)
+
+def add(head, elt):
+    ring.ring_add(_c(head), _c(elt))
+
+def del_(elt):
+    ring.ring_del(_c(elt))
+
+def move_to_head(head, elt):
+    ring.ring_move_to_head(_c(head), _c(elt))
+
+def iteritems(head):
+    here = head.r_next
+    while here != head.node:
+        yield here
+        here = here.r_next
+
+def ringlen(head):
+    count = 0
+    for _ in iteritems(head):
+        count += 1
+    return count
+
+def get_object(node):
+    return ffi.from_handle(node.object)
+
+print CPersistentRing()