Re: [Zope-dev] SVN: zope.interface/branches/jinty-mem/src/zope/interface/interface.py Improve CPU performance of previous memory optimization
-----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 tseaver@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-----
On 9 November 2010 18:35, Tres Seaver <tseaver@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. 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
-----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@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@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-----
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
participants (3)
-
Brian Sutherland -
Laurence Rowe -
Tres Seaver