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

2010-11-09 Thread Tres Seaver 
-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.


Tres.
- -- 
===
Tres Seaver  +1 540-429-0999  tsea...@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/

iEYEARECAAYFAkzZlFUACgkQ+gerLs4ltQ5ZYQCfRyDUGofCMiER447yJjBeEduu
E5IAniZu6SbOmYZC0XJt/4WeXOY2u5oD
=cNXP
-END PGP SIGNATURE-

___
Zope-Dev maillist  -  Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 https://mail.zope.org/mailman/listinfo/zope-announce
 https://mail.zope.org/mailman/listinfo/zope )

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

2010-11-09 Thread Laurence Rowe 
On 9 November 2010 18:35, Tres Seaver  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.

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.

Laurence
___
Zope-Dev maillist  -  Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 https://mail.zope.org/mailman/listinfo/zope-announce
 https://mail.zope.org/mailman/listinfo/zope )

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

2010-11-09 Thread 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  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  tsea...@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-

___
Zope-Dev maillist  -  Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 https://mail.zope.org/mailman/listinfo/zope-announce
 https://mail.zope.org/mailman/listinfo/zope )

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

2010-11-11 Thread Brian Sutherland 
On Tue, Nov 09, 2010 at 03:01:09PM -0500, Tres Seaver wrote:
> > 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.

Thanks, I fixed the threading issue in Specification.subscribe. Given
that that part of subscribe is not run very often, I think we can live
with limited name mangling.

> 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.

Great, I quoted you on that ;)

The setdefault fix for the threading issue is not compatible with the
use of __slots__. I couldn't find another way to do it.

-- 
Brian Sutherland
___
Zope-Dev maillist  -  Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 https://mail.zope.org/mailman/listinfo/zope-announce
 https://mail.zope.org/mailman/listinfo/zope )