Skip to content

Z
ZEO
Commit db5a7f5a authored by Barry Warsaw's avatar Barry Warsaw
Browse files
Options

Code cleanup and XXX comments. 

check_ring() rename to ring_corrupt() since that matches the return
semantics a bit better.

scan_gc_items(): Get rid of the placeholder, which just seemed like a
confusing way to stash here->next until after the PyObject_SetAttr()
call.
parent 5513b31c
Hide whitespace changes
Inline Side-by-side
Showing with 177 additions and 150 deletions
src/Persistence/cPickleCache.c
View file @ db5a7f5a
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
static char cPickleCache_doc_string[] = static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\n" "Defines the PickleCache used by ZODB Connection objects.\n"
"\n" "\n"
"$Id: cPickleCache.c,v 1.48 2002/04/02 19:47:19 jeremy Exp $\n"; "$Id: cPickleCache.c,v 1.49 2002/04/02 22:22:03 bwarsaw Exp $\n";
#define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;} #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
#define UNLESS(E) if(!(E)) #define UNLESS(E) if(!(E))
...@@ -44,20 +44,27 @@ static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed; ...@@ -44,20 +44,27 @@ static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed;
#define ENGINE_NOISE(A) ((void)A) #define ENGINE_NOISE(A) ((void)A)
#endif #endif
/* the layout of this struct is the same as the start of ccobject_head in cPersistence.c */ /* This object is the pickle cache. The layout of this struct is the same
* as the start of ccobject_head in cPersistence.c
* XXX Why do they need to have the same layouts?
*/
typedef struct { typedef struct {
CACHE_HEAD CACHE_HEAD
int klass_count; int klass_count; /* XXX for ZClass support? */
PyObject *data; PyObject *data; /* oid -> object dict */
PyObject *jar; PyObject *jar; /* Connection object */
PyObject *setklassstate; PyObject *setklassstate; /* ??? */
int cache_size; int cache_size; /* number of items in cache */
int ring_lock; int ring_lock; /* ??? */
/* XXX Settable from Python, this appears to be a ratio controlling how
* the cache gets gradually smaller. It is probably an error for this
* to be negative.
*/
int cache_drain_resistance; int cache_drain_resistance;
} ccobject; } ccobject;
static int present_in_ring(ccobject *self, CPersistentRing *target); static int present_in_ring(ccobject *self, CPersistentRing *target);
static int check_ring(ccobject *self, const char *context); static int ring_corrupt(ccobject *self, const char *context);
static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v); static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
/* ---------------------------------------------------------------- */ /* ---------------------------------------------------------------- */
...@@ -85,7 +92,10 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context) ...@@ -85,7 +92,10 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context)
PyObject *object; PyObject *object;
object = (PyObject *)(((char *)here) - offsetof(cPersistentObject, ring)); /* given a pointer to a ring slot in a cPersistent_HEAD, we want to get
* the pointer to the Python object that slot is embedded in.
*/
object = (PyObject *)(((void *)here) - offsetof(cPersistentObject, ring));
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
if (!PyExtensionInstance_Check(object)) { if (!PyExtensionInstance_Check(object)) {
...@@ -119,7 +129,6 @@ scan_gc_items(ccobject *self,int target) ...@@ -119,7 +129,6 @@ scan_gc_items(ccobject *self,int target)
{ {
cPersistentObject *object; cPersistentObject *object;
int error; int error;
CPersistentRing placeholder;
CPersistentRing *here = self->ring_home.next; CPersistentRing *here = self->ring_home.next;
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
...@@ -128,19 +137,24 @@ scan_gc_items(ccobject *self,int target) ...@@ -128,19 +137,24 @@ scan_gc_items(ccobject *self,int target)
safety_counter = 10000; safety_counter = 10000;
#endif #endif
/* Scan through the ring until we either find the ring_home (i.e. start
* of the ring, or we've ghosted enough objects to reach the target
* size.
*/
while (1) { while (1) {
if (check_ring(self, "mid-gc")) if (ring_corrupt(self, "mid-gc"))
return -1; return -1;
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
if (!safety_counter--) { if (!safety_counter--) {
/* This loop has been running for a very long time. /* This loop has been running for a very long time. It is
It is possible that someone loaded a very large number of objects, possible that someone loaded a very large number of objects,
and now wants us to blow them all away. However it may and now wants us to blow them all away. However it may also
also indicate a logic error. If the loop has been running this indicate a logic error. If the loop has been running this
long then you really have to doubt it will ever terminate. long then you really have to doubt it will ever terminate.
In the MUCH_RING_CHECKING build we prefer to raise an exception In the MUCH_RING_CHECKING build we prefer to raise an
here */ exception here
*/
PyErr_SetString(PyExc_RuntimeError, PyErr_SetString(PyExc_RuntimeError,
"scan_gc_items safety counter exceeded"); "scan_gc_items safety counter exceeded");
return -1; return -1;
...@@ -172,26 +186,24 @@ scan_gc_items(ccobject *self,int target) ...@@ -172,26 +186,24 @@ scan_gc_items(ccobject *self,int target)
else if (object->state == cPersistent_UPTODATE_STATE) { else if (object->state == cPersistent_UPTODATE_STATE) {
/* deactivate it. This is the main memory saver. */ /* deactivate it. This is the main memory saver. */
ENGINE_NOISE("G"); /* Save the next pointer of the object we're about to ghostify,
* so that we can follow the link after the ghosted object is
* removed from the ring (via ghostify()).
*/
CPersistentRing *next = here->next;
/* add a placeholder */ ENGINE_NOISE("G");
placeholder.next = here->next;
placeholder.prev = here;
here->next->prev = &placeholder;
here->next = &placeholder;
/* In Python, "obj._p_changed = None" spells, ghostify */
error = PyObject_SetAttr((PyObject *)object, py__p_changed, error = PyObject_SetAttr((PyObject *)object, py__p_changed,
Py_None); Py_None);
/* unlink the placeholder */ here = next;
placeholder.next->prev = placeholder.prev;
placeholder.prev->next = placeholder.next;
here = placeholder.next; if (error)
if(error)
return -1; /* problem */ return -1; /* problem */
} else { }
else {
ENGINE_NOISE("."); ENGINE_NOISE(".");
here = here->next; here = here->next;
} }
...@@ -201,12 +213,16 @@ scan_gc_items(ccobject *self,int target) ...@@ -201,12 +213,16 @@ scan_gc_items(ccobject *self,int target)
static PyObject * static PyObject *
lockgc(ccobject *self, int target_size) lockgc(ccobject *self, int target_size)
{ {
/* We think this is thread-safe because of the GIL, and there's nothing
* in between checking the ring_lock and acquiring it that calls back
* into Python.
*/
if (self->ring_lock) { if (self->ring_lock) {
Py_INCREF(Py_None); Py_INCREF(Py_None);
return Py_None; return Py_None;
} }
if (check_ring(self, "pre-gc")) if (ring_corrupt(self, "pre-gc"))
return NULL; return NULL;
ENGINE_NOISE("<"); ENGINE_NOISE("<");
self->ring_lock = 1; self->ring_lock = 1;
...@@ -216,7 +232,7 @@ lockgc(ccobject *self, int target_size) ...@@ -216,7 +232,7 @@ lockgc(ccobject *self, int target_size)
} }
self->ring_lock = 0; self->ring_lock = 0;
ENGINE_NOISE(">\n"); ENGINE_NOISE(">\n");
if (check_ring(self, "post-gc")) if (ring_corrupt(self, "post-gc"))
return NULL; return NULL;
Py_INCREF(Py_None); Py_INCREF(Py_None);
...@@ -409,7 +425,7 @@ cc_lru_items(ccobject *self, PyObject *args) ...@@ -409,7 +425,7 @@ cc_lru_items(ccobject *self, PyObject *args)
return NULL; return NULL;
} }
if (check_ring(self, "pre-cc_items")) if (ring_corrupt(self, "pre-cc_items"))
return NULL; return NULL;
l = PyList_New(0); l = PyList_New(0);
...@@ -566,7 +582,7 @@ cc_getattr(ccobject *self, char *name) ...@@ -566,7 +582,7 @@ cc_getattr(ccobject *self, char *name)
{ {
PyObject *r; PyObject *r;
if (check_ring(self, "getattr")) if (ring_corrupt(self, "getattr"))
return NULL; return NULL;
if(*name=='c') if(*name=='c')
...@@ -641,7 +657,7 @@ cc_subscript(ccobject *self, PyObject *key) ...@@ -641,7 +657,7 @@ cc_subscript(ccobject *self, PyObject *key)
{ {
PyObject *r; PyObject *r;
if (check_ring(self, "__getitem__")) if (ring_corrupt(self, "__getitem__"))
return NULL; return NULL;
r = (PyObject *)object_from_oid(self, key); r = (PyObject *)object_from_oid(self, key);
...@@ -727,7 +743,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v) ...@@ -727,7 +743,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
*/ */
} }
if (check_ring(self, "pre-setitem")) if (ring_corrupt(self, "pre-setitem"))
return -1; return -1;
if (PyDict_SetItem(self->data, key, v)) if (PyDict_SetItem(self->data, key, v))
return -1; return -1;
...@@ -749,7 +765,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v) ...@@ -749,7 +765,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
Py_DECREF(v); Py_DECREF(v);
} }
if (check_ring(self, "post-setitem")) if (ring_corrupt(self, "post-setitem"))
return -1; return -1;
else else
return 0; return 0;
...@@ -762,7 +778,7 @@ cc_del_item(ccobject *self, PyObject *key) ...@@ -762,7 +778,7 @@ cc_del_item(ccobject *self, PyObject *key)
cPersistentObject *p; cPersistentObject *p;
/* unlink this item from the ring */ /* unlink this item from the ring */
if (check_ring(self, "pre-delitem")) if (ring_corrupt(self, "pre-delitem"))
return -1; return -1;
v = (PyObject *)object_from_oid(self, key); v = (PyObject *)object_from_oid(self, key);
...@@ -800,7 +816,7 @@ cc_del_item(ccobject *self, PyObject *key) ...@@ -800,7 +816,7 @@ cc_del_item(ccobject *self, PyObject *key)
return -1; return -1;
} }
if (check_ring(self, "post-delitem")) if (ring_corrupt(self, "post-delitem"))
return -1; return -1;
return 0; return 0;
...@@ -816,7 +832,7 @@ cc_ass_sub(ccobject *self, PyObject *key, PyObject *v) ...@@ -816,7 +832,7 @@ cc_ass_sub(ccobject *self, PyObject *key, PyObject *v)
} }
static int static int
_check_ring(ccobject *self, const char *context) _ring_corrupt(ccobject *self, const char *context)
{ {
CPersistentRing *here = &(self->ring_home); CPersistentRing *here = &(self->ring_home);
int expected = 1 + self->non_ghost_count; int expected = 1 + self->non_ghost_count;
...@@ -861,18 +877,11 @@ _check_ring(ccobject *self, const char *context) ...@@ -861,18 +877,11 @@ _check_ring(ccobject *self, const char *context)
return 0; return 0;
} }
/* XXX This function returns false if everything is okay, which is
backwards given its name. All the tests say:
if (check_ring(...))
return error
*/
static int static int
check_ring(ccobject *self, const char *context) ring_corrupt(ccobject *self, const char *context)
{ {
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
int code = _check_ring(self,context); int code = _ring_corrupt(self,context);
if (code) { if (code) {
PyErr_Format(PyExc_RuntimeError, PyErr_Format(PyExc_RuntimeError,
"broken ring (code %d) in %s, size %d", "broken ring (code %d) in %s, size %d",
......
src/ZODB/cPickleCache.c
View file @ db5a7f5a
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
static char cPickleCache_doc_string[] = static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\n" "Defines the PickleCache used by ZODB Connection objects.\n"
"\n" "\n"
"$Id: cPickleCache.c,v 1.48 2002/04/02 19:47:19 jeremy Exp $\n"; "$Id: cPickleCache.c,v 1.49 2002/04/02 22:22:03 bwarsaw Exp $\n";
#define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;} #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
#define UNLESS(E) if(!(E)) #define UNLESS(E) if(!(E))
...@@ -44,20 +44,27 @@ static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed; ...@@ -44,20 +44,27 @@ static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed;
#define ENGINE_NOISE(A) ((void)A) #define ENGINE_NOISE(A) ((void)A)
#endif #endif
/* the layout of this struct is the same as the start of ccobject_head in cPersistence.c */ /* This object is the pickle cache. The layout of this struct is the same
* as the start of ccobject_head in cPersistence.c
* XXX Why do they need to have the same layouts?
*/
typedef struct { typedef struct {
CACHE_HEAD CACHE_HEAD
int klass_count; int klass_count; /* XXX for ZClass support? */
PyObject *data; PyObject *data; /* oid -> object dict */
PyObject *jar; PyObject *jar; /* Connection object */
PyObject *setklassstate; PyObject *setklassstate; /* ??? */
int cache_size; int cache_size; /* number of items in cache */
int ring_lock; int ring_lock; /* ??? */
/* XXX Settable from Python, this appears to be a ratio controlling how
* the cache gets gradually smaller. It is probably an error for this
* to be negative.
*/
int cache_drain_resistance; int cache_drain_resistance;
} ccobject; } ccobject;
static int present_in_ring(ccobject *self, CPersistentRing *target); static int present_in_ring(ccobject *self, CPersistentRing *target);
static int check_ring(ccobject *self, const char *context); static int ring_corrupt(ccobject *self, const char *context);
static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v); static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
/* ---------------------------------------------------------------- */ /* ---------------------------------------------------------------- */
...@@ -85,7 +92,10 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context) ...@@ -85,7 +92,10 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context)
PyObject *object; PyObject *object;
object = (PyObject *)(((char *)here) - offsetof(cPersistentObject, ring)); /* given a pointer to a ring slot in a cPersistent_HEAD, we want to get
* the pointer to the Python object that slot is embedded in.
*/
object = (PyObject *)(((void *)here) - offsetof(cPersistentObject, ring));
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
if (!PyExtensionInstance_Check(object)) { if (!PyExtensionInstance_Check(object)) {
...@@ -119,7 +129,6 @@ scan_gc_items(ccobject *self,int target) ...@@ -119,7 +129,6 @@ scan_gc_items(ccobject *self,int target)
{ {
cPersistentObject *object; cPersistentObject *object;
int error; int error;
CPersistentRing placeholder;
CPersistentRing *here = self->ring_home.next; CPersistentRing *here = self->ring_home.next;
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
...@@ -128,19 +137,24 @@ scan_gc_items(ccobject *self,int target) ...@@ -128,19 +137,24 @@ scan_gc_items(ccobject *self,int target)
safety_counter = 10000; safety_counter = 10000;
#endif #endif
/* Scan through the ring until we either find the ring_home (i.e. start
* of the ring, or we've ghosted enough objects to reach the target
* size.
*/
while (1) { while (1) {
if (check_ring(self, "mid-gc")) if (ring_corrupt(self, "mid-gc"))
return -1; return -1;
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
if (!safety_counter--) { if (!safety_counter--) {
/* This loop has been running for a very long time. /* This loop has been running for a very long time. It is
It is possible that someone loaded a very large number of objects, possible that someone loaded a very large number of objects,
and now wants us to blow them all away. However it may and now wants us to blow them all away. However it may also
also indicate a logic error. If the loop has been running this indicate a logic error. If the loop has been running this
long then you really have to doubt it will ever terminate. long then you really have to doubt it will ever terminate.
In the MUCH_RING_CHECKING build we prefer to raise an exception In the MUCH_RING_CHECKING build we prefer to raise an
here */ exception here
*/
PyErr_SetString(PyExc_RuntimeError, PyErr_SetString(PyExc_RuntimeError,
"scan_gc_items safety counter exceeded"); "scan_gc_items safety counter exceeded");
return -1; return -1;
...@@ -172,26 +186,24 @@ scan_gc_items(ccobject *self,int target) ...@@ -172,26 +186,24 @@ scan_gc_items(ccobject *self,int target)
else if (object->state == cPersistent_UPTODATE_STATE) { else if (object->state == cPersistent_UPTODATE_STATE) {
/* deactivate it. This is the main memory saver. */ /* deactivate it. This is the main memory saver. */
ENGINE_NOISE("G"); /* Save the next pointer of the object we're about to ghostify,
* so that we can follow the link after the ghosted object is
* removed from the ring (via ghostify()).
*/
CPersistentRing *next = here->next;
/* add a placeholder */ ENGINE_NOISE("G");
placeholder.next = here->next;
placeholder.prev = here;
here->next->prev = &placeholder;
here->next = &placeholder;
/* In Python, "obj._p_changed = None" spells, ghostify */
error = PyObject_SetAttr((PyObject *)object, py__p_changed, error = PyObject_SetAttr((PyObject *)object, py__p_changed,
Py_None); Py_None);
/* unlink the placeholder */ here = next;
placeholder.next->prev = placeholder.prev;
placeholder.prev->next = placeholder.next;
here = placeholder.next; if (error)
if(error)
return -1; /* problem */ return -1; /* problem */
} else { }
else {
ENGINE_NOISE("."); ENGINE_NOISE(".");
here = here->next; here = here->next;
} }
...@@ -201,12 +213,16 @@ scan_gc_items(ccobject *self,int target) ...@@ -201,12 +213,16 @@ scan_gc_items(ccobject *self,int target)
static PyObject * static PyObject *
lockgc(ccobject *self, int target_size) lockgc(ccobject *self, int target_size)
{ {
/* We think this is thread-safe because of the GIL, and there's nothing
* in between checking the ring_lock and acquiring it that calls back
* into Python.
*/
if (self->ring_lock) { if (self->ring_lock) {
Py_INCREF(Py_None); Py_INCREF(Py_None);
return Py_None; return Py_None;
} }
if (check_ring(self, "pre-gc")) if (ring_corrupt(self, "pre-gc"))
return NULL; return NULL;
ENGINE_NOISE("<"); ENGINE_NOISE("<");
self->ring_lock = 1; self->ring_lock = 1;
...@@ -216,7 +232,7 @@ lockgc(ccobject *self, int target_size) ...@@ -216,7 +232,7 @@ lockgc(ccobject *self, int target_size)
} }
self->ring_lock = 0; self->ring_lock = 0;
ENGINE_NOISE(">\n"); ENGINE_NOISE(">\n");
if (check_ring(self, "post-gc")) if (ring_corrupt(self, "post-gc"))
return NULL; return NULL;
Py_INCREF(Py_None); Py_INCREF(Py_None);
...@@ -409,7 +425,7 @@ cc_lru_items(ccobject *self, PyObject *args) ...@@ -409,7 +425,7 @@ cc_lru_items(ccobject *self, PyObject *args)
return NULL; return NULL;
} }
if (check_ring(self, "pre-cc_items")) if (ring_corrupt(self, "pre-cc_items"))
return NULL; return NULL;
l = PyList_New(0); l = PyList_New(0);
...@@ -566,7 +582,7 @@ cc_getattr(ccobject *self, char *name) ...@@ -566,7 +582,7 @@ cc_getattr(ccobject *self, char *name)
{ {
PyObject *r; PyObject *r;
if (check_ring(self, "getattr")) if (ring_corrupt(self, "getattr"))
return NULL; return NULL;
if(*name=='c') if(*name=='c')
...@@ -641,7 +657,7 @@ cc_subscript(ccobject *self, PyObject *key) ...@@ -641,7 +657,7 @@ cc_subscript(ccobject *self, PyObject *key)
{ {
PyObject *r; PyObject *r;
if (check_ring(self, "__getitem__")) if (ring_corrupt(self, "__getitem__"))
return NULL; return NULL;
r = (PyObject *)object_from_oid(self, key); r = (PyObject *)object_from_oid(self, key);
...@@ -727,7 +743,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v) ...@@ -727,7 +743,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
*/ */
} }
if (check_ring(self, "pre-setitem")) if (ring_corrupt(self, "pre-setitem"))
return -1; return -1;
if (PyDict_SetItem(self->data, key, v)) if (PyDict_SetItem(self->data, key, v))
return -1; return -1;
...@@ -749,7 +765,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v) ...@@ -749,7 +765,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
Py_DECREF(v); Py_DECREF(v);
} }
if (check_ring(self, "post-setitem")) if (ring_corrupt(self, "post-setitem"))
return -1; return -1;
else else
return 0; return 0;
...@@ -762,7 +778,7 @@ cc_del_item(ccobject *self, PyObject *key) ...@@ -762,7 +778,7 @@ cc_del_item(ccobject *self, PyObject *key)
cPersistentObject *p; cPersistentObject *p;
/* unlink this item from the ring */ /* unlink this item from the ring */
if (check_ring(self, "pre-delitem")) if (ring_corrupt(self, "pre-delitem"))
return -1; return -1;
v = (PyObject *)object_from_oid(self, key); v = (PyObject *)object_from_oid(self, key);
...@@ -800,7 +816,7 @@ cc_del_item(ccobject *self, PyObject *key) ...@@ -800,7 +816,7 @@ cc_del_item(ccobject *self, PyObject *key)
return -1; return -1;
} }
if (check_ring(self, "post-delitem")) if (ring_corrupt(self, "post-delitem"))
return -1; return -1;
return 0; return 0;
...@@ -816,7 +832,7 @@ cc_ass_sub(ccobject *self, PyObject *key, PyObject *v) ...@@ -816,7 +832,7 @@ cc_ass_sub(ccobject *self, PyObject *key, PyObject *v)
} }
static int static int
_check_ring(ccobject *self, const char *context) _ring_corrupt(ccobject *self, const char *context)
{ {
CPersistentRing *here = &(self->ring_home); CPersistentRing *here = &(self->ring_home);
int expected = 1 + self->non_ghost_count; int expected = 1 + self->non_ghost_count;
...@@ -861,18 +877,11 @@ _check_ring(ccobject *self, const char *context) ...@@ -861,18 +877,11 @@ _check_ring(ccobject *self, const char *context)
return 0; return 0;
} }
/* XXX This function returns false if everything is okay, which is
backwards given its name. All the tests say:
if (check_ring(...))
return error
*/
static int static int
check_ring(ccobject *self, const char *context) ring_corrupt(ccobject *self, const char *context)
{ {
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
int code = _check_ring(self,context); int code = _ring_corrupt(self,context);
if (code) { if (code) {
PyErr_Format(PyExc_RuntimeError, PyErr_Format(PyExc_RuntimeError,
"broken ring (code %d) in %s, size %d", "broken ring (code %d) in %s, size %d",
......
src/persistent/cPickleCache.c
View file @ db5a7f5a
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
static char cPickleCache_doc_string[] = static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\n" "Defines the PickleCache used by ZODB Connection objects.\n"
"\n" "\n"
"$Id: cPickleCache.c,v 1.48 2002/04/02 19:47:19 jeremy Exp $\n"; "$Id: cPickleCache.c,v 1.49 2002/04/02 22:22:03 bwarsaw Exp $\n";
#define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;} #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
#define UNLESS(E) if(!(E)) #define UNLESS(E) if(!(E))
...@@ -44,20 +44,27 @@ static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed; ...@@ -44,20 +44,27 @@ static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed;
#define ENGINE_NOISE(A) ((void)A) #define ENGINE_NOISE(A) ((void)A)
#endif #endif
/* the layout of this struct is the same as the start of ccobject_head in cPersistence.c */ /* This object is the pickle cache. The layout of this struct is the same
* as the start of ccobject_head in cPersistence.c
* XXX Why do they need to have the same layouts?
*/
typedef struct { typedef struct {
CACHE_HEAD CACHE_HEAD
int klass_count; int klass_count; /* XXX for ZClass support? */
PyObject *data; PyObject *data; /* oid -> object dict */
PyObject *jar; PyObject *jar; /* Connection object */
PyObject *setklassstate; PyObject *setklassstate; /* ??? */
int cache_size; int cache_size; /* number of items in cache */
int ring_lock; int ring_lock; /* ??? */
/* XXX Settable from Python, this appears to be a ratio controlling how
* the cache gets gradually smaller. It is probably an error for this
* to be negative.
*/
int cache_drain_resistance; int cache_drain_resistance;
} ccobject; } ccobject;
static int present_in_ring(ccobject *self, CPersistentRing *target); static int present_in_ring(ccobject *self, CPersistentRing *target);
static int check_ring(ccobject *self, const char *context); static int ring_corrupt(ccobject *self, const char *context);
static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v); static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
/* ---------------------------------------------------------------- */ /* ---------------------------------------------------------------- */
...@@ -85,7 +92,10 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context) ...@@ -85,7 +92,10 @@ object_from_ring(ccobject *self, CPersistentRing *here, const char *context)
PyObject *object; PyObject *object;
object = (PyObject *)(((char *)here) - offsetof(cPersistentObject, ring)); /* given a pointer to a ring slot in a cPersistent_HEAD, we want to get
* the pointer to the Python object that slot is embedded in.
*/
object = (PyObject *)(((void *)here) - offsetof(cPersistentObject, ring));
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
if (!PyExtensionInstance_Check(object)) { if (!PyExtensionInstance_Check(object)) {
...@@ -119,7 +129,6 @@ scan_gc_items(ccobject *self,int target) ...@@ -119,7 +129,6 @@ scan_gc_items(ccobject *self,int target)
{ {
cPersistentObject *object; cPersistentObject *object;
int error; int error;
CPersistentRing placeholder;
CPersistentRing *here = self->ring_home.next; CPersistentRing *here = self->ring_home.next;
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
...@@ -128,19 +137,24 @@ scan_gc_items(ccobject *self,int target) ...@@ -128,19 +137,24 @@ scan_gc_items(ccobject *self,int target)
safety_counter = 10000; safety_counter = 10000;
#endif #endif
/* Scan through the ring until we either find the ring_home (i.e. start
* of the ring, or we've ghosted enough objects to reach the target
* size.
*/
while (1) { while (1) {
if (check_ring(self, "mid-gc")) if (ring_corrupt(self, "mid-gc"))
return -1; return -1;
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
if (!safety_counter--) { if (!safety_counter--) {
/* This loop has been running for a very long time. /* This loop has been running for a very long time. It is
It is possible that someone loaded a very large number of objects, possible that someone loaded a very large number of objects,
and now wants us to blow them all away. However it may and now wants us to blow them all away. However it may also
also indicate a logic error. If the loop has been running this indicate a logic error. If the loop has been running this
long then you really have to doubt it will ever terminate. long then you really have to doubt it will ever terminate.
In the MUCH_RING_CHECKING build we prefer to raise an exception In the MUCH_RING_CHECKING build we prefer to raise an
here */ exception here
*/
PyErr_SetString(PyExc_RuntimeError, PyErr_SetString(PyExc_RuntimeError,
"scan_gc_items safety counter exceeded"); "scan_gc_items safety counter exceeded");
return -1; return -1;
...@@ -172,26 +186,24 @@ scan_gc_items(ccobject *self,int target) ...@@ -172,26 +186,24 @@ scan_gc_items(ccobject *self,int target)
else if (object->state == cPersistent_UPTODATE_STATE) { else if (object->state == cPersistent_UPTODATE_STATE) {
/* deactivate it. This is the main memory saver. */ /* deactivate it. This is the main memory saver. */
ENGINE_NOISE("G"); /* Save the next pointer of the object we're about to ghostify,
* so that we can follow the link after the ghosted object is
* removed from the ring (via ghostify()).
*/
CPersistentRing *next = here->next;
/* add a placeholder */ ENGINE_NOISE("G");
placeholder.next = here->next;
placeholder.prev = here;
here->next->prev = &placeholder;
here->next = &placeholder;
/* In Python, "obj._p_changed = None" spells, ghostify */
error = PyObject_SetAttr((PyObject *)object, py__p_changed, error = PyObject_SetAttr((PyObject *)object, py__p_changed,
Py_None); Py_None);
/* unlink the placeholder */ here = next;
placeholder.next->prev = placeholder.prev;
placeholder.prev->next = placeholder.next;
here = placeholder.next; if (error)
if(error)
return -1; /* problem */ return -1; /* problem */
} else { }
else {
ENGINE_NOISE("."); ENGINE_NOISE(".");
here = here->next; here = here->next;
} }
...@@ -201,12 +213,16 @@ scan_gc_items(ccobject *self,int target) ...@@ -201,12 +213,16 @@ scan_gc_items(ccobject *self,int target)
static PyObject * static PyObject *
lockgc(ccobject *self, int target_size) lockgc(ccobject *self, int target_size)
{ {
/* We think this is thread-safe because of the GIL, and there's nothing
* in between checking the ring_lock and acquiring it that calls back
* into Python.
*/
if (self->ring_lock) { if (self->ring_lock) {
Py_INCREF(Py_None); Py_INCREF(Py_None);
return Py_None; return Py_None;
} }
if (check_ring(self, "pre-gc")) if (ring_corrupt(self, "pre-gc"))
return NULL; return NULL;
ENGINE_NOISE("<"); ENGINE_NOISE("<");
self->ring_lock = 1; self->ring_lock = 1;
...@@ -216,7 +232,7 @@ lockgc(ccobject *self, int target_size) ...@@ -216,7 +232,7 @@ lockgc(ccobject *self, int target_size)
} }
self->ring_lock = 0; self->ring_lock = 0;
ENGINE_NOISE(">\n"); ENGINE_NOISE(">\n");
if (check_ring(self, "post-gc")) if (ring_corrupt(self, "post-gc"))
return NULL; return NULL;
Py_INCREF(Py_None); Py_INCREF(Py_None);
...@@ -409,7 +425,7 @@ cc_lru_items(ccobject *self, PyObject *args) ...@@ -409,7 +425,7 @@ cc_lru_items(ccobject *self, PyObject *args)
return NULL; return NULL;
} }
if (check_ring(self, "pre-cc_items")) if (ring_corrupt(self, "pre-cc_items"))
return NULL; return NULL;
l = PyList_New(0); l = PyList_New(0);
...@@ -566,7 +582,7 @@ cc_getattr(ccobject *self, char *name) ...@@ -566,7 +582,7 @@ cc_getattr(ccobject *self, char *name)
{ {
PyObject *r; PyObject *r;
if (check_ring(self, "getattr")) if (ring_corrupt(self, "getattr"))
return NULL; return NULL;
if(*name=='c') if(*name=='c')
...@@ -641,7 +657,7 @@ cc_subscript(ccobject *self, PyObject *key) ...@@ -641,7 +657,7 @@ cc_subscript(ccobject *self, PyObject *key)
{ {
PyObject *r; PyObject *r;
if (check_ring(self, "__getitem__")) if (ring_corrupt(self, "__getitem__"))
return NULL; return NULL;
r = (PyObject *)object_from_oid(self, key); r = (PyObject *)object_from_oid(self, key);
...@@ -727,7 +743,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v) ...@@ -727,7 +743,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
*/ */
} }
if (check_ring(self, "pre-setitem")) if (ring_corrupt(self, "pre-setitem"))
return -1; return -1;
if (PyDict_SetItem(self->data, key, v)) if (PyDict_SetItem(self->data, key, v))
return -1; return -1;
...@@ -749,7 +765,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v) ...@@ -749,7 +765,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
Py_DECREF(v); Py_DECREF(v);
} }
if (check_ring(self, "post-setitem")) if (ring_corrupt(self, "post-setitem"))
return -1; return -1;
else else
return 0; return 0;
...@@ -762,7 +778,7 @@ cc_del_item(ccobject *self, PyObject *key) ...@@ -762,7 +778,7 @@ cc_del_item(ccobject *self, PyObject *key)
cPersistentObject *p; cPersistentObject *p;
/* unlink this item from the ring */ /* unlink this item from the ring */
if (check_ring(self, "pre-delitem")) if (ring_corrupt(self, "pre-delitem"))
return -1; return -1;
v = (PyObject *)object_from_oid(self, key); v = (PyObject *)object_from_oid(self, key);
...@@ -800,7 +816,7 @@ cc_del_item(ccobject *self, PyObject *key) ...@@ -800,7 +816,7 @@ cc_del_item(ccobject *self, PyObject *key)
return -1; return -1;
} }
if (check_ring(self, "post-delitem")) if (ring_corrupt(self, "post-delitem"))
return -1; return -1;
return 0; return 0;
...@@ -816,7 +832,7 @@ cc_ass_sub(ccobject *self, PyObject *key, PyObject *v) ...@@ -816,7 +832,7 @@ cc_ass_sub(ccobject *self, PyObject *key, PyObject *v)
} }
static int static int
_check_ring(ccobject *self, const char *context) _ring_corrupt(ccobject *self, const char *context)
{ {
CPersistentRing *here = &(self->ring_home); CPersistentRing *here = &(self->ring_home);
int expected = 1 + self->non_ghost_count; int expected = 1 + self->non_ghost_count;
...@@ -861,18 +877,11 @@ _check_ring(ccobject *self, const char *context) ...@@ -861,18 +877,11 @@ _check_ring(ccobject *self, const char *context)
return 0; return 0;
} }
/* XXX This function returns false if everything is okay, which is
backwards given its name. All the tests say:
if (check_ring(...))
return error
*/
static int static int
check_ring(ccobject *self, const char *context) ring_corrupt(ccobject *self, const char *context)
{ {
#ifdef MUCH_RING_CHECKING #ifdef MUCH_RING_CHECKING
int code = _check_ring(self,context); int code = _ring_corrupt(self,context);
if (code) { if (code) {
PyErr_Format(PyExc_RuntimeError, PyErr_Format(PyExc_RuntimeError,
"broken ring (code %d) in %s, size %d", "broken ring (code %d) in %s, size %d",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7