Hi folks, While messing with Zope2's webdav implementation, I ran across this bit of memory-management code: dflag = hasattr(ob, '_p_changed') and (ob._p_changed == None) ... stuff ... if dflag: ob._p_deactivate() I actually think this should be: dflag = not getattr(ob, '_p_changed', None) ... stuff ... if dflag: ob._p_deactivate() .. because when _p_changed on a persistent object (as I read it in the persistence interface file) is None, the object is a ghost. The object will never be a ghost here because non-_p_ attributes are looked up on it before we check for _p_changed, and even if it was a ghost, deactivating a ghost (to turn it into a ghost) is just not useful. I think the only time we don't want to deactivate it is if it has been changed (when _p_changed is True). Or at least that seems to be the intent. Does this sound right? I have a feeling the answer will be crickets, but I figure what the heck. - C
After a little further digging, I've found that code of the same ilk exists in Zope 2's OFS.FindSupport, App.DavLockManager, Products.ZCatalog.ZCatalog and Products.ZCatalog.CatalogAwareness. Variants on the theme exist in CMFCore.CMFCatalogAware as well as CMFCore.WorkflowTool. As far as I can tell, wherever memory management through deactivation is employed in Zope 2 and CMF, it's done in a way that seems to have no useful effect, at least if my theory is right. - C On Jan 10, 2008, at 8:58 PM, Chris McDonough wrote:
Hi folks,
While messing with Zope2's webdav implementation, I ran across this bit of memory-management code:
dflag = hasattr(ob, '_p_changed') and (ob._p_changed == None) ... stuff ... if dflag: ob._p_deactivate()
I actually think this should be:
dflag = not getattr(ob, '_p_changed', None) ... stuff ... if dflag: ob._p_deactivate()
.. because when _p_changed on a persistent object (as I read it in the persistence interface file) is None, the object is a ghost. The object will never be a ghost here because non-_p_ attributes are looked up on it before we check for _p_changed, and even if it was a ghost, deactivating a ghost (to turn it into a ghost) is just not useful. I think the only time we don't want to deactivate it is if it has been changed (when _p_changed is True). Or at least that seems to be the intent.
Does this sound right? I have a feeling the answer will be crickets, but I figure what the heck.
- C
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists -http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
On Jan 10, 2008, at 8:58 PM, Chris McDonough wrote:
Hi folks,
Your note today pointing at code I wrote made me pay closer attention. :)
While messing with Zope2's webdav implementation, I ran across this bit of memory-management code:
dflag = hasattr(ob, '_p_changed') and (ob._p_changed == None) ... stuff ... if dflag: ob._p_deactivate()
I actually think this should be:
dflag = not getattr(ob, '_p_changed', None) ... stuff ... if dflag: ob._p_deactivate()
.. because when _p_changed on a persistent object (as I read it in the persistence interface file) is None, the object is a ghost. The object will never be a ghost here because non-_p_ attributes are looked up on it before we check for _p_changed, and even if it was a ghost, deactivating a ghost (to turn it into a ghost) is just not useful. I think the only time we don't want to deactivate it is if it has been changed (when _p_changed is True). Or at least that seems to be the intent.
Does this sound right?
No. The important thing to note above is that the check is made *before* ".. stuff ..". The idea is that we want to free any objects we loaded while doing whatever we are doing. We check if an object is a ghost before doing something and then, if it was a ghost before, we try to make it a ghost when we're done. We try to leave the object as we found it. With your change, we'd be ghostifying objects that were presumably useful, because they were in the cache, and we'd be failing to ghostify objects that we resurrected just for the operation.
I have a feeling the answer will be crickets, but I figure what the heck.
:) Jim -- Jim Fulton Zope Corporation
On Jan 11, 2008, at 9:24 AM, Jim Fulton wrote:
While messing with Zope2's webdav implementation, I ran across this bit of memory-management code:
dflag = hasattr(ob, '_p_changed') and (ob._p_changed == None) ... stuff ... if dflag: ob._p_deactivate()
I actually think this should be:
dflag = not getattr(ob, '_p_changed', None) ... stuff ... if dflag: ob._p_deactivate()
.. because when _p_changed on a persistent object (as I read it in the persistence interface file) is None, the object is a ghost. The object will never be a ghost here because non-_p_ attributes are looked up on it before we check for _p_changed, and even if it was a ghost, deactivating a ghost (to turn it into a ghost) is just not useful. I think the only time we don't want to deactivate it is if it has been changed (when _p_changed is True). Or at least that seems to be the intent.
Does this sound right?
No.
The important thing to note above is that the check is made *before* ".. stuff ..". The idea is that we want to free any objects we loaded while doing whatever we are doing. We check if an object is a ghost before doing something and then, if it was a ghost before, we try to make it a ghost when we're done. We try to leave the object as we found it. With your change, we'd be ghostifying objects that were presumably useful, because they were in the cache, and we'd be failing to ghostify objects that we resurrected just for the operation.
I see. The earliest place I've seen this code is almost ten years old, it's amazing you remember. The intent was misunderstood over the years, because code like this got into various places: for ob in obj.listDAVObjects(): if hasattr(ob,"meta_type"): if ob.meta_type=="Broken Because Product is Gone": continue dflag=hasattr(ob, '_p_changed') and (ob._p_changed == None) the (ob._p_changed == None) will never be true here as we will have woken it up by looking for 'meta_type'. This is what got me confused initially.
With your change, we'd be ghostifying objects that were presumably useful, because they were in the cache, and we'd be failing to ghostify objects that we resurrected just for the operation.
The former is true, the latter no. Objects that are resurrected just for the operation would be ghostified too. Only objects that were _p_changed = True would not be. But it doesn't matter, I just needed to understand the intent, and now I do. I don't intend to check my competing example code in, I'll just fix the code around the places where the pattern was misunderstood. Related.... but fuzzier... is it your expectation that the amount of memory used for a database walk routine that tries to do memory management via some combination of connection.cacheMinimize-or- cacheGC() every-n-iterations (no calls to individual objects' _p_deactivate) should be close to one which uses _p_deactivate aggressively against the objects being walked? In my experience, no combination of the "aggregate" cache management calls seems to work nearly as well as aggressively _p_deactivate-ing walked objects directly while walking a large object tree (at least under ZODB 3.6). It seems like doing cacheMinimize, etc just doesn't really have much effect on real memory usage during the walk when it's the only thing used. It's a difficult thing to test, as you need a truly huge database to finaly see the failure mode (which is that you run out of RAM ;-), but that's my experience anyway. - C
On Jan 11, 2008, at 1:08 PM, Chris McDonough wrote: ...
Related.... but fuzzier... is it your expectation that the amount of memory used for a database walk routine that tries to do memory management via some combination of connection.cacheMinimize-or- cacheGC() every-n-iterations (no calls to individual objects' _p_deactivate) should be close to one which uses _p_deactivate aggressively against the objects being walked? In my experience, no combination of the "aggregate" cache management calls seems to work nearly as well as aggressively _p_deactivate-ing walked objects directly while walking a large object tree (at least under ZODB 3.6). It seems like doing cacheMinimize, etc just doesn't really have much effect on real memory usage during the walk when it's the only thing used. It's a difficult thing to test, as you need a truly huge database to finaly see the failure mode (which is that you run out of RAM ;-), but that's my experience anyway.
Python isn't good at returning memory to the OS, so you really can't free it. Calling _p_deactivate along the way prevents it from growing much in the first place. BTW, in situations in which you don't know which objects to deactivate, an alternative is to call cacheGC on the connection frequently. This is fairly inexpensive and incremental. Jim -- Jim Fulton Zope Corporation
Jim Fulton wrote at 2008-1-11 18:20 -0500:
... BTW, in situations in which you don't know which objects to deactivate, an alternative is to call cacheGC on the connection frequently. This is fairly inexpensive and incremental.
But, it can kill volatile variables as well -- which may lead to trouble, until we have implemented something like "Garanteed lifetime for volatile variables". Current potential trouble includes: * inconsistent transactions in relational databases (as "_v_" variables are used to hold the database connection) * broken Archetypes copy/move code (as "_v_" variables are used to communicate with 'manage_afterAdd/beforeDelete further down the tree) -- Dieter
On Jan 13, 2008, at 7:49 AM, Dieter Maurer wrote:
Jim Fulton wrote at 2008-1-11 18:20 -0500:
... BTW, in situations in which you don't know which objects to deactivate, an alternative is to call cacheGC on the connection frequently. This is fairly inexpensive and incremental.
But, it can kill volatile variables as well -- which may lead to trouble, until we have implemented something like "Garanteed lifetime for volatile variables".
Current potential trouble includes:
* inconsistent transactions in relational databases (as "_v_" variables are used to hold the database connection)
This is an interesting case.
* broken Archetypes copy/move code (as "_v_" variables are used to communicate with 'manage_afterAdd/beforeDelete further down the tree)
Perhaps volatile variables are a bad idea. They enable many subtle errors. OTOH, I fear that without them, people would resort to other tricks that might be even worse. What sort of lifetime would you consider appropriate for volatile variables? You example above suggests that they should stick around until a transaction boundary (not including savepoints). I wonder if there are other examples that might want something else. Or perhaps people should take the name "volatile" more seriously. Jim -- Jim Fulton Zope Corporation
On Jan 13, 2008, at 10:40 AM, Jim Fulton wrote:
On Jan 13, 2008, at 7:49 AM, Dieter Maurer wrote:
Jim Fulton wrote at 2008-1-11 18:20 -0500:
... BTW, in situations in which you don't know which objects to deactivate, an alternative is to call cacheGC on the connection frequently. This is fairly inexpensive and incremental.
But, it can kill volatile variables as well -- which may lead to trouble, until we have implemented something like "Garanteed lifetime for volatile variables".
I've got no problem with the current behavior of volatiles, I understand how they work. But personally I've not been able to successfully write walk code against very large databases that doesn't consume RAM without bound using only cacheGC. If I write the same type of code using _p_deactivate against each object I walk, I can keep memory under some sort of control. - C
Jim Fulton wrote at 2008-1-13 10:40 -0500:
On Jan 13, 2008, at 7:49 AM, Dieter Maurer wrote:
Jim Fulton wrote at 2008-1-11 18:20 -0500:
... BTW, in situations in which you don't know which objects to deactivate, an alternative is to call cacheGC on the connection frequently. This is fairly inexpensive and incremental.
But, it can kill volatile variables as well -- which may lead to trouble, until we have implemented something like "Garanteed lifetime for volatile variables".
Current potential trouble includes:
* inconsistent transactions in relational databases (as "_v_" variables are used to hold the database connection)
This is an interesting case.
* broken Archetypes copy/move code (as "_v_" variables are used to communicate with 'manage_afterAdd/beforeDelete further down the tree)
Perhaps volatile variables are a bad idea. They enable many subtle errors. OTOH, I fear that without them, people would resort to other tricks that might be even worse.
What sort of lifetime would you consider appropriate for volatile variables? You example above suggests that they should stick around until a transaction boundary (not including savepoints).
You know, there is a proposal from me about "Garanteed lifetime for volatile variables" in the ZODB wiki for months -- and we (in our private copy) use the corresponding implementation for years (and this implementation was also committed to the Zope bugtracker for years).
I wonder if there are other examples that might want something else.
I think that 2 lifetimes are sufficient: * can disappear at any time * lives at least until the (full) transaction boundary
Or perhaps people should take the name "volatile" more seriously.
Then the legitimate use cases for attributes that are not arbitrarily volatile (detailed in my proposal) will need a different solution -- *AND* at least the Zope code for database adapters must use this new solution. -- Dieter
On Jan 14, 2008, at 1:21 PM, Dieter Maurer wrote: ...
You know, there is a proposal from me about "Garanteed lifetime for volatile variables" in the ZODB wiki for months -- and we (in our private copy) use the corresponding implementation for years (and this implementation was also committed to the Zope bugtracker for years).
You know, you could have provided a link. Jim -- Jim Fulton Zope Corporation
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Jim Fulton wrote:
On Jan 14, 2008, at 1:21 PM, Dieter Maurer wrote: ...
You know, there is a proposal from me about "Garanteed lifetime for volatile variables" in the ZODB wiki for months -- and we (in our private copy) use the corresponding implementation for years (and this implementation was also committed to the Zope bugtracker for years).
You know, you could have provided a link.
Googling: http://www.google.com/search?q=site%3Awiki.zope.org+volatile+lifetime brings it up pretty easily: http://wiki.zope.org/ZODB/VolatileAttributeLifetimeGarantee Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFHi7G6+gerLs4ltQ4RApAnAKC9itSCgQ3oR0mzr4vpH9D9TkGgKwCeMiI2 9eAdmrlMSVseldLtoaA0Gjw= =qZ+2 -----END PGP SIGNATURE-----
Jim Fulton wrote:
You know, you could have provided a link.
I believe it's Dieter's policy never to actually provide links to anything ;-) Chris -- Simplistix - Content Management, Zope & Python Consulting - http://www.simplistix.co.uk
Chris Withers wrote at 2008-1-15 09:42 +0000:
You know, you could have provided a link.
I believe it's Dieter's policy never to actually provide links to anything ;-)
Indeed, I never search for others... If I have the link at hand, you may get a link, but if not, then your effort to search is as big as mine. -- Dieter
Dieter Maurer schrieb:
Chris Withers wrote at 2008-1-15 09:42 +0000:
You know, you could have provided a link. I believe it's Dieter's policy never to actually provide links to anything ;-)
Indeed, I never search for others...
If I have the link at hand, you may get a link, but if not, then your effort to search is as big as mine.
That doesn't from the perspective of group economics, though. -- gocept gmbh & co. kg - forsterstrasse 29 - 06112 halle (saale) - germany www.gocept.com - ct@gocept.com - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development
participants (6)
-
Chris McDonough -
Chris Withers -
Christian Theune -
Dieter Maurer -
Jim Fulton -
Tres Seaver