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

[Andrew Bennetts]

Some general comments on Python performance... mostly trivia, but perhaps of
interest. :)

On Thu, Jun 01, 2006 at 09:01:37PM +0200, Jan Hudec wrote:
[...]
> 
>     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.

The string is in fact interned.  CPython interns string literals of valid
identifiers.

If you made getattr a local, either using the make_constants decorator from
http://aspn.activestate.com/ASPN/Cookbook/Python/Recipe/277940 or just defining
use_getattr as "def use_getattr(i, getattr=getattr): ...", you'd save the look
up of the getattr function by name.  That would save two dictionary lookups; one
miss in the module globals dict, and then the hit in the builtins dict.  Or at
least that's the theory; for some reason doing this gives slightly worse results
for me.  I'm not sure why; I think it may be because one-argument functions are
special-cased, and this trick breaks that.

The other obvious cost here is the function call.  Although getattr is
implemented in C, calling it still requires building a tuple of the args.  (Some
C functions that always take exactly one argument don't have this cost, but
mostly they do).

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

classattr isn't a class attribute, though!  Well, it is for cNone objects, but
it's an *instance* attribute for cSlot and cDict objects.

(perhaps that's intentionally what you're testing, but your benchmark doesn't
make that clear...)

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

Python function calls are *really* slow compared to basically any other
operation in python, due to the overhead of setting up and tearing down frames.
(One of the nice things about generators is that iterating them doesn't have
this cost; the frame is already set up, so re-entering the generator function is
very fast).

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

Yeah, but trading speed for correctness isn't such a great idea ;)

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

I believe this should be:

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

Note that I'd expect this to be slightly slower at accessing classattr than:

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

Because generally attributes are first looked for on the instance, then on each
class in the MRO in turn, until it is found, or all classes are exhausted.

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

And again, this shouldn't do "self.classattr = True" in __init__, as that makes
an instance attribute, completely masking the class attribute with the same
name.

-Andrew.