Remove the cache_deactivate_after argument from the cPickleCache

constructor, since it is ignored and there is no current plan to
support two caches with almost-but-not-quite-the-same arguments.

This change has effects in many files.  The Connection and DB don't
pass this argument and don't bother setting it explicitly when it is
reset via DB APIs like setCacheDeactivateAfter().  XXX These APIs
remain, since existing code may depend on them, but they have no
effect.

New policy in cPersistence.c: A Persistent object can't have its
_p_jar set or deleted once it is in a cache.  Persistent already
implemented this policy for _p_oid; it seems safer to do the same for
the jar.

Add ringlen() method to cache objects (implemented as cc_ringlen).
This returns the length of the doubly linked list of non-ghost
objects.  Same as len(cache.lru_items()), but more efficient.  Only
used for testing at the moment.

In ring_corrupt(), don't raise a new exception if one has already been
set.  The old behavior masked useful information about the original
error / traceback.

src/Persistence/cPersistence.c

@@ -14,7 +14,7 @@
 static char cPersistence_doc_string[] = 
 "Defines Persistent mixin class for persistent objects.\n"
 "\n"
-"$Id: cPersistence.c,v 1.58 2002/04/05 01:12:48 jeremy Exp $\n";
+"$Id: cPersistence.c,v 1.59 2002/04/15 18:42:51 jeremy Exp $\n";
 
 #include "cPersistence.h"
 
@@ -572,19 +572,19 @@ _setattro(cPersistentObject *self, PyObject *oname, PyObject *v,
       return -1;
 	
   if (*name == '_' && name[1] == 'p' && name[2] == '_') {
-      if (name[3] == 'o' && name[4] == 'i' && name[5] == 'd' && ! name[6]) {
+      if (strcmp(name + 3, "oid") == 0) {
 	  if (self->cache) {
 	      int result;
 	      if (v == NULL) {
 		  PyErr_SetString(PyExc_ValueError,
-				  "can not delete the oid of a cached object");
+				  "can not delete oid of cached object");
 		  return -1;
 	      }
 	      if (PyObject_Cmp(self->oid, v, &result) < 0)
 		  return -1;
 	      if (result) {
 		  PyErr_SetString(PyExc_ValueError,
-				  "can not change the oid of a cached object");
+				  "can not change oid of cached object");
 		  return -1;
 	      }
 	  }
@@ -592,32 +592,41 @@ _setattro(cPersistentObject *self, PyObject *oname, PyObject *v,
 	  ASSIGN(self->oid, v);
 	  return 0;
       }
-      if (name[3]=='j' && name[4]=='a' && name[5]=='r' && ! name[6])
-	{
+      else if (strcmp(name + 3, "jar") == 0) {
+	  if (self->cache && self->jar) {
+	      int result;
+	      if (v == NULL) {
+		  PyErr_SetString(PyExc_ValueError,
+				  "can not delete jar of cached object");
+		  return -1;
+	      }
+	      if (PyObject_Cmp(self->jar, v, &result) < 0)
+		  return -1;
+	      if (result) {
+		  PyErr_SetString(PyExc_ValueError,
+				  "can not change jar of cached object");
+		  return -1;
+	      }
+	  }
 	  Py_XINCREF(v);
 	  ASSIGN(self->jar, v);
 	  return 0;
       }
-      if (name[3]=='s' && strcmp(name+4,"erial")==0)
-	{
-	  if (v)
-	    {
-	      if (PyString_Check(v) && PyString_Size(v)==8)
+      else if (strcmp(name + 3, "serial") == 0) {
+	  if (v) {
+	      if (PyString_Check(v) && PyString_GET_SIZE(v) == 8)
 		  memcpy(self->serial, PyString_AS_STRING(v), 8);
-	      else
-		{
+	      else {
 		  PyErr_SetString(PyExc_ValueError,
 				  "_p_serial must be an 8-character string");
 		  return -1;
 	      }
-	    }
-	  else
+	  } else 
 	      memset(self->serial, 0, 8);
 	  return 0;
       }
-      if (name[3]=='c' && strcmp(name+4,"hanged")==0) 
-	{
-	  if (! v)
+      else if (strcmp(name+3, "changed") == 0) {
+	  if (!v)
 	    {
 	      /* delatter is used to invalidate the object
 	         *even* if it has changed.

src/Persistence/cPickleCache.c

@@ -88,7 +88,7 @@ process must skip such objects, rather than deactivating them.
 static char cPickleCache_doc_string[] =
 "Defines the PickleCache used by ZODB Connection objects.\n"
 "\n"
-"$Id: cPickleCache.c,v 1.57 2002/04/15 15:17:33 chrisw Exp $\n";
+"$Id: cPickleCache.c,v 1.58 2002/04/15 18:42:51 jeremy Exp $\n";
 
 #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
 #define UNLESS(E) if(!(E))
@@ -150,7 +150,9 @@ typedef struct {
 
 } ccobject;
 
+#ifdef MUCH_RING_CHECKING
 static int present_in_ring(ccobject *self, CPersistentRing *target);
+#endif
 static int ring_corrupt(ccobject *self, const char *context);
 static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
 
@@ -214,7 +216,9 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context)
 static int
 scan_gc_items(ccobject *self,int target)
 {
-    /* This function must only be called with the ring lock held */
+    /* This function must only be called with the ring lock held,
+       because it places a non-object placeholder in the ring.
+    */
 
     cPersistentObject *object;
     int error;
@@ -632,6 +636,19 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
     return 0;
 }
 
+static PyObject *
+cc_ringlen(ccobject *self, PyObject *args)
+{
+    CPersistentRing *here;
+    int c = 0;
+
+    if (!PyArg_ParseTuple(args, ":ringlen"))
+	return NULL;
+    for (here = self->ring_home.next; here != &self->ring_home;
+	 here = here->next)
+	c++;
+    return PyInt_FromLong(c);
+}
 
 static struct PyMethodDef cc_methods[] = {
   {"lru_items", (PyCFunction)cc_lru_items, METH_VARARGS,
@@ -658,6 +675,8 @@ static struct PyMethodDef cc_methods[] = {
    "invalidate(oids) -- invalidate one, many, or all ids"},
   {"get", (PyCFunction)cc_get, METH_VARARGS,
    "get(key [, default]) -- get an item, or a default"},
+  {"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
+   "ringlen() -- Returns number of non-ghost items in cache."},
   {NULL,		NULL}		/* sentinel */
 };
 
@@ -771,14 +790,14 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     cPersistentObject *p;
 
     if (!PyExtensionInstance_Check(v)) {
-	PyErr_SetString(PyExc_ValueError, 
+	PyErr_SetString(PyExc_TypeError, 
 			"Cache values must be persistent objects.");
 	return -1;
     }
     class = (PyExtensionClass *)(v->ob_type);
     if (!((class->class_flags & PERSISTENT_TYPE_FLAG)
 	  && v->ob_type->tp_basicsize >= sizeof(cPersistentObject))) {
-	PyErr_SetString(PyExc_ValueError, 
+	PyErr_SetString(PyExc_TypeError, 
 			"Cache values must be persistent objects.");
 	/* Must be either persistent classes (ie ZClasses), or instances
 	of persistent classes (ie Python classeses that derive from
@@ -790,7 +809,6 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
      *  persistent class.
      */
     oid = PyObject_GetAttr(v, py__p_oid);
-
     if (oid == NULL)
 	return -1;
     /* XXX key and oid should both be PyString objects.
@@ -806,6 +824,8 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
 	return -1;
     }
 
+    /* XXX check that object has valid _p_jar? */
+
     object_again = object_from_oid(self, key);
     if (object_again) {
 	if (object_again != v) {
@@ -911,7 +931,7 @@ cc_del_item(ccobject *self, PyObject *key)
 
     if (PyDict_DelItem(self->data, key) < 0) {
 	PyErr_SetString(PyExc_RuntimeError,
-			"unexpectedly couldnt remove key in cc_ass_sub");
+			"unexpectedly couldn't remove key in cc_ass_sub");
 	return -1;
     }
 
@@ -958,7 +978,9 @@ _ring_corrupt(ccobject *self, const char *context)
         if (here->next->prev != here) 
 	    return 10;
         if (!self->ring_lock) {
-            /* if the ring must be locked then it only contains object other than persistent instances */
+            /* If the ring must be locked, then it only contains
+	       object other than persistent instances.
+	    */ 
             if (here != &self->ring_home) {
                 cPersistentObject *object = object_from_ring(self, here, 
 							     context);
@@ -986,8 +1008,9 @@ static int
 ring_corrupt(ccobject *self, const char *context)
 {
 #ifdef MUCH_RING_CHECKING
-    int code = _ring_corrupt(self,context);
+    int code = _ring_corrupt(self, context);
     if (code) {
+	if (!PyErr_Occurred())
 	    PyErr_Format(PyExc_RuntimeError,
 			 "broken ring (code %d) in %s, size %d",
 			 code, context, PyDict_Size(self->data));
@@ -997,6 +1020,7 @@ ring_corrupt(ccobject *self, const char *context)
     return 0;
 }
 
+#ifdef MUCH_RING_CHECKING
 static int
 present_in_ring(ccobject *self,CPersistentRing *target)
 {
@@ -1009,6 +1033,7 @@ present_in_ring(ccobject *self,CPersistentRing *target)
         here = here->next;
     }
 }
+#endif
 
 static PyMappingMethods cc_as_mapping = {
   (inquiry)cc_length,		/*mp_length*/
@@ -1038,7 +1063,7 @@ static PyTypeObject Cctype = {
 };
 
 static ccobject *
-newccobject(PyObject *jar, int cache_size, int cache_age)
+newccobject(PyObject *jar, int cache_size)
 {
     ccobject *self;
   
@@ -1071,16 +1096,16 @@ newccobject(PyObject *jar, int cache_size, int cache_age)
 static PyObject *
 cCM_new(PyObject *self, PyObject *args)
 {
-    int cache_size=100, cache_age=1000;
+    int cache_size=100;
     PyObject *jar;
 
-    if (!PyArg_ParseTuple(args, "O|ii", &jar, &cache_size, &cache_age))
+    if (!PyArg_ParseTuple(args, "O|i", &jar, &cache_size))
 	return NULL;
-    return (PyObject*)newccobject(jar, cache_size, cache_age);
+    return (PyObject*)newccobject(jar, cache_size);
 }
 
 static struct PyMethodDef cCM_methods[] = {
-  {"PickleCache",(PyCFunction)cCM_new,	METH_VARARGS, ""},
+  {"PickleCache", (PyCFunction)cCM_new,	METH_VARARGS, ""},
   {NULL,		NULL}		/* sentinel */
 };
 

src/ZODB/Connection.py

@@ -13,8 +13,8 @@
 ##############################################################################
 """Database connection support
 
-$Id: Connection.py,v 1.65 2002/04/13 20:17:22 jeremy Exp $"""
-__version__='$Revision: 1.65 $'[11:-2]
+$Id: Connection.py,v 1.66 2002/04/15 18:42:50 jeremy Exp $"""
+__version__='$Revision: 1.66 $'[11:-2]
 
 from cPickleCache import PickleCache, MUCH_RING_CHECKING
 from POSException import ConflictError, ReadConflictError
@@ -69,11 +69,14 @@ class Connection(ExportImport.ExportImport):
                  cache_deactivate_after=60):
         """Create a new Connection"""
         self._version=version
-        self._cache=cache=PickleCache(self, cache_size, cache_deactivate_after)
+        self._cache = cache = PickleCache(self, cache_size)
         if version:
             # Caches for versions end up empty if the version
             # is not used for a while. Non-version caches
             # keep their content indefinitely.
+
+            # XXX Why do we want version caches to behave this way?
+
             self._cache.cache_drain_resistance = 100
         self._incrgc=self.cacheGC=cache.incrgc
         self._invalidated=d={}
@@ -209,8 +212,7 @@ class Connection(ExportImport.ExportImport):
         self._code_timestamp = global_code_timestamp
         self._invalidated.clear()
         orig_cache = self._cache
-        self._cache = PickleCache(self, orig_cache.cache_size,
-                                  orig_cache.cache_age)
+        self._cache = PickleCache(self, orig_cache.cache_size)
 
     def abort(self, object, transaction):
         """Abort the object in the transaction.
@@ -226,7 +228,8 @@ class Connection(ExportImport.ExportImport):
         self._cache.full_sweep(dt)
         
     def cacheMinimize(self, dt=0):
-        self._cache.minimize(dt)
+        # dt is ignored
+        self._cache.minimize()
 
     __onCloseCallbacks = None
     

src/ZODB/DB.py

@@ -13,8 +13,8 @@
 ##############################################################################
 """Database objects
 
-$Id: DB.py,v 1.40 2002/03/27 10:14:04 htrd Exp $"""
-__version__='$Revision: 1.40 $'[11:-2]
+$Id: DB.py,v 1.41 2002/04/15 18:42:51 jeremy Exp $"""
+__version__='$Revision: 1.41 $'[11:-2]
 
 import cPickle, cStringIO, sys, POSException, UndoLogCompatible
 from Connection import Connection
@@ -62,10 +62,10 @@ class DB(UndoLogCompatible.UndoLogCompatible):
         self._temps=[]
         self._pool_size=pool_size
         self._cache_size=cache_size
-        self._cache_deactivate_after=cache_deactivate_after
+        self._cache_deactivate_after = cache_deactivate_after
         self._version_pool_size=version_pool_size
         self._version_cache_size=version_cache_size
-        self._version_cache_deactivate_after=version_cache_deactivate_after
+        self._version_cache_deactivate_after = version_cache_deactivate_after
 
         self._miv_cache={}
 
@@ -157,17 +157,19 @@ class DB(UndoLogCompatible.UndoLogCompatible):
 
         Organized by class."""
 
-        detail={}
-        def f(con,detail=detail,have_detail=detail.has_key):
+        detail = {}
+        def f(con, detail=detail, have_detail=detail.has_key):
             for oid, ob in con._cache.items():
                 module = getattr(ob.__class__, '__module__', '')
                 module = module and '%s.' % module or ''
-                c="%s%s" % (module, ob.__class__.__name__)
-                if have_detail(c): detail[c]=detail[c]+1
-                else: detail[c]=1
+                c = "%s%s" % (module, ob.__class__.__name__)
+                if have_detail(c):
+                    detail[c] = detail[c] + 1
+                else:
+                    detail[c] = 1
         
         self._connectionMap(f)
-        detail=detail.items()
+        detail = detail.items()
         detail.sort()
         return detail
 
@@ -224,7 +226,7 @@ class DB(UndoLogCompatible.UndoLogCompatible):
     def cacheSize(self):
         m=[0]
         def f(con, m=m):
-            m[0]=m[0]+con._cache.cache_non_ghost_count
+            m[0] = m[0] + con._cache.cache_non_ghost_count
 
         self._connectionMap(f)
         return m[0]
@@ -247,8 +249,11 @@ class DB(UndoLogCompatible.UndoLogCompatible):
     def exportFile(self, oid, file=None):
         raise 'Not yet implemented'
                            
-    def getCacheDeactivateAfter(self): return self._cache_deactivate_after
-    def getCacheSize(self): return self._cache_size
+    def getCacheDeactivateAfter(self):
+        return self._cache_deactivate_after
+    
+    def getCacheSize(self):
+        return self._cache_size
 
     def getName(self): return self._storage.getName()
 
@@ -258,9 +263,12 @@ class DB(UndoLogCompatible.UndoLogCompatible):
 
     def getVersionCacheDeactivateAfter(self):
         return self._version_cache_deactivate_after
-    def getVersionCacheSize(self): return self._version_cache_size
     
-    def getVersionPoolSize(self): return self._version_pool_size
+    def getVersionCacheSize(self):
+        return self._version_cache_size
+
+    def getVersionPoolSize(self):
+        return self._version_pool_size
 
     def importFile(self, file):
         raise 'Not yet implemented'
@@ -322,7 +330,8 @@ class DB(UndoLogCompatible.UndoLogCompatible):
         cache[h]=oid, v
         return v
 
-    def objectCount(self): return len(self._storage)
+    def objectCount(self):
+        return len(self._storage)
         
     def open(self, version='', transaction=None, temporary=0, force=None,
              waitflag=1):
@@ -361,9 +370,7 @@ class DB(UndoLogCompatible.UndoLogCompatible):
                 # a one-use connection.
                 c=self.klass(
                     version=version,
-                    cache_size=self._version_cache_size,
-                    cache_deactivate_after=
-                    self._version_cache_deactivate_after)
+                    cache_size=self._version_cache_size)
                 c._setDB(self)
                 self._temps.append(c)
                 if transaction is not None: transaction[id(c)]=c
@@ -412,17 +419,13 @@ class DB(UndoLogCompatible.UndoLogCompatible):
                     if self._version_pool_size > len(allocated) or force:
                         c=self.klass(
                             version=version,
-                            cache_size=self._version_cache_size,
-                            cache_deactivate_after=
-                            self._version_cache_deactivate_after)
+                            cache_size=self._version_cache_size)
                         allocated.append(c)
                         pool.append(c)
                 elif self._pool_size > len(allocated) or force:
                     c=self.klass(
                         version=version,
-                        cache_size=self._cache_size,
-                        cache_deactivate_after=
-                        self._cache_deactivate_after)
+                        cache_size=self._cache_size)
                     allocated.append(c)
                     pool.append(c)
                     
@@ -492,19 +495,26 @@ class DB(UndoLogCompatible.UndoLogCompatible):
             raise
                            
     def setCacheDeactivateAfter(self, v):
-        self._cache_deactivate_after=v
-        for c in self._pools[0][''][1]:
-            c._cache.cache_age=v
+        self._cache_deactivate_after = v
+        d = self._pools[0]
+        pool_info = d.get('')
+        if pool_info is not None:
+            for c in pool_info[1]:
+                c._cache.cache_age = v
 
     def setCacheSize(self, v):
-        self._cache_size=v
-        for c in self._pools[0][''][1]:
-            c._cache.cache_size=v
+        self._cache_size = v
+        d = self._pools[0]
+        pool_info = d.get('')
+        if pool_info is not None:
+            for c in pool_info[1]:
+                c._cache.cache_size = v
 
     def setClassFactory(self, factory):
-        self._classFactory=factory
+        self._classFactory = factory
 
-    def setPoolSize(self, v): self._pool_size=v
+    def setPoolSize(self, v):
+        self._pool_size=v
     
     def setVersionCacheDeactivateAfter(self, v):
         self._version_cache_deactivate_after=v

src/ZODB/cPersistence.c

@@ -14,7 +14,7 @@
 static char cPersistence_doc_string[] = 
 "Defines Persistent mixin class for persistent objects.\n"
 "\n"
-"$Id: cPersistence.c,v 1.58 2002/04/05 01:12:48 jeremy Exp $\n";
+"$Id: cPersistence.c,v 1.59 2002/04/15 18:42:51 jeremy Exp $\n";
 
 #include "cPersistence.h"
 
@@ -572,19 +572,19 @@ _setattro(cPersistentObject *self, PyObject *oname, PyObject *v,
       return -1;
 	
   if (*name == '_' && name[1] == 'p' && name[2] == '_') {
-      if (name[3] == 'o' && name[4] == 'i' && name[5] == 'd' && ! name[6]) {
+      if (strcmp(name + 3, "oid") == 0) {
 	  if (self->cache) {
 	      int result;
 	      if (v == NULL) {
 		  PyErr_SetString(PyExc_ValueError,
-				  "can not delete the oid of a cached object");
+				  "can not delete oid of cached object");
 		  return -1;
 	      }
 	      if (PyObject_Cmp(self->oid, v, &result) < 0)
 		  return -1;
 	      if (result) {
 		  PyErr_SetString(PyExc_ValueError,
-				  "can not change the oid of a cached object");
+				  "can not change oid of cached object");
 		  return -1;
 	      }
 	  }
@@ -592,32 +592,41 @@ _setattro(cPersistentObject *self, PyObject *oname, PyObject *v,
 	  ASSIGN(self->oid, v);
 	  return 0;
       }
-      if (name[3]=='j' && name[4]=='a' && name[5]=='r' && ! name[6])
-	{
+      else if (strcmp(name + 3, "jar") == 0) {
+	  if (self->cache && self->jar) {
+	      int result;
+	      if (v == NULL) {
+		  PyErr_SetString(PyExc_ValueError,
+				  "can not delete jar of cached object");
+		  return -1;
+	      }
+	      if (PyObject_Cmp(self->jar, v, &result) < 0)
+		  return -1;
+	      if (result) {
+		  PyErr_SetString(PyExc_ValueError,
+				  "can not change jar of cached object");
+		  return -1;
+	      }
+	  }
 	  Py_XINCREF(v);
 	  ASSIGN(self->jar, v);
 	  return 0;
       }
-      if (name[3]=='s' && strcmp(name+4,"erial")==0)
-	{
-	  if (v)
-	    {
-	      if (PyString_Check(v) && PyString_Size(v)==8)
+      else if (strcmp(name + 3, "serial") == 0) {
+	  if (v) {
+	      if (PyString_Check(v) && PyString_GET_SIZE(v) == 8)
 		  memcpy(self->serial, PyString_AS_STRING(v), 8);
-	      else
-		{
+	      else {
 		  PyErr_SetString(PyExc_ValueError,
 				  "_p_serial must be an 8-character string");
 		  return -1;
 	      }
-	    }
-	  else
+	  } else 
 	      memset(self->serial, 0, 8);
 	  return 0;
       }
-      if (name[3]=='c' && strcmp(name+4,"hanged")==0) 
-	{
-	  if (! v)
+      else if (strcmp(name+3, "changed") == 0) {
+	  if (!v)
 	    {
 	      /* delatter is used to invalidate the object
 	         *even* if it has changed.

src/ZODB/cPickleCache.c

@@ -88,7 +88,7 @@ process must skip such objects, rather than deactivating them.
 static char cPickleCache_doc_string[] =
 "Defines the PickleCache used by ZODB Connection objects.\n"
 "\n"
-"$Id: cPickleCache.c,v 1.57 2002/04/15 15:17:33 chrisw Exp $\n";
+"$Id: cPickleCache.c,v 1.58 2002/04/15 18:42:51 jeremy Exp $\n";
 
 #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
 #define UNLESS(E) if(!(E))
@@ -150,7 +150,9 @@ typedef struct {
 
 } ccobject;
 
+#ifdef MUCH_RING_CHECKING
 static int present_in_ring(ccobject *self, CPersistentRing *target);
+#endif
 static int ring_corrupt(ccobject *self, const char *context);
 static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
 
@@ -214,7 +216,9 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context)
 static int
 scan_gc_items(ccobject *self,int target)
 {
-    /* This function must only be called with the ring lock held */
+    /* This function must only be called with the ring lock held,
+       because it places a non-object placeholder in the ring.
+    */
 
     cPersistentObject *object;
     int error;
@@ -632,6 +636,19 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
     return 0;
 }
 
+static PyObject *
+cc_ringlen(ccobject *self, PyObject *args)
+{
+    CPersistentRing *here;
+    int c = 0;
+
+    if (!PyArg_ParseTuple(args, ":ringlen"))
+	return NULL;
+    for (here = self->ring_home.next; here != &self->ring_home;
+	 here = here->next)
+	c++;
+    return PyInt_FromLong(c);
+}
 
 static struct PyMethodDef cc_methods[] = {
   {"lru_items", (PyCFunction)cc_lru_items, METH_VARARGS,
@@ -658,6 +675,8 @@ static struct PyMethodDef cc_methods[] = {
    "invalidate(oids) -- invalidate one, many, or all ids"},
   {"get", (PyCFunction)cc_get, METH_VARARGS,
    "get(key [, default]) -- get an item, or a default"},
+  {"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
+   "ringlen() -- Returns number of non-ghost items in cache."},
   {NULL,		NULL}		/* sentinel */
 };
 
@@ -771,14 +790,14 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     cPersistentObject *p;
 
     if (!PyExtensionInstance_Check(v)) {
-	PyErr_SetString(PyExc_ValueError, 
+	PyErr_SetString(PyExc_TypeError, 
 			"Cache values must be persistent objects.");
 	return -1;
     }
     class = (PyExtensionClass *)(v->ob_type);
     if (!((class->class_flags & PERSISTENT_TYPE_FLAG)
 	  && v->ob_type->tp_basicsize >= sizeof(cPersistentObject))) {
-	PyErr_SetString(PyExc_ValueError, 
+	PyErr_SetString(PyExc_TypeError, 
 			"Cache values must be persistent objects.");
 	/* Must be either persistent classes (ie ZClasses), or instances
 	of persistent classes (ie Python classeses that derive from
@@ -790,7 +809,6 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
      *  persistent class.
      */
     oid = PyObject_GetAttr(v, py__p_oid);
-
     if (oid == NULL)
 	return -1;
     /* XXX key and oid should both be PyString objects.
@@ -806,6 +824,8 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
 	return -1;
     }
 
+    /* XXX check that object has valid _p_jar? */
+
     object_again = object_from_oid(self, key);
     if (object_again) {
 	if (object_again != v) {
@@ -911,7 +931,7 @@ cc_del_item(ccobject *self, PyObject *key)
 
     if (PyDict_DelItem(self->data, key) < 0) {
 	PyErr_SetString(PyExc_RuntimeError,
-			"unexpectedly couldnt remove key in cc_ass_sub");
+			"unexpectedly couldn't remove key in cc_ass_sub");
 	return -1;
     }
 
@@ -958,7 +978,9 @@ _ring_corrupt(ccobject *self, const char *context)
         if (here->next->prev != here) 
 	    return 10;
         if (!self->ring_lock) {
-            /* if the ring must be locked then it only contains object other than persistent instances */
+            /* If the ring must be locked, then it only contains
+	       object other than persistent instances.
+	    */ 
             if (here != &self->ring_home) {
                 cPersistentObject *object = object_from_ring(self, here, 
 							     context);
@@ -986,8 +1008,9 @@ static int
 ring_corrupt(ccobject *self, const char *context)
 {
 #ifdef MUCH_RING_CHECKING
-    int code = _ring_corrupt(self,context);
+    int code = _ring_corrupt(self, context);
     if (code) {
+	if (!PyErr_Occurred())
 	    PyErr_Format(PyExc_RuntimeError,
 			 "broken ring (code %d) in %s, size %d",
 			 code, context, PyDict_Size(self->data));
@@ -997,6 +1020,7 @@ ring_corrupt(ccobject *self, const char *context)
     return 0;
 }
 
+#ifdef MUCH_RING_CHECKING
 static int
 present_in_ring(ccobject *self,CPersistentRing *target)
 {
@@ -1009,6 +1033,7 @@ present_in_ring(ccobject *self,CPersistentRing *target)
         here = here->next;
     }
 }
+#endif
 
 static PyMappingMethods cc_as_mapping = {
   (inquiry)cc_length,		/*mp_length*/
@@ -1038,7 +1063,7 @@ static PyTypeObject Cctype = {
 };
 
 static ccobject *
-newccobject(PyObject *jar, int cache_size, int cache_age)
+newccobject(PyObject *jar, int cache_size)
 {
     ccobject *self;
   
@@ -1071,16 +1096,16 @@ newccobject(PyObject *jar, int cache_size, int cache_age)
 static PyObject *
 cCM_new(PyObject *self, PyObject *args)
 {
-    int cache_size=100, cache_age=1000;
+    int cache_size=100;
     PyObject *jar;
 
-    if (!PyArg_ParseTuple(args, "O|ii", &jar, &cache_size, &cache_age))
+    if (!PyArg_ParseTuple(args, "O|i", &jar, &cache_size))
 	return NULL;
-    return (PyObject*)newccobject(jar, cache_size, cache_age);
+    return (PyObject*)newccobject(jar, cache_size);
 }
 
 static struct PyMethodDef cCM_methods[] = {
-  {"PickleCache",(PyCFunction)cCM_new,	METH_VARARGS, ""},
+  {"PickleCache", (PyCFunction)cCM_new,	METH_VARARGS, ""},
   {NULL,		NULL}		/* sentinel */
 };
 

src/persistent/cPersistence.c

@@ -14,7 +14,7 @@
 static char cPersistence_doc_string[] = 
 "Defines Persistent mixin class for persistent objects.\n"
 "\n"
-"$Id: cPersistence.c,v 1.58 2002/04/05 01:12:48 jeremy Exp $\n";
+"$Id: cPersistence.c,v 1.59 2002/04/15 18:42:51 jeremy Exp $\n";
 
 #include "cPersistence.h"
 
@@ -572,19 +572,19 @@ _setattro(cPersistentObject *self, PyObject *oname, PyObject *v,
       return -1;
 	
   if (*name == '_' && name[1] == 'p' && name[2] == '_') {
-      if (name[3] == 'o' && name[4] == 'i' && name[5] == 'd' && ! name[6]) {
+      if (strcmp(name + 3, "oid") == 0) {
 	  if (self->cache) {
 	      int result;
 	      if (v == NULL) {
 		  PyErr_SetString(PyExc_ValueError,
-				  "can not delete the oid of a cached object");
+				  "can not delete oid of cached object");
 		  return -1;
 	      }
 	      if (PyObject_Cmp(self->oid, v, &result) < 0)
 		  return -1;
 	      if (result) {
 		  PyErr_SetString(PyExc_ValueError,
-				  "can not change the oid of a cached object");
+				  "can not change oid of cached object");
 		  return -1;
 	      }
 	  }
@@ -592,32 +592,41 @@ _setattro(cPersistentObject *self, PyObject *oname, PyObject *v,
 	  ASSIGN(self->oid, v);
 	  return 0;
       }
-      if (name[3]=='j' && name[4]=='a' && name[5]=='r' && ! name[6])
-	{
+      else if (strcmp(name + 3, "jar") == 0) {
+	  if (self->cache && self->jar) {
+	      int result;
+	      if (v == NULL) {
+		  PyErr_SetString(PyExc_ValueError,
+				  "can not delete jar of cached object");
+		  return -1;
+	      }
+	      if (PyObject_Cmp(self->jar, v, &result) < 0)
+		  return -1;
+	      if (result) {
+		  PyErr_SetString(PyExc_ValueError,
+				  "can not change jar of cached object");
+		  return -1;
+	      }
+	  }
 	  Py_XINCREF(v);
 	  ASSIGN(self->jar, v);
 	  return 0;
       }
-      if (name[3]=='s' && strcmp(name+4,"erial")==0)
-	{
-	  if (v)
-	    {
-	      if (PyString_Check(v) && PyString_Size(v)==8)
+      else if (strcmp(name + 3, "serial") == 0) {
+	  if (v) {
+	      if (PyString_Check(v) && PyString_GET_SIZE(v) == 8)
 		  memcpy(self->serial, PyString_AS_STRING(v), 8);
-	      else
-		{
+	      else {
 		  PyErr_SetString(PyExc_ValueError,
 				  "_p_serial must be an 8-character string");
 		  return -1;
 	      }
-	    }
-	  else
+	  } else 
 	      memset(self->serial, 0, 8);
 	  return 0;
       }
-      if (name[3]=='c' && strcmp(name+4,"hanged")==0) 
-	{
-	  if (! v)
+      else if (strcmp(name+3, "changed") == 0) {
+	  if (!v)
 	    {
 	      /* delatter is used to invalidate the object
 	         *even* if it has changed.

src/persistent/cPickleCache.c

@@ -88,7 +88,7 @@ process must skip such objects, rather than deactivating them.
 static char cPickleCache_doc_string[] =
 "Defines the PickleCache used by ZODB Connection objects.\n"
 "\n"
-"$Id: cPickleCache.c,v 1.57 2002/04/15 15:17:33 chrisw Exp $\n";
+"$Id: cPickleCache.c,v 1.58 2002/04/15 18:42:51 jeremy Exp $\n";
 
 #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
 #define UNLESS(E) if(!(E))
@@ -150,7 +150,9 @@ typedef struct {
 
 } ccobject;
 
+#ifdef MUCH_RING_CHECKING
 static int present_in_ring(ccobject *self, CPersistentRing *target);
+#endif
 static int ring_corrupt(ccobject *self, const char *context);
 static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
 
@@ -214,7 +216,9 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context)
 static int
 scan_gc_items(ccobject *self,int target)
 {
-    /* This function must only be called with the ring lock held */
+    /* This function must only be called with the ring lock held,
+       because it places a non-object placeholder in the ring.
+    */
 
     cPersistentObject *object;
     int error;
@@ -632,6 +636,19 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
     return 0;
 }
 
+static PyObject *
+cc_ringlen(ccobject *self, PyObject *args)
+{
+    CPersistentRing *here;
+    int c = 0;
+
+    if (!PyArg_ParseTuple(args, ":ringlen"))
+	return NULL;
+    for (here = self->ring_home.next; here != &self->ring_home;
+	 here = here->next)
+	c++;
+    return PyInt_FromLong(c);
+}
 
 static struct PyMethodDef cc_methods[] = {
   {"lru_items", (PyCFunction)cc_lru_items, METH_VARARGS,
@@ -658,6 +675,8 @@ static struct PyMethodDef cc_methods[] = {
    "invalidate(oids) -- invalidate one, many, or all ids"},
   {"get", (PyCFunction)cc_get, METH_VARARGS,
    "get(key [, default]) -- get an item, or a default"},
+  {"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
+   "ringlen() -- Returns number of non-ghost items in cache."},
   {NULL,		NULL}		/* sentinel */
 };
 
@@ -771,14 +790,14 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     cPersistentObject *p;
 
     if (!PyExtensionInstance_Check(v)) {
-	PyErr_SetString(PyExc_ValueError, 
+	PyErr_SetString(PyExc_TypeError, 
 			"Cache values must be persistent objects.");
 	return -1;
     }
     class = (PyExtensionClass *)(v->ob_type);
     if (!((class->class_flags & PERSISTENT_TYPE_FLAG)
 	  && v->ob_type->tp_basicsize >= sizeof(cPersistentObject))) {
-	PyErr_SetString(PyExc_ValueError, 
+	PyErr_SetString(PyExc_TypeError, 
 			"Cache values must be persistent objects.");
 	/* Must be either persistent classes (ie ZClasses), or instances
 	of persistent classes (ie Python classeses that derive from
@@ -790,7 +809,6 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
      *  persistent class.
      */
     oid = PyObject_GetAttr(v, py__p_oid);
-
     if (oid == NULL)
 	return -1;
     /* XXX key and oid should both be PyString objects.
@@ -806,6 +824,8 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
 	return -1;
     }
 
+    /* XXX check that object has valid _p_jar? */
+
     object_again = object_from_oid(self, key);
     if (object_again) {
 	if (object_again != v) {
@@ -911,7 +931,7 @@ cc_del_item(ccobject *self, PyObject *key)
 
     if (PyDict_DelItem(self->data, key) < 0) {
 	PyErr_SetString(PyExc_RuntimeError,
-			"unexpectedly couldnt remove key in cc_ass_sub");
+			"unexpectedly couldn't remove key in cc_ass_sub");
 	return -1;
     }
 
@@ -958,7 +978,9 @@ _ring_corrupt(ccobject *self, const char *context)
         if (here->next->prev != here) 
 	    return 10;
         if (!self->ring_lock) {
-            /* if the ring must be locked then it only contains object other than persistent instances */
+            /* If the ring must be locked, then it only contains
+	       object other than persistent instances.
+	    */ 
             if (here != &self->ring_home) {
                 cPersistentObject *object = object_from_ring(self, here, 
 							     context);
@@ -986,8 +1008,9 @@ static int
 ring_corrupt(ccobject *self, const char *context)
 {
 #ifdef MUCH_RING_CHECKING
-    int code = _ring_corrupt(self,context);
+    int code = _ring_corrupt(self, context);
     if (code) {
+	if (!PyErr_Occurred())
 	    PyErr_Format(PyExc_RuntimeError,
 			 "broken ring (code %d) in %s, size %d",
 			 code, context, PyDict_Size(self->data));
@@ -997,6 +1020,7 @@ ring_corrupt(ccobject *self, const char *context)
     return 0;
 }
 
+#ifdef MUCH_RING_CHECKING
 static int
 present_in_ring(ccobject *self,CPersistentRing *target)
 {
@@ -1009,6 +1033,7 @@ present_in_ring(ccobject *self,CPersistentRing *target)
         here = here->next;
     }
 }
+#endif
 
 static PyMappingMethods cc_as_mapping = {
   (inquiry)cc_length,		/*mp_length*/
@@ -1038,7 +1063,7 @@ static PyTypeObject Cctype = {
 };
 
 static ccobject *
-newccobject(PyObject *jar, int cache_size, int cache_age)
+newccobject(PyObject *jar, int cache_size)
 {
     ccobject *self;
   
@@ -1071,16 +1096,16 @@ newccobject(PyObject *jar, int cache_size, int cache_age)
 static PyObject *
 cCM_new(PyObject *self, PyObject *args)
 {
-    int cache_size=100, cache_age=1000;
+    int cache_size=100;
     PyObject *jar;
 
-    if (!PyArg_ParseTuple(args, "O|ii", &jar, &cache_size, &cache_age))
+    if (!PyArg_ParseTuple(args, "O|i", &jar, &cache_size))
 	return NULL;
-    return (PyObject*)newccobject(jar, cache_size, cache_age);
+    return (PyObject*)newccobject(jar, cache_size);
 }
 
 static struct PyMethodDef cCM_methods[] = {
-  {"PickleCache",(PyCFunction)cCM_new,	METH_VARARGS, ""},
+  {"PickleCache", (PyCFunction)cCM_new,	METH_VARARGS, ""},
   {NULL,		NULL}		/* sentinel */
 };