Re: [Zope-dev] SVN: zope.interface/branches/jinty-mem/src/zope/interface/interface.py Improve CPU performance of previous memory optimization
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 )
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 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
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
-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 )