[Zope-CMF] Re: Making TypesTool faster

[Martin Aspeli]

Tres Seaver wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Alexander Limi wrote:
>> On Wed, 02 May 2007 10:03:39 -0700, Martin Aspeli  
>> <optilude at gmx.net> wrote:
>>
>>> +1, but I don't think the two need to be mutually exclusive.
>> Amen.
>>
>>> At present, Archetypes-based content types cannot be used with a factory  
>>> (I
>>> tried hard, but there are some acquisition-related/factory related  
>>> reasons);
>>> I'd like to refactor this, but we can't for Plone 3.0 at least.
>> Right. Obviously, we'd like to do this "the proper way", but we can't do  
>> that for a while yet.
> 
> I do have a notion which would make this "better, faster, stronger" for
> Plone, which is much more afflicted by this problem than other CMF-based
> sites (because it presents an add menu on each page):  I think the
> bottleneck is actually inside 'ATContentTypes.lib.contraintypes', in the
> 'getDefaultAddableTypes' method of the 'ConstrainTypesMixin' class:  it
> could cache the typeinfo list (or a list of mappings derived from it),
> with the container and the user_id as keys.
> 
> Most of the win would come immediatelely, and at the place most affected
> by the clurrent slowness, leaving us time to figure out why Z3-style
> factories don't work with AT.

I know why... or rather, I did know, and I'm sure if I looked it again I 
could remember. It has to do with the extra work that's being done in 
the auto-generated factory (see ClassGen.py) which calls 
initializeArchetype(). I'd love to get rid of this (for various 
reasons), but I tried to rewrite it as a factory and failed (this is 
where I don't remember why).

>>> Given that Alec & co essentially had a patch which worked, I suggest  
>>> that we
>>> have a look at it, and include it if possible, and then encourage the use
>>> factories in general. I'd hope we could avoid deprecating it outright  
>>> until
>>> we can fix up AT to use them, though.
>> Also note that according to Alec, it would work better as a instance-level  
>> thing than a thread-level one.
> 
> The cache is actually based only on the container (which could be a
> path) and the user ID.  It would be a perfect use of a RAM Cache Manager
>  (assuming that we cached mappings rather than persistent references).
> Using a volatile here (which is what I assume you mean) would dilute the
> "locality" of the cache by the number of database connections in use.
> 
> I've looksd, and all the callers of 'getDefaultAddableTypes' need only
> the ID and / or the title of the FTI, which simplifies things a bunch.
> 
>> Isn't this something like 2 lines of code in TypesTool? :)
> 
> I don't think so.  Getting the tests right is likely to be *lots* of
> lines of code in 'tests/test_TypesTool.py', if we make the change there.

I feel like we're having a very abstract discussions here. Have you 
looked at the patch Alec/Ben/Mike/Rob did? If not, Limi - can you find 
it and paste it here so that we can discuss its merits? I don't recall 
whether it came with tests, but even so it didn't look terribly complicated.

Martin