We recently added estimated_size. We originally added it as a new

unsigned long field after a signed char state field and a 3-character
reserved field.  This didn't work because there are packages in the
wild that have their own copies of cPersistence.h that didn't see the
update.

To get around this, we used the reserved space by making
estimated_size a 24-bit bit field in the space occupied by the old
3-character reserved field.  To fit in 24 bits, we made the units of
estimated_size 64-character blocks.  This allows is to handle up to a
GB.  We should never see that, but to be paranoid, we also truncate
sizes greater than 1GB.  We also set the minimum size to 64 bytes.

src/BTrees/BTreeModuleTemplate.c

@@ -323,7 +323,22 @@ PreviousBucket(Bucket **current, Bucket *first)
         trailing = first;
 	PER_USE_OR_RETURN(first, -1);
         first = first->next;
-	PER_UNUSE(trailing);
+
+
+
+
+
+
+
+    ((trailing)->state==cPersistent_STICKY_STATE
+     &&
+     ((trailing)->state=cPersistent_UPTODATE_STATE));
+
+    PER_ACCESSED(trailing);
+
+
+
+
 
 	if (first == *current) {
 	    *current = trailing;

src/persistent/cPersistence.c

@@ -89,7 +89,8 @@ unghostify(cPersistentObject *self)
         if (self->cache) {
             /* Create a node in the ring for this unghostified object. */
             self->cache->non_ghost_count++;
-	    self->cache->total_estimated_size += self->estimated_size;
+	    self->cache->total_estimated_size += 
+              _estimated_size_in_bytes(self->estimated_size);
 	    ring_add(&self->cache->ring_home, &self->ring);
 	    Py_INCREF(self);
         }
@@ -145,7 +146,8 @@ unlink_from_ring(cPersistentObject *self)
     /* if we're ghostifying an object, we better have some non-ghosts */
     assert(self->cache->non_ghost_count > 0);
     self->cache->non_ghost_count--;
-    self->cache->total_estimated_size -= self->estimated_size;
+    self->cache->total_estimated_size -=
+      _estimated_size_in_bytes(self->estimated_size);
     ring_del(&self->ring);
 }
 
@@ -176,7 +178,8 @@ ghostify(cPersistentObject *self)
     /* If we're ghostifying an object, we better have some non-ghosts. */
     assert(self->cache->non_ghost_count > 0);
     self->cache->non_ghost_count--;
-    self->cache->total_estimated_size -= self->estimated_size;
+    self->cache->total_estimated_size -= 
+      _estimated_size_in_bytes(self->estimated_size);
     ring_del(&self->ring);
     self->state = cPersistent_GHOST_STATE;
     dictptr = _PyObject_GetDictPtr((PyObject *)self);
@@ -1017,29 +1020,30 @@ Per_get_state(cPersistentObject *self)
 static PyObject *
 Per_get_estimated_size(cPersistentObject *self)
 {
-  return PyInt_FromLong(self->estimated_size);
+  return PyInt_FromLong(_estimated_size_in_bytes(self->estimated_size));
 }
 
 static int
 Per_set_estimated_size(cPersistentObject *self, PyObject *v)
 {
-    if (v) {
-        if (PyInt_Check(v)) {
-	    if (PyInt_AS_LONG(v) < 0) {
-	        PyErr_SetString(PyExc_ValueError,
-			        "_p_estimated_size must not be negative");
-	        return -1;
-	    }
-	    self->estimated_size = PyInt_AS_LONG(v);
-	}
-	else {
-	    PyErr_SetString(PyExc_ValueError,
-			    "_p_estimated_size must be an integer");
-	    return -1;
-	}
-    } else
-        self->estimated_size = 0;
-    return 0;
+  if (v) {
+    if (PyInt_Check(v)) {
+      long lv = PyInt_AS_LONG(v);
+      if (lv < 0) {
+        PyErr_SetString(PyExc_ValueError,
+                        "_p_estimated_size must not be negative");
+        return -1;
+      }
+      self->estimated_size = _estimated_size_in_24_bits(lv);
+    }
+    else {
+      PyErr_SetString(PyExc_ValueError,
+                      "_p_estimated_size must be an integer");
+      return -1;
+    }
+  } else
+    self->estimated_size = 0;
+  return 0;
 }
 
 static PyGetSetDef Per_getsets[] = {

src/persistent/cPersistence.h

@@ -24,7 +24,7 @@
     PyObject_HEAD \
     CPersistentRing ring_home; \
     int non_ghost_count; \
-    PY_LONG_LONG total_estimated_size;   /* total estimated size of items in cache */
+    PY_LONG_LONG total_estimated_size;
 
 struct ccobject_head_struct;
 
@@ -60,9 +60,28 @@ typedef struct ccobject_head_struct PerCache;
     PerCache *cache; \
     CPersistentRing ring; \
     char serial[8]; \
-    signed char state; \
-    unsigned char reserved[3]; \
-    unsigned long estimated_size;
+    signed state:8;                              \
+    unsigned estimated_size:24;
+
+/* We recently added estimated_size.  We originally added it as a new
+   unsigned long field after a signed char state field and a
+   3-character reserved field.  This didn't work because there
+   are packages in the wild that have their own copies of cPersistence.h
+   that didn't see the update.  
+
+   To get around this, we used the reserved space by making
+   estimated_size a 24-bit bit field in the space occupied by the old
+   3-character reserved field.  To fit in 24 bits, we made the units
+   of estimated_size 64-character blocks.  This allows is to handle up
+   to a GB.  We should never see that, but to be paranoid, we also
+   truncate sizes greater than 1GB.  We also set the minimum size to
+   64 bytes.
+
+   We use the _estimated_size_in_24_bits and _estimated_size_in_bytes
+   macros both to avoid repetition and to make intent a little clearer.
+*/
+#define _estimated_size_in_24_bits(I) ((I) > 1073741696 ? 16777215 : (I)/64+1)
+#define _estimated_size_in_bytes(I) ((I)*64)
 
 #define cPersistent_GHOST_STATE -1
 #define cPersistent_UPTODATE_STATE 0

src/persistent/cPickleCache.c

@@ -638,27 +638,29 @@ cc_ringlen(ccobject *self)
 static PyObject *
 cc_update_object_size_estimation(ccobject *self, PyObject *args)
 {
-    PyObject *oid;
-    cPersistentObject *v;
-    unsigned int new_size;
-    if (!PyArg_ParseTuple(args, "OI:updateObjectSizeEstimation", &oid, &new_size))
-	return NULL;
-    /* Note: reference borrowed */
-    v = (cPersistentObject *)PyDict_GetItem(self->data, oid);
-    if (v) {
-        /* we know this object -- update our "total_size_estimation"
-           we must only update when the object is in the ring
-	*/
-        if (v->ring.r_next) {
-            self->total_estimated_size += new_size - v->estimated_size;
-	    /* we do this in "Connection" as we need it even when the
-	       object is not in the cache (or not the ring)
-	    */
-	    /* v->estimated_size = new_size; */
-	}
+  PyObject *oid;
+  cPersistentObject *v;
+  unsigned int new_size;
+  if (!PyArg_ParseTuple(args, "OI:updateObjectSizeEstimation", &oid, &new_size))
+    return NULL;
+  /* Note: reference borrowed */
+  v = (cPersistentObject *)PyDict_GetItem(self->data, oid);
+  if (v) {
+    /* we know this object -- update our "total_size_estimation"
+       we must only update when the object is in the ring
+    */
+    if (v->ring.r_next) {
+      self->total_estimated_size += _estimated_size_in_bytes(
+             _estimated_size_in_24_bits(new_size) - v->estimated_size
+             );
+      /* we do this in "Connection" as we need it even when the
+         object is not in the cache (or not the ring)
+      */
+      /* v->estimated_size = new_size; */
     }
-    Py_RETURN_NONE;
- }
+  }
+  Py_RETURN_NONE;
+}
 
 
 static struct PyMethodDef cc_methods[] = {
@@ -1058,7 +1060,8 @@ static PyGetSetDef cc_getsets[] = {
 static PyMemberDef cc_members[] = {
     {"cache_size", T_INT, offsetof(ccobject, cache_size)},
     {"cache_size_bytes", T_LONG, offsetof(ccobject, cache_size_bytes)},
-    {"total_estimated_size", T_LONG, offsetof(ccobject, total_estimated_size), RO},
+    {"total_estimated_size", T_LONG, offsetof(ccobject, total_estimated_size),
+     RO},
     {"cache_drain_resistance", T_INT,
      offsetof(ccobject, cache_drain_resistance)},
     {"cache_non_ghost_count", T_INT, offsetof(ccobject, non_ghost_count), RO},

src/persistent/tests/persistent.txt

@@ -94,7 +94,13 @@ Of course, the estimated size must not be negative.
   0
   >>> p._p_estimated_size = 1000
   >>> p._p_estimated_size
-  1000
+  1024
+
+Huh?  Why is the estimated size coming out different than what we put
+in? The reason is that the size isn't stored exactly.  For backward
+compatibility reasons, the size needs to fit in 24 bits, so,
+internally, it is adjusted somewhat.
+
   >>> p._p_estimated_size = -1
   Traceback (most recent call last):
   ....