[Zope-dev] SVN: zope.interface/branches/jinty-mem/src/zope/interface/interface.py Improve CPU performance of previous memory optimization

[Tres Seaver]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/09/2010 02:47 PM, Laurence Rowe wrote:
> On 9 November 2010 18:35, Tres Seaver <tseaver at palladion.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 11/09/2010 08:26 AM, Wichert Akkerman wrote:
>>> On 11/9/10 14:22 , Brian Sutherland wrote:
>>>> Log message for revision 118295:
>>>>    Improve CPU performance of previous memory optimization
>>>>
>>>> Changed:
>>>>    U   zope.interface/branches/jinty-mem/src/zope/interface/interface.py
>>>>
>>>> -=-
>>>> Modified: zope.interface/branches/jinty-mem/src/zope/interface/interface.py
>>>> ===================================================================
>>>> --- zope.interface/branches/jinty-mem/src/zope/interface/interface.py        2010-11-09 08:31:37 UTC (rev 118294)
>>>> +++ zope.interface/branches/jinty-mem/src/zope/interface/interface.py        2010-11-09 13:22:27 UTC (rev 118295)
>>>> @@ -51,6 +51,7 @@
>>>>       # infrastructure in place.
>>>>       #
>>>>       #implements(IElement)
>>>> +    __tagged_values = None
>>>>
>>>>       def __init__(self, __name__, __doc__=''):
>>>>           """Create an 'attribute' description
>>>> @@ -72,22 +73,27 @@
>>>>
>>>>       def getTaggedValue(self, tag):
>>>>           """ Returns the value associated with 'tag'. """
>>>> -        return getattr(self, '_Element__tagged_values', {})[tag]
>>>> +        if self.__tagged_values is None:
>>>> +            return default
>>>> +        return self.__tagged_values[tag]
>>>
>>> You can even optimise this further:
>>>
>>>        tv = self.__tagged_values
>>>        if tv is None:
>>>            return default
>>>        return tv[tv]
>>>
>>> that avoids a second attribute lookup. You may also want to benchmark
>>> that versus using a __tagged_values={} on the class and doing a simple
>>> return self.__tagged_values.get(tag, default_
>>
>> - -1:  mutable class defaults are a bug magnet.
> 
> None is immutable so I don't think that is a problem in this case.

Wiggy's later suggestion was to use '{}' as a class-level default, which
is what my -1 was for.

> I think the is a possible threading issue with Element.setTaggedValue
> and Specification.subscribe - if two threads called the method
> concurrently, then one of the values might be lost. I think the
> correct way to do it would be:
> 
>     tv = self.__tagged_values
>     if tv is None:
>         tv = self.__dict__.setdefault('_Element__tagged_values', {})
>     tv[tag] = value
> 
> This does bring the name mangling back though.

I'm pretty sure we can safely neglect threading issues here:  no sane
code will call 'setTaggedValue' except at import time, when we should be
serialized by Python's own import lock.


Tres.
- -- 
===================================================================
Tres Seaver          +1 540-429-0999          tseaver at palladion.com
Palladion Software   "Excellence by Design"    http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkzZqIUACgkQ+gerLs4ltQ6nBwCfe7QuTKam33YV7gxsLkO8ere/
OC4AoLEwXHNNRKxdbArD25p9ycMX1QdJ
=4MsJ
-----END PGP SIGNATURE-----