[Zope-Checkins] CVS: ZODB3/Persistence - cPickleCache.c:1.85.8.5

Jeremy Hylton jeremy@zope.com
Tue, 8 Jul 2003 09:56:54 -0400


Update of /cvs-repository/ZODB3/Persistence
In directory cvs.zope.org:/tmp/cvs-serv32403/Persistence

Modified Files:
      Tag: zodb33-devel-branch
	cPickleCache.c 
Log Message:
Bunch of cache changes.

Replace getattr and setattr hooks with members and getsets.  

Give type an __init__ and replace PickleCache function provided by
module with type.



=== ZODB3/Persistence/cPickleCache.c 1.85.8.4 => 1.85.8.5 ===
--- ZODB3/Persistence/cPickleCache.c:1.85.8.4	Mon Jul  7 14:22:29 2003
+++ ZODB3/Persistence/cPickleCache.c	Tue Jul  8 09:56:49 2003
@@ -94,6 +94,7 @@
 
 #define DONT_USE_CPERSISTENCECAPI
 #include "cPersistence.h"
+#include "structmember.h"
 #include <time.h>
 #include <stddef.h>
 #undef Py_FindMethod
@@ -158,7 +159,7 @@
      */
     while (1) {
 	/* back to the home position. stop looking */
-        if (here == &self->ring_home)
+        if (here == &self->ring_home) 
             return 0;
 
         /* At this point we know that the ring only contains nodes
@@ -195,19 +196,17 @@
             error = PyObject_SetAttr((PyObject *)object, py__p_changed, 
 				     Py_None);
 
-
             /* unlink the placeholder */
             placeholder.next->prev = placeholder.prev;
             placeholder.prev->next = placeholder.next;
 
             here = placeholder.next;
 
-            if (error)
+            if (error) 
                 return -1; /* problem */
         }
-        else {
+        else 
             here = here->next;
-        }
     }
 }
 
@@ -305,26 +304,22 @@
 }
 
 static PyObject *
-cc_invalidate(ccobject *self, PyObject *args)
+cc_invalidate(ccobject *self, PyObject *inv)
 {
-  PyObject *inv, *key, *v;
+  PyObject *key, *v;
   int i = 0;
 
-  if (PyArg_ParseTuple(args, "O!", &PyDict_Type, &inv)) {
+  if (PyDict_Check(inv)) {
       while (PyDict_Next(inv, &i, &key, &v))
 	  _invalidate(self, key);
       PyDict_Clear(inv);
   }
   else {
-      PyErr_Clear();
-      if (!PyArg_ParseTuple(args, "O:invalidate", &inv)) 
-	  return NULL;
       if (PyString_Check(inv))
 	  _invalidate(self, inv);
       else {
 	  int l;
 	  
-	  PyErr_Clear();
 	  l = PyObject_Length(inv);
 	  if (l < 0)
 	      return NULL;
@@ -366,14 +361,17 @@
 }
 
 static PyObject *
-cc_klass_items(ccobject *self, PyObject *args)
+cc_items(ccobject *self)
+{
+    return PyObject_CallMethod(self->data, "items", "");
+}
+
+static PyObject *
+cc_klass_items(ccobject *self)
 {
     PyObject *l,*k,*v;
     int p = 0;
 
-    if (!PyArg_ParseTuple(args, ":klass_items")) 
-	return NULL;
-
     l = PyList_New(PyDict_Size(self->data));
     if (l == NULL) 
 	return NULL;
@@ -398,14 +396,11 @@
 }
 
 static PyObject *
-cc_lru_items(ccobject *self, PyObject *args)
+cc_lru_items(ccobject *self)
 {
     PyObject *l;
     CPersistentRing *here;
 
-    if (!PyArg_ParseTuple(args, ":lru_items")) 
-	return NULL;
-
     if (self->ring_lock) {
 	/* When the ring lock is held, we have no way of know which
 	   ring nodes belong to persistent objects, and which a
@@ -511,13 +506,11 @@
 }
 
 static PyObject *
-cc_ringlen(ccobject *self, PyObject *args)
+cc_ringlen(ccobject *self)
 {
     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++;
@@ -525,100 +518,162 @@
 }
 
 static struct PyMethodDef cc_methods[] = {
-  {"lru_items", (PyCFunction)cc_lru_items, METH_VARARGS,
-   "List (oid, object) pairs from the lru list, as 2-tuples.\n"
-   },
-  {"klass_items", (PyCFunction)cc_klass_items, METH_VARARGS,
-   "List (oid, object) pairs of cached persistent classes.\n"
-   },
-  {"full_sweep", (PyCFunction)cc_full_sweep, METH_VARARGS,
-   "full_sweep([age]) -- Perform a full sweep of the cache\n\n"
-   "Supported for backwards compatibility.  If the age argument is 0,\n"
-   "behaves like minimize().  Otherwise, behaves like incrgc()."
-   },
-  {"minimize",	(PyCFunction)cc_minimize, METH_VARARGS,
-   "minimize([ignored]) -- Remove as many objects as possible\n\n"
-   "Ghostify all objects that are not modified.  Takes an optional\n"
-   "argument, but ignores it."
-   },
-  {"incrgc", (PyCFunction)cc_incrgc, METH_VARARGS,
-   "incrgc([n]) -- Perform incremental garbage collection\n\n"
-   "Some other implementations support an optional parameter 'n' which\n"
-   "indicates a repetition count; this value is ignored.\n"},
-  {"invalidate", (PyCFunction)cc_invalidate, METH_VARARGS,
-   "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 */
+    {"items", (PyCFunction)cc_items, METH_NOARGS,
+     "Return list of oid, object pairs for all items in cache."},
+    {"lru_items", (PyCFunction)cc_lru_items, METH_NOARGS,
+     "List (oid, object) pairs from the lru list, as 2-tuples."},
+    {"klass_items", (PyCFunction)cc_klass_items, METH_NOARGS,
+     "List (oid, object) pairs of cached persistent classes."},
+    {"full_sweep", (PyCFunction)cc_full_sweep, METH_VARARGS,
+     "full_sweep([age]) -- Perform a full sweep of the cache\n\n"
+     "Supported for backwards compatibility.  If the age argument is 0,\n"
+     "behaves like minimize().  Otherwise, behaves like incrgc()."},
+    {"minimize",	(PyCFunction)cc_minimize, METH_VARARGS,
+     "minimize([ignored]) -- Remove as many objects as possible\n\n"
+     "Ghostify all objects that are not modified.  Takes an optional\n"
+     "argument, but ignores it."},
+    {"incrgc", (PyCFunction)cc_incrgc, METH_VARARGS,
+     "incrgc([n]) -- Perform incremental garbage collection\n\n"
+     "Some other implementations support an optional parameter 'n' which\n"
+     "indicates a repetition count; this value is ignored."},
+    {"invalidate", (PyCFunction)cc_invalidate, METH_O,
+     "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_NOARGS,
+     "ringlen() -- Returns number of non-ghost items in cache."},
+    {NULL, NULL}		/* sentinel */
 };
 
+static int
+cc_init(ccobject *self, PyObject *args, PyObject *kwds)
+{
+    int cache_size = 100;
+    PyObject *jar;
+
+    if (!PyArg_ParseTuple(args, "O|i", &jar, &cache_size))
+	return -1;
+
+    self->setklassstate = self->jar = NULL;
+    self->data = PyDict_New();
+    if (self->data == NULL) {
+	Py_DECREF(self);
+	return -1;
+    }
+    /* Untrack the dict mapping oids to objects.
+
+    The dict contains uncounted references to ghost objects, so it
+    isn't safe for GC to visit it.  If GC finds an object with more
+    referents that refcounts, it will die with an assertion failure.
+
+    When the cache participates in GC, it will need to traverse the
+    objects in the doubly-linked list, which will account for all the
+    non-ghost objects.
+    */
+    PyObject_GC_UnTrack((void *)self->data);
+    self->setklassstate = PyObject_GetAttrString(jar, "setklassstate");
+    if (self->setklassstate == NULL) {
+	Py_DECREF(self);
+	return -1;
+    }
+    self->jar = jar; 
+    Py_INCREF(jar);
+    self->cache_size = cache_size;
+    self->non_ghost_count = 0;
+    self->klass_count = 0;
+    self->cache_drain_resistance = 0;
+    self->ring_lock = 0;
+    self->ring_home.next = &self->ring_home;
+    self->ring_home.prev = &self->ring_home;
+    return 0;
+}
+
 static void
 cc_dealloc(ccobject *self)
 {
     Py_XDECREF(self->data);
     Py_XDECREF(self->jar);
     Py_XDECREF(self->setklassstate);
-    PyMem_DEL(self);
+    PyObject_GC_Del(self);
 }
 
-static PyObject *
-cc_getattr(ccobject *self, char *name)
+static int
+cc_clear(ccobject *self)
 {
-  if(*name=='c')
-    {
-      if(strcmp(name,"cache_age")==0)
-	return PyInt_FromLong(0);   /* this cache does not use this value */
-      if(strcmp(name,"cache_size")==0)
-	return PyInt_FromLong(self->cache_size);
-      if(strcmp(name,"cache_drain_resistance")==0)
-	return PyInt_FromLong(self->cache_drain_resistance);
-      if(strcmp(name,"cache_non_ghost_count")==0)
-	return PyInt_FromLong(self->non_ghost_count);
-      if(strcmp(name,"cache_klass_count")==0)
-	return PyInt_FromLong(self->klass_count);
-      if(strcmp(name,"cache_data")==0)
-	{
-	  /* now a copy of our data; the ring is too fragile */
-	  return PyDict_Copy(self->data);
-	}
+    int pos;
+    PyObject *k, *v;
+    /* Clearing the cache is delicate.
+       
+    A non-ghost object will show up in the ring and in the dict.  If
+    we deallocating the dict before clearing the ring, the GC will
+    decref each object in the dict.  Since the dict references are
+    uncounted, this will lead to objects having negative refcounts.
+
+    Freeing the non-ghost objects should eliminate many objects from
+    the cache, but there may still be ghost objects left.  It's
+    not safe to decref the dict until it's empty, so we need to manually
+    clear those out of the dict, too.  We accomplish that by replacing
+    all the ghost objects with None.
+    */
+
+    /* XXX Is it possible to deadlock if clear is called at a bad time? 
+
+    Yes.  When lockgc() executes, it is possible to cause enough
+    excess allocations to trigger a gc.  So we need a better solution.
+    
+    */
+    if (!lockgc(self, 0)) {
+	PyErr_WriteUnraisable((PyObject *)self);
+	return -1;
     }
+    assert(self->non_ghost_count == 0);
+
+    Py_XDECREF(self->jar);
+    Py_XDECREF(self->setklassstate);
 
-  if (strcmp(name, "items") == 0)
-      return PyObject_GetAttrString(self->data, name);
-  return Py_FindMethod(cc_methods, (PyObject *)self, name);
+    while (PyDict_Next(self->data, &pos, &k, &v)) {
+	Py_INCREF(v);
+	if (PyDict_SetItem(self->data, k, Py_None) < 0)
+	    return -1;
+    }
+    Py_XDECREF(self->data);
+    self->data = NULL;
+    self->jar = NULL;
+    self->setklassstate = NULL;
+    return 0;
 }
 
 static int
-cc_setattr(ccobject *self, char *name, PyObject *value)
+cc_traverse(ccobject *self, visitproc visit, void *arg)
 {
-  if(value)
-    {
-      int v;
-
-      if(strcmp(name,"cache_age")==0)
-	{
-	  /* this cache doesnt use the age */
-	  return 0;
-	}
+    int err;
+    CPersistentRing *here;
 
-      if (strcmp(name, "cache_size") == 0) {
-	    if (!PyArg_Parse(value,"i",&v)) 
-		return -1;
-	    self->cache_size=v;
-	    return 0;
-      }
+#define VISIT(SLOT) \
+    if (SLOT) { \
+	err = visit((PyObject *)(SLOT), arg); \
+	if (err) \
+		     return err; \
+    }
+    
+    VISIT(self->jar);
+    VISIT(self->setklassstate);
 
-      if (strcmp(name, "cache_drain_resistance") == 0) {
-	  if (!PyArg_Parse(value,"i",&v)) 
-	      return -1;
-	  self->cache_drain_resistance = v;
-	  return 0;
-      }
+    here = self->ring_home.next;
+
+    /* It is possible that an object is traversed after it is cleared.
+       In that case, there is no ring.
+    */
+    if (!here)
+	return 0;
+
+    while (here != &self->ring_home) {
+	cPersistentObject *o = OBJECT_FROM_RING(self, here, "foo");
+	VISIT(o);
+	here = here->next;
     }
-  PyErr_SetString(PyExc_AttributeError, name);
-  return -1;
+    
+    return 0;
 }
 
 static int
@@ -824,83 +879,75 @@
   (objobjargproc)cc_ass_sub,	/*mp_ass_subscript*/
 };
 
-static PyTypeObject Cctype = {
-    PyObject_HEAD_INIT(NULL)
-    0,				/*ob_size*/
-    "Persistence.cPickleCache",	/*tp_name*/
-    sizeof(ccobject),		/*tp_basicsize*/
-    0,				/*tp_itemsize*/
-    /* methods */
-    (destructor)cc_dealloc,	/*tp_dealloc*/
-    (printfunc)0,		/*tp_print*/
-    (getattrfunc)cc_getattr,	/*tp_getattr*/
-    (setattrfunc)cc_setattr,	/*tp_setattr*/
-    (cmpfunc)0,			/*tp_compare*/
-    (reprfunc)0,   		/*tp_repr*/
-    0,				/*tp_as_number*/
-    0,				/*tp_as_sequence*/
-    &cc_as_mapping,		/*tp_as_mapping*/
-    (hashfunc)0,		/*tp_hash*/
-    (ternaryfunc)0,		/*tp_call*/
-    (reprfunc)0,  		/*tp_str*/
-};
-
-static ccobject *
-newccobject(PyObject *jar, int cache_size)
+static PyObject *
+cc_cache_data(ccobject *self, void *context)
 {
-    ccobject *self;
-  
-    self = PyObject_NEW(ccobject, &Cctype);
-    if (self == NULL)
-	return NULL;
-    self->setklassstate = self->jar = NULL;
-    self->data = PyDict_New();
-    if (self->data == NULL) {
-	Py_DECREF(self);
-	return NULL;
-    }
-    /* Untrack the dict mapping oids to objects.
+    return PyDict_Copy(self->data);
+}
 
-    The dict contains uncounted references to ghost objects, so it
-    isn't safe for GC to visit it.  If GC finds an object with more
-    referents that refcounts, it will die with an assertion failure.
+static PyGetSetDef cc_getsets[] = {
+    {"cache_data", (getter)cc_cache_data},
+    {NULL}
+};
 
-    When the cache participates in GC, it will need to traverse the
-    objects in the doubly-linked list, which will account for all the
-    non-ghost objects.
-    */
-    PyObject_GC_UnTrack((void *)self->data);
-    self->setklassstate = PyObject_GetAttrString(jar, "setklassstate");
-    if (self->setklassstate == NULL) {
-	Py_DECREF(self);
-	return NULL;
-    }
-    self->jar = jar; 
-    Py_INCREF(jar);
-    self->cache_size = cache_size;
-    self->non_ghost_count = 0;
-    self->klass_count = 0;
-    self->cache_drain_resistance = 0;
-    self->ring_lock = 0;
-    self->ring_home.next = &self->ring_home;
-    self->ring_home.prev = &self->ring_home;
-    return self;
-}
 
-static PyObject *
-cCM_new(PyObject *self, PyObject *args)
-{
-    int cache_size=100;
-    PyObject *jar;
+static PyMemberDef cc_members[] = {
+    {"cache_size", T_INT, offsetof(ccobject, cache_size)},
+    {"cache_drain_resistance", T_INT, 
+     offsetof(ccobject, cache_drain_resistance)},
+    {"cache_non_ghost_count", T_INT, offsetof(ccobject, non_ghost_count), RO},
+    {"cache_klass_count", T_INT, offsetof(ccobject, klass_count), RO},
+    {NULL}
+};
 
-    if (!PyArg_ParseTuple(args, "O|i", &jar, &cache_size))
-	return NULL;
-    return (PyObject*)newccobject(jar, cache_size);
-}
+/* This module is compiled as a shared library.  Some compilers don't
+   allow addresses of Python objects defined in other libraries to be
+   used in static initializers here.  The DEFERRED_ADDRESS macro is
+   used to tag the slots where such addresses appear; the module init
+   function must fill in the tagged slots at runtime.  The argument is
+   for documentation -- the macro ignores it.
+*/
+#define DEFERRED_ADDRESS(ADDR) 0
 
-static struct PyMethodDef cCM_methods[] = {
-    {"PickleCache", (PyCFunction)cCM_new, METH_VARARGS},
-    {NULL, NULL} /* sentinel */
+static PyTypeObject Cctype = {
+    PyObject_HEAD_INIT(DEFERRED_ADDRESS(&PyType_Type))
+    0,					/* ob_size */
+    "Persistence.cPickleCache",		/* tp_name */
+    sizeof(ccobject),			/* tp_basicsize */
+    0,					/* tp_itemsize */
+    (destructor)cc_dealloc,		/* tp_dealloc */
+    0,					/* tp_print */
+    0,					/* tp_getattr */
+    0,					/* tp_setattr */
+    0,					/* tp_compare */
+    0,					/* tp_repr */
+    0,					/* tp_as_number */
+    0,					/* tp_as_sequence */
+    &cc_as_mapping,			/* tp_as_mapping */
+    0,					/* tp_hash */
+    0,					/* tp_call */
+    0,					/* tp_str */
+    0,					/* tp_getattro */
+    0,					/* tp_setattro */
+    0,					/* tp_as_buffer */
+    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
+    					/* tp_flags */
+    0,					/* tp_doc */
+    (traverseproc)cc_traverse,		/* tp_traverse */
+    (inquiry)cc_clear,			/* tp_clear */
+    0,					/* tp_richcompare */
+    0,					/* tp_weaklistoffset */
+    0,					/* tp_iter */
+    0,					/* tp_iternext */
+    cc_methods,				/* tp_methods */
+    cc_members,				/* tp_members */
+    cc_getsets,				/* tp_getset */
+    0,					/* tp_base */
+    0,					/* tp_dict */
+    0,					/* tp_descr_get */
+    0,					/* tp_descr_set */
+    0,					/* tp_dictoffset */
+    (initproc)cc_init,			/* tp_init */
 };
 
 void
@@ -909,9 +956,12 @@
     PyObject *m;
 
     Cctype.ob_type = &PyType_Type;
+    Cctype.tp_new = &PyType_GenericNew;
+    if (PyType_Ready(&Cctype) < 0) {
+	return;
+    }
 
-    m = Py_InitModule4("cPickleCache", cCM_methods, cPickleCache_doc_string,
-		       (PyObject*)NULL, PYTHON_API_VERSION);
+    m = Py_InitModule3("cPickleCache", NULL, cPickleCache_doc_string);
 
     capi = (cPersistenceCAPIstruct *)PyCObject_Import(
 	"Persistence.cPersistence", "CAPI");
@@ -924,5 +974,10 @@
     py__p_changed = PyString_InternFromString("_p_changed");
     py__p_oid = PyString_InternFromString("_p_oid");
 
-    PyModule_AddStringConstant(m, "cache_variant", "stiff/c");
+    if (PyModule_AddStringConstant(m, "cache_variant", "stiff/c") < 0)
+	return;
+
+    /* This leaks a reference to Cctype, but it doesn't matter. */
+    if (PyModule_AddObject(m, "PickleCache", (PyObject *)&Cctype) < 0)
+	return;
 }