Commit 9ddbacb0 authored by Kirill Smelkov's avatar Kirill Smelkov Committed by Julien Muchembled

persistent: On deactivate release in-slots objects too

( This is backport of https://github.com/zopefoundation/persistent/pull/44
  to ZODB-3.10 )

On ._p_deactivate() and ._p_invalidate(), when an object goes to ghost
state, objects referenced by all its attributes, except related to
persistence machinery, are released, this way freeing memory (if they
were referenced only from going-to-ghost object).

That's the idea - an object in ghost state is simply a stub, which loads
its content on first access (via hooking into get/set attr) while
occupying minimal memory in not-yet-loaded state.

However the above is not completely true right now, as currently on
ghostification only object's .__dict__ is released, while in-slots objects
are retained attached to ghost object staying in RAM:

    ---- 8< ----
    from ZODB import DB
    from persistent import Persistent
    import gc

    db = DB(None)
    jar = db.open()

    class C:
        def __init__(self, v):
            self.v = v
        def __del__(self):
            print 'released (%s)' % self.v

    class P1(Persistent):
        pass

    class P2(Persistent):
        __slots__ = ('aaa')

    p1 = P1()
    jar.add(p1)
    p1.aaa = C(1)

    p2 = P2()
    jar.add(p2)
    p2.aaa = C(2)

    p1._p_invalidate()
    # "released (1)" is printed

    p2._p_invalidate()
    gc.collect()
    # "released (2)" is NOT printed     <--
    ---- 8< ----

So teach ghostify() & friends to release objects in slots to free-up
memory when an object goes to ghost state.

NOTE PyErr_Occurred() added after ghostify() calls because
pickle_slotnames() can raise an error, but we do not want to change
ghostify() prototype for backward compatibility reason - as it is used
in cPersistenceCAPIstruct.

( I hit this bug with wendelin.core which uses proxies to load
  data from DB to virtual memory manager and then deactivate proxy right
  after load has been completed:

  https://lab.nexedi.com/nexedi/wendelin.core/blob/f7803634/bigfile/file_zodb.py#L239
  https://lab.nexedi.com/nexedi/wendelin.core/blob/f7803634/bigfile/file_zodb.py#L295 )
parent 524c2714
...@@ -29,6 +29,9 @@ Bugs Fixed ...@@ -29,6 +29,9 @@ Bugs Fixed
- Call _p_resolveConflict() even if a conflicting change doesn't change the - Call _p_resolveConflict() even if a conflicting change doesn't change the
state. This reverts to the behaviour of 3.10.3 and older. state. This reverts to the behaviour of 3.10.3 and older.
- Ensure that ``__slots__`` attributes are cleared when a persistent object
is ghostified.
3.10.5 (2011-11-19) 3.10.5 (2011-11-19)
=================== ===================
......
...@@ -75,6 +75,7 @@ fatal_1350(cPersistentObject *self, const char *caller, const char *detail) ...@@ -75,6 +75,7 @@ fatal_1350(cPersistentObject *self, const char *caller, const char *detail)
#endif #endif
static void ghostify(cPersistentObject*); static void ghostify(cPersistentObject*);
static PyObject * pickle_slotnames(PyTypeObject *cls);
/* Load the state of the object, unghostifying it. Upon success, return 1. /* Load the state of the object, unghostifying it. Upon success, return 1.
* If an error occurred, re-ghostify the object and return -1. * If an error occurred, re-ghostify the object and return -1.
...@@ -141,7 +142,7 @@ accessed(cPersistentObject *self) ...@@ -141,7 +142,7 @@ accessed(cPersistentObject *self)
static void static void
ghostify(cPersistentObject *self) ghostify(cPersistentObject *self)
{ {
PyObject **dictptr; PyObject **dictptr, *slotnames;
/* are we already a ghost? */ /* are we already a ghost? */
if (self->state == cPersistent_GHOST_STATE) if (self->state == cPersistent_GHOST_STATE)
...@@ -171,6 +172,8 @@ ghostify(cPersistentObject *self) ...@@ -171,6 +172,8 @@ ghostify(cPersistentObject *self)
_estimated_size_in_bytes(self->estimated_size); _estimated_size_in_bytes(self->estimated_size);
ring_del(&self->ring); ring_del(&self->ring);
self->state = cPersistent_GHOST_STATE; self->state = cPersistent_GHOST_STATE;
/* clear __dict__ */
dictptr = _PyObject_GetDictPtr((PyObject *)self); dictptr = _PyObject_GetDictPtr((PyObject *)self);
if (dictptr && *dictptr) if (dictptr && *dictptr)
{ {
...@@ -178,6 +181,38 @@ ghostify(cPersistentObject *self) ...@@ -178,6 +181,38 @@ ghostify(cPersistentObject *self)
*dictptr = NULL; *dictptr = NULL;
} }
/* clear all slots besides _p_* */
slotnames = pickle_slotnames(Py_TYPE(self));
if (slotnames && slotnames != Py_None)
{
int i;
for (i = 0; i < PyList_GET_SIZE(slotnames); i++)
{
PyObject *name;
char *cname;
int is_special;
name = PyList_GET_ITEM(slotnames, i);
if (PyBytes_Check(name))
{
cname = PyBytes_AS_STRING(name);
is_special = !strncmp(cname, "_p_", 3);
if (is_special) /* skip persistent */
{
continue;
}
}
/* NOTE: this skips our delattr hook */
if (PyObject_GenericSetAttr((PyObject *)self, name, NULL) < 0)
/* delattr of non-set slot will raise AttributeError - we
* simply ignore. */
PyErr_Clear();
}
}
Py_XDECREF(slotnames);
/* We remove the reference to the just ghosted object that the ring /* We remove the reference to the just ghosted object that the ring
* holds. Note that the dictionary of oids->objects has an uncounted * holds. Note that the dictionary of oids->objects has an uncounted
* reference, so if the ring's reference was the only one, this frees * reference, so if the ring's reference was the only one, this frees
...@@ -261,6 +296,8 @@ Per__p_deactivate(cPersistentObject *self) ...@@ -261,6 +296,8 @@ Per__p_deactivate(cPersistentObject *self)
called directly. Methods that override this need to called directly. Methods that override this need to
do the same! */ do the same! */
ghostify(self); ghostify(self);
if (PyErr_Occurred())
return NULL;
} }
Py_INCREF(Py_None); Py_INCREF(Py_None);
...@@ -289,6 +326,8 @@ Per__p_invalidate(cPersistentObject *self) ...@@ -289,6 +326,8 @@ Per__p_invalidate(cPersistentObject *self)
if (Per_set_changed(self, NULL) < 0) if (Per_set_changed(self, NULL) < 0)
return NULL; return NULL;
ghostify(self); ghostify(self);
if (PyErr_Occurred())
return NULL;
} }
Py_INCREF(Py_None); Py_INCREF(Py_None);
return Py_None; return Py_None;
......
...@@ -180,6 +180,30 @@ class PersistenceTest(unittest.TestCase): ...@@ -180,6 +180,30 @@ class PersistenceTest(unittest.TestCase):
self.assertEqual(obj._p_changed, None) self.assertEqual(obj._p_changed, None)
self.assertEqual(obj._p_state, GHOST) self.assertEqual(obj._p_state, GHOST)
def test__p_invalidate_from_changed_w_slots(self):
from persistent import Persistent
class Derived(Persistent):
__slots__ = ('myattr1', 'myattr2')
def __init__(self):
self.myattr1 = 'value1'
self.myattr2 = 'value2'
obj = Derived()
jar = self._makeJar()
jar.add(obj)
obj._p_activate()
obj._p_changed = True
jar._loaded = []
jar._registered = []
self.assertEqual(Derived.myattr1.__get__(obj), 'value1')
self.assertEqual(Derived.myattr2.__get__(obj), 'value2')
obj._p_invalidate()
self.assertIs(obj._p_changed, None)
self.assertEqual(list(jar._loaded), [])
self.assertRaises(AttributeError, lambda: Derived.myattr1.__get__(obj))
self.assertRaises(AttributeError, lambda: Derived.myattr2.__get__(obj))
self.assertEqual(list(jar._loaded), [])
self.assertEqual(list(jar._registered), [])
def test_initial_serial(self): def test_initial_serial(self):
NOSERIAL = "\000" * 8 NOSERIAL = "\000" * 8
obj = self._makeOne() obj = self._makeOne()
......
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