On Tue, Aug 17, 2010 at 7:34 PM, Carl Witty <[email protected]> wrote: > On Tue, Aug 17, 2010 at 6:40 PM, Robert Bradshaw > <[email protected]> wrote: >> On Tue, Aug 17, 2010 at 6:25 PM, Carl Witty <[email protected]> wrote: >> >>> I'm really quite sure that the performance bug was due to a >>> Cython-created __getattr__ wrapper. >>> >>> In detail: when you create a non-cdef subclass of a cdef class with >>> __getattr__, Python has to decide what to stick in the tp_getattro >>> slot. It notices that there's a __getattr__ defined (that's not a >>> Python-created wrapper object), and so it puts in a generic function >>> that looks up __getattribute__ and __getattr__ in the type dictionary. >>> >>> I guess copying __getattribute__ to __getattr__ would slightly change >>> the behavior when somebody accessed __getattr__ directly. (It >>> wouldn't change the behavior of normal attribute lookup on the >>> object.) >>> >>> With the current cython-devel we could probably fix the Sage >>> __getattr__ performance issue by changing Parent and Element to define >>> __getattribute__ instead of __getattr__. (Maybe this is what you were >>> saying?) >> >> No, defining __getattribute__ is much worse than defining __getattr__ >> because then it needs to call __getattribute__ as an opaque Python >> function before invoking the standard Python lookup mechanisms. >> >> Really, I just need to read some code and do some benchmarks. > > In Python, __getattribute__ is much worse than __getattr__. But in > Cython, __getattribute__ and __getattr__ both set the tp_getattro > slot; so you can replace a __getattr__ with a __getattribute__ that's > just about as efficient. > > In either case, we must avoid having Python call __getattribute__ or > __getattr__ as an opaque Python function; that's the source of the > performance problem. It's vital that Python calls the tp_getattro > slot directly. The way to make that happen is to make sure that any > __getattribute__ or __getattr__ definitions are Python-generated > wrappers (or, at least, that the wrapper's Py_TYPE is > PyWrapperDescr_Type).
Yep. I confirmed that just setting creating __getattr__ does incur the performance loss, even if __getattribute__ was good. It looks like there's no way to have a __getattr__ python-visible attribute without a performance penalty. Given that this only impacts cdef classes, what contracts are we making about the availability of unbound special methods? For example, we're OK with not exposing __long__. I'm leaning towards a cdef class being as fast as possible, while a normal class is 100% python semantics. Perhaps we should control this with a flag. For Sage I know we don't care about having __getattr__ visible as much as the speed penalty for every single attribute lookup of derived classes. - Robert _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
