Skip to content

Commit

Permalink
Add a non-CFFI implementation of the Ring abstraction based on the de…
Browse files Browse the repository at this point in the history
…que work, and add a tox environment to test it. Only one ZODB test fails.
  • Loading branch information
jamadden committed Apr 28, 2015
1 parent 38d86ef commit 012ea94
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 118 deletions.
63 changes: 26 additions & 37 deletions persistent/picklecache.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ def locked(self, *args, **kwargs):
self._is_sweeping_ring = False
return locked

from collections import deque

from . import ring
from .ring import Ring

@implementer(IPickleCache)
class PickleCache(object):
Expand Down Expand Up @@ -102,8 +100,7 @@ def __init__(self, jar, target_size=0, cache_size_bytes=0):
self.non_ghost_count = 0
self.persistent_classes = {}
self.data = weakref.WeakValueDictionary()
self.ring_home = ring.CPersistentRingHead()
self.ring_nodes = set()
self.ring = Ring()
self.cache_size_bytes = cache_size_bytes

# IPickleCache API
Expand Down Expand Up @@ -163,11 +160,8 @@ def __setitem__(self, oid, value):
else:
self.data[oid] = value
_gc_monitor(value)
node = ring.CPersistentRing(value)
object.__setattr__(value, '_Persistent__ring', node)
if _OGA(value, '_p_state') != GHOST:
ring.add(self.ring_home, node)
self.ring_nodes.add(node)
if _OGA(value, '_p_state') != GHOST and value not in self.ring:
self.ring.add(value)
self.non_ghost_count += 1

def __delitem__(self, oid):
Expand All @@ -179,7 +173,7 @@ def __delitem__(self, oid):
del self.persistent_classes[oid]
else:
value = self.data.pop(oid)
self._remove_from_ring(value)
self.ring.delete(value)

def get(self, oid, default=None):
""" See IPickleCache.
Expand All @@ -200,24 +194,19 @@ def mru(self, oid):
return False # marker return for tests

value = self.data[oid]
node = _OGA(value, '_Persistent__ring')
if node is None: # tests
return

was_in_ring = bool(node.r_next)
was_in_ring = value in self.ring
if not was_in_ring:
if _OGA(value, '_p_state') != GHOST:
ring.add(self.ring_home, node)
self.ring_nodes.add(node)
self.ring.add(value)
self.non_ghost_count += 1
else:
ring.move_to_head(self.ring_home, node)
self.ring.move_to_head(value)

def ringlen(self):
""" See IPickleCache.
"""
return ring.ringlen(self.ring_home)

return len(self.ring)

def items(self):
""" See IPickleCache.
Expand All @@ -228,8 +217,7 @@ def lru_items(self):
""" See IPickleCache.
"""
result = []
for item in ring.iteritems(self.ring_home):
obj = ring.get_object(item)
for obj in self.ring:
result.append((obj._p_oid, obj))
return result

Expand Down Expand Up @@ -356,15 +344,17 @@ def update_object_size_estimation(self, oid, new_size):

@_sweeping_ring
def _sweep(self, target, target_size_bytes=0):
# lock
ejected = 0
for here in list(ring.iteritems(self.ring_home)):

# To avoid mutating datastructures in place or making a copy,
# and to work efficiently with both the CFFI ring and the
# deque-based ring, we collect the objects and their indexes
# up front and then hand them off for ejection.
# We don't use enumerate because that's slow under PyPy
i = -1
to_eject = []
for value in self.ring:
if self.non_ghost_count <= target and (self.total_estimated_size <= target_size_bytes or not target_size_bytes):
break
value = ring.get_object(here)
here = here.r_next

i += 1
if value._p_state == UPTODATE:
# The C implementation will only evict things that are specifically
# in the up-to-date state
Expand All @@ -387,9 +377,13 @@ 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)):
self._remove_from_ring(value)
to_eject.append((i, value))
self.non_ghost_count -= 1
ejected += 1

ejected = len(to_eject)
if ejected:
self.ring.delete_all(to_eject)
del to_eject # Got to clear our local if we want the GC to get the weak refs

if ejected and _SWEEP_NEEDS_GC:
# See comments on _SWEEP_NEEDS_GC
Expand All @@ -401,7 +395,7 @@ def _invalidate(self, oid):
value = self.data.get(oid)
if value is not None and value._p_state != GHOST:
value._p_invalidate()
self._remove_from_ring(value)
self.ring.delete(value)
elif oid in self.persistent_classes:
persistent_class = self.persistent_classes[oid]
del self.persistent_classes[oid]
Expand All @@ -412,8 +406,3 @@ def _invalidate(self, oid):
persistent_class._p_invalidate()
except AttributeError:
pass

def _remove_from_ring(self, value):
if value._Persistent__ring.r_next:
ring.del_(value._Persistent__ring)
self.ring_nodes.discard(value._Persistent__ring)
175 changes: 113 additions & 62 deletions persistent/ring.py
Original file line number Diff line number Diff line change
@@ -1,84 +1,135 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-
"""
##############################################################################
#
# Copyright (c) 2009 Zope Foundation and Contributors.
# All Rights Reserved.
#
# This software is subject to the provisions of the Zope Public License,
# Version 2.1 (ZPL). A copy of the ZPL should accompany this distribution.
# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
# FOR A PARTICULAR PURPOSE.
#
##############################################################################

#pylint: disable=W0212

try:
from cffi import FFI
import os

this_dir = os.path.dirname(os.path.abspath(__file__))

"""
from cffi import FFI
import pkg_resources
import os
ffi = FFI()
with open(os.path.join(this_dir, 'ring.h')) as f:
ffi.cdef(f.read())

ffi = FFI()
ring = ffi.verify("""
#include "ring.c"
""", include_dirs=[os.path.dirname(os.path.abspath(__file__))])

ffi.cdef("""
typedef struct CPersistentRingEx_struct
{
struct CPersistentRingEx_struct *r_prev;
struct CPersistentRingEx_struct *r_next;
void* object;
} CPersistentRingEx;
""")
_OGA = object.__getattribute__
_OSA = object.__setattr__

ffi.cdef(pkg_resources.resource_string('persistent', 'ring.h'))
#pylint: disable=E1101
class _CFFIRing(object):

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__))])
__slots__ = ('ring_home', 'ring_to_obj')

def __init__(self):
node = self.ring_home = ffi.new("CPersistentRing*")
node.r_next = node
node.r_prev = node

class CPersistentRing(object):
self.ring_to_obj = dict()

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 __len__(self):
return len(self.ring_to_obj)

def __getattr__(self, name):
return getattr(self.node, name)
def __contains__(self, pobj):
return getattr(pobj, '_Persistent__ring', self) in self.ring_to_obj

def get_object(self):
return get_object(self.node)
def add(self, pobj):
node = ffi.new("CPersistentRing*")
ring.ring_add(self.ring_home, node)
self.ring_to_obj[node] = pobj
object.__setattr__(pobj, '_Persistent__ring', node)

def CPersistentRingHead():
head = CPersistentRing()
head.node.r_next = head.node
head.node.r_prev = head.node
return head
def delete(self, pobj):
node = getattr(pobj, '_Persistent__ring', None)
if node is not None and node.r_next:
ring.ring_del(node)
self.ring_to_obj.pop(node, None)

def _c(node):
return ffi.cast("CPersistentRing*", node.node)
def move_to_head(self, pobj):
node = pobj._Persistent__ring
ring.ring_move_to_head(self.ring_home, node)

def add(head, elt):
ring.ring_add(_c(head), _c(elt))
def delete_all(self, indexes_and_values):
for _, value in indexes_and_values:
self.delete(value)

def del_(elt):
ring.ring_del(_c(elt))
def iteritems(self):
head = self.ring_home
here = head.r_next
while here != head:
yield here
here = here.r_next

def move_to_head(head, elt):
ring.ring_move_to_head(_c(head), _c(elt))
def __iter__(self):
ring_to_obj = self.ring_to_obj
for node in self.iteritems():
yield ring_to_obj[node]

def iteritems(head):
here = head.r_next
while here != head.node:
yield here
here = here.r_next
Ring = _CFFIRing

def ringlen(head):
count = 0
for _ in iteritems(head):
count += 1
return count
except ImportError:

def get_object(node):
return ffi.from_handle(node.object)
from collections import deque

print CPersistentRing()
class _DequeRing(object):

__slots__ = ('ring', 'ring_oids')

def __init__(self):

self.ring = deque()
self.ring_oids = set()

def __len__(self):
return len(self.ring)

def __contains__(self, pobj):
return pobj._p_oid in self.ring_oids

def add(self, pobj):
self.ring.append(pobj)
self.ring_oids.add(pobj._p_oid)

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)
i = 0 # Using a manual numeric counter instead of enumerate() is much faster on PyPy
for o in self.ring:
if o is pobj:
del self.ring[i]
self.ring_oids.discard(pobj._p_oid)
return 1
i += 1

def move_to_head(self, 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):
return iter(self.ring)

Ring = _DequeRing
35 changes: 17 additions & 18 deletions persistent/tests/test_picklecache.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,16 @@
# FOR A PARTICULAR PURPOSE.
#
##############################################################################
import unittest
import gc
_is_jython = hasattr(gc, 'getJythonGCFlags')
import os
import platform
import sys
import unittest

_py_impl = getattr(platform, 'python_implementation', lambda: None)
_is_pypy = _py_impl() == 'PyPy'
_is_jython = 'java' in sys.platform

_marker = object()

class PickleCacheTests(unittest.TestCase):
Expand Down Expand Up @@ -970,22 +977,6 @@ def bad_deactivate():
finally:
persistent.picklecache._SWEEPABLE_TYPES = sweep_types

def test_invalidate_not_in_cache(self):
# A contrived test of corruption
cache = self._makeOne()
p = self._makePersist(jar=cache.jar)

p._p_state = 0 # non-ghost, get in the ring
cache[p._p_oid] = p
self.assertEqual(cache.cache_non_ghost_count, 1)

from ..ring import get_object,ffi
self.assertEqual(get_object(cache.ring_home.r_next), p)
cache.ring_home.r_next.object = ffi.NULL

# Nothing to test, just that it doesn't break
cache._invalidate(p._p_oid)

if _is_jython:
def with_deterministic_gc(f):
def test(self):
Expand Down Expand Up @@ -1066,6 +1057,14 @@ def _p_invalidate(cls):

self.assertTrue(pclass.invalidated)

def test_ring_impl(self):
from .. import ring

if _is_pypy or os.getenv('USING_CFFI'):
self.assertEqual(ring.Ring, ring._CFFIRing)
else:
self.assertEqual(ring.Ring, ring._DequeRing)

class DummyPersistent(object):

def _p_invalidate(self):
Expand Down
Loading

0 comments on commit 012ea94

Please sign in to comment.