[MERGE] Use slots consistently in InventoryEntry hierarchy -- regenerated against new head. Seeking another reviewer.

[Jan Hudec]

On Wed, May 31, 2006 at 09:15:28 -0400, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Jan Hudec wrote:
> > And
> > quite many operations would have to look at it anyway. So this leads me
> > to thinking that perhaps I should unify the core and non-core attributes
> > and use methods like you suggested for everything, not just the
> > proplist.
> 
> This sounds like it would be slower than using slots, and possibly
> slower than normal variable access, so please benchmark if you make that
> kind of change.

I have done a bit of profiling just to see whether it's best to have a class
attribute where the corresponding instance attribute does not make sense, use
try/catch, use condition on the type etc. And these are the results on my
machine:

  call_overhead:  slot=0.343  dict=0.340  none=0.340
  call_overhead:  slot=0.341  dict=0.340  none=0.339
  use_try_catch:  slot=0.481  dict=0.582  none=5.319
    use_getattr:  slot=0.702  dict=0.809  none=1.392
  use_classattr:  slot=0.422  dict=0.460  none=0.414
   use_property:  slot=0.438  dict=0.437  none=0.864
         use_if:  slot=0.648  dict=0.828  none=0.581
    use_hasattr:  slot=0.844  dict=1.010  none=1.395

The slot, dict and none are results for accessing an attribute on following
class instances respectively:

    class cNone(object):
	kind = 'bad'
	classattr = None
	propattr = property(lambda _: None)
	__slots__ = []

    class cSlot(cNone):
	kind = 'good'
	__slots__ = ['classattr', 'notattr', 'propattr']
	def __init__(self):
	    self.classattr = True
	    self.notattr = True
	    self.propattr = True

    class cDict(cNone):
	kind = 'good'
	propattr = None
	def __init__(self):
	    self.classattr = True
	    self.notattr = True
	    self.propattr = True


The call_overhead times calling just:
    def call_overhead(i):
	return None
so we should subtract it from the access time. And than the functions are
respectively:

    def use_try_catch(i):
	try:
	    return i.notattr
	except AttributeError:
	    return None

You can see this is Ok if the attribute exists, but just plain appaling when
it does not.

    def use_getattr(i):
	return getattr(i, 'notattr', None)

The turns out to be quite dissatisfactory. That surprises me a little bit,
but I guess that it can be explained by the need to lookup getattr by name
and the fact the attribute name is not interned.

    def use_classattr(i):
	return i.classattr

The classattr is a classattr in the base class. As you can see, it's the
fastest in all cases.

    def use_property(i):
	return i.propattr

The call to lambda self: None seems to really hurt performance, so I'm going
to avoid this now and in future. Especially since when the instance does not
have __dict__, class attribute makes a good read-only attribute on the
instance.

    def use_if(i):
	if i.kind == 'good':
	    return i.notattr
	else:
	    return None

This does two attribute accesses, so it is inevitably slower. Ok, I admit it
was a bad idea.

    def use_hasattr(i):
	if hasattr(i, 'notattr'):
	    return i.notattr
	else:
	    return None

Although the documentation says it calls getattr and catches the exception,
it is quite significantly faster than try/catch in the bad case. I guess
given the explicit way python handles exceptions on the C level, they are
a lot easier to catch there.

Also notice, that the slots actually do a lot less difference, than the
access method used.

And that's all, folks. So I'll add class attributes into InventoryEntry for
all attributes that only apply to some of it's descendants.

I attach the measurement script as a whole if you are interested. Notice,
that there is one more measurement running for the each set, that is not
printed. That is because I found that the first run tens to give higher
results, sometimes by more than 1/3!

-- 
						 Jan 'Bulb' Hudec <bulb at ucw.cz>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ,time-access.py
Type: text/x-python
Size: 1990 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060601/c8a4efc6/attachment.py 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060601/c8a4efc6/attachment.pgp