ExtensionClass misses ZODB persistent reference optimization

Bug #143657 reported by Dieter Maurer
2
Affects Status Importance Assigned to Milestone
Zope 2
Fix Released
Medium
Hanno Schlichting

Bug Description

The ZODB implements an important optimization for persistent references:
it can include a class reference in a persistent reference
such that a ghost can be instanciated without loading the object.

In order for this optimization to be possible, the class must
not define "__getnewargs__". Unfortunately, "ExtensionClass.Base"
defines it and therefore prevents this optimization for
ExtensionClass instances (i.e. for all Zope2 objects).

The attached patch removes the "__getnewargs__" definition
from "ExtensionClass.Base", changes the "_reduce" definition
accordingly and updates the tests.

Revision history for this message
Dieter Maurer (d.maurer) wrote :
Revision history for this message
Tres Seaver (tseaver) wrote :

Status: Pending => Accepted

 Supporters added: jim

Assinging to Jim for his evaluation (and to test mail delivery).

Revision history for this message
Jim Fulton (jim-zope) wrote :

Testing mail delivery :)

Revision history for this message
Jim Fulton (jim-zope) wrote :

more testing

Revision history for this message
Tres Seaver (tseaver) wrote :

Test mail delivery again. "This time for sure!"

Revision history for this message
Tres Seaver (tseaver) wrote :

Dieter reports that this patch works around a bug in ZODB which causes subitems of an ExtensionClass-based
folder to be loaded whenever the parent is loaded, just to get the class:

  http://groups.google.com/group/plone-users/browse_thread/thread/64df7bde356360c8/49d9390074f6b58c?lnk=raot#msg_a177c1246bb4e5f1

Revision history for this message
Hanno Schlichting (hannosch) wrote :

I created a branch and applied a modified version of the patch to it at svn+ssh://svn.zope.org/repos/main/ExtensionClass/branches/hannosch-lp-143657

I tried to validate the decrease in reference loading (or rather unghostifying). Using some simple tests I wasn't able to see any difference between this version and the current trunk. So this will need some actual verification and tests before I'd consider it ready to be merged into trunk.

Revision history for this message
Dieter Maurer (d.maurer) wrote :

Note that the optimazation takes effect when persistent references are *written*. Then, the decision is made whether later unghostifying of the persistent container requires loading the object itself (or whether the reference contains already all
necessary information to create a ghost).

This means that you have to write a persistent container with the patch applied in order to be able to see a reduced number
of loading later.

Revision history for this message
Hanno Schlichting (hannosch) wrote :

@dmaurer: yes, I understood that this is an optimization that influences what kind of ZODB reference is written. I started off with a completely new Data.fs using the new code and looked at the various ZMI cache (extreme) details. I tried to do the same TTW actions and compared those against a new Data.fs without the patch. I couldn't spot any actual differences there, so I'll have to do some actual low level tests for this :)

Revision history for this message
Dieter Maurer (d.maurer) wrote :

You should not expect to see differences in the ZODB cache. The functional end result with or without the optimazation
is identical: each persistent reference is transformed into a ghost object. However, with optimized references, the ghost
can be created with information from the reference only while with unoptimized references, a storage load is necessary.

You can verify the effect of the optimazation by calling "ZODB.serialize.ObjectWriter.persistent_id". It will return "oid, cls"
for optimized persistent references and "oid" for unoptimized ones.

Alternatively, you can debug log the ZEO interaction and check the number of ZEO load operations perfomred
(I used an equivalent instrumentation of "ZEO.ServerStub.StorageServer.zeoLoad" for the verification).

Revision history for this message
Hanno Schlichting (hannosch) wrote :

Thanks for the clarification Dieter!

I somehow expected the referenced objects to be unghosted before and staying as ghosts with the optimization. But that's of course nonsense. I'll try to instrument the ZEO load operation as you suggested.

Changed in zope2:
assignee: Jim Fulton (jim-zope) → Hanno Schlichting (hannosch)
Revision history for this message
Hanno Schlichting (hannosch) wrote :

I've been able to verify the effect by instrumenting the FileStorage.load with a simple counter.

For a current Plone 4.0a5 site the differences are in the hundreds of fewer databases loads. Simply loading the front page for the first time requires about 700 instead of 2000 database loads. Adding a page for the first time still required 230 vs. 100 loads. Over time this effect drops off when the ZODB cache is filled up. The effect will be more noticeable in production sites where objects are frequently not in the cache.

Changed in zope2:
status: Confirmed → Fix Committed
Revision history for this message
Dieter Maurer (d.maurer) wrote :

Thank you for working on and finishing this bug report (and others) !

Revision history for this message
Tres Seaver (tseaver) wrote :

ExtensionClass 2.13.0 has been released with this fix:

  http://pypi.python.org/pypi/ExtensionClass

The next release of Zope 2.12 will pick up this change automatically. In the meanwhile, you can safely update the ExtensionClass egg yourself in your site-packages / buildout.

Changed in zope2:
status: Fix Committed → Fix Released
Revision history for this message
Hanno Schlichting (hannosch) wrote :

As a reference, this bug caused http://dev.plone.org/plone/ticket/10318 to appear, which I hopefully fixed in:

https://mail.zope.org/pipermail/checkins/2010-April/044713.html
https://mail.zope.org/pipermail/checkins/2010-April/044715.html

and released in 2.12.3. In short without the __getnewargs__ on Acquisition wrapper classes, you would suddenly get persistent references of the form ("some poid", "Acquisition.ImpliciAcquirerWrapper"). Giving the wrappers a __getnewargs__ ensures that the simple ("some poid") form is used in these cases.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.