#11342: Make getattr faster on parents and elements
---------------------------+------------------------------------------------
Reporter: SimonKing | Owner: tbd
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.7.1
Component: performance | Keywords: getattr parent element
Work_issues: | Upstream: N/A
Reviewer: | Author: Simon King
Merged: | Dependencies: #9944
---------------------------+------------------------------------------------
Old description:
> If an attribute of a parent or element can not be found by inspecting the
> method resolution order, `__getattr__` is invoked. It tries to obtain the
> attribute from the parent class or the element class of the category of
> the parent. If it can not be found, the attribute error is raised using a
> Cython function `sage.structure.parent.raise_attribute_error`.
>
> I see several ways to make both `__getattr__` and
> `sage.structure.parent.raise_attribute_error` a lot faster. Details will
> be given in the comments.
>
> Because of one doctest in `sage.categories.primer`, it is needed that the
> category of polynomial rings is properly initialised. Therefore:
>
> Depends on #9944
New description:
If an attribute of a parent or element can not be found by inspecting the
method resolution order, `__getattr__` is invoked. It tries to obtain the
attribute from the parent class or the element class of the category of
the parent. If it can not be found, the attribute error is raised using a
Cython function `sage.structure.parent.raise_attribute_error`.
I see several ways to make both `__getattr__` and
`sage.structure.parent.raise_attribute_error` a lot faster. Details will
be given in the comments.
Because of one doctest in `sage.categories.primer`, it is needed that the
category of polynomial rings is properly initialised. Therefore:
Depends on #9944
Apply [attachment:trac11342-AttributeErrorMessage.patch]
--
Comment(by SimonKing):
Using an additional idea, I was able to improve the timings even further.
I have already indicated that the process of raising the attribute error
actually is a bottle neck. To be precise: Most time is spent for creating
the nicely formatted error message.
The point is: The typical fate of an `AttributeError` is being caught.
Hence, nobody will ever see the error message, except under very special
circumstances.
Therefore I suggest that the error message will only be created if someone
wants to see it. This is possible by introducing a new class
`sage.structure.parent.AttributeErrorMessage`. Its instances simply store
a class `cls` and an attribute name, and its string representation is the
well-known error message stating that `cls` has no attribute of the given
name. This string representation is not created during initialisation but
only when needed.
Hence, rather than calling `raise_attribute_error(self,name)`, one would
do `raise AttributeError, AttributeErrorMessage(self,name)`. That is '''a
lot''' faster:
{{{
sage: cython("""
....: from sage.structure.parent import AttributeErrorMessage
....: def test(self,name,long m):
....: cdef long i
....: for i from 0<=i<m:
....: try:
....: raise AttributeError, AttributeErrorMessage(self,name)
....: except AttributeError:
....: pass
....: """)
sage: P.<a,b,c> = PolynomialRing(QQ)
sage: %time test(P,'bla',10^7)
CPU times: user 3.10 s, sys: 0.00 s, total: 3.10 s
Wall time: 3.10 s
sage: %time test(QQ,'bla',10^7)
CPU times: user 3.06 s, sys: 0.00 s, total: 3.06 s
Wall time: 3.07 s
}}}
Compare with the timings for `raise_attribute_error` in my previous
comments!
Other than that, I added the specification that the attribute name is
supposed to be a string:
{{{
def __getattr__(self, str name):
}}}
instead of
{{{
def __getattr__(self, name):
}}}
'''__Updated timings__'''
I am repeating the tests from my previous comments.
1. non-existing private attributes
There is a clear improvement, even compared with the previous patch
version.
{{{
sage: a = 1
sage: timeit('try: QQ.__bla\nexcept: pass')
625 loops, best of 3: 10.5 µs per loop
sage: timeit('try: a.__bla\nexcept: pass')
625 loops, best of 3: 8.78 µs per loop
sage: timeit('try: QQ.__bla_\nexcept: pass')
}}}
2. non-existing non-private attributes
There is a clear improvement, even compared with the previous patch
version.
{{{
sage: timeit('try: QQ.__bla_\nexcept: pass')
625 loops, best of 3: 14.1 µs per loop
sage: timeit('try: a.__bla_\nexcept: pass')
625 loops, best of 3: 16.1 µs per loop
sage: timeit('try: QQ.bla\nexcept: pass')
625 loops, best of 3: 13.8 µs per loop
sage: timeit('try: a.bla\nexcept: pass')
625 loops, best of 3: 16.3 µs per loop
}}}
3. existing attributes inherited from the category
{{{
sage: timeit('try: QQ.sum\nexcept: pass',number=10^5)
100000 loops, best of 3: 741 ns per loop
sage: timeit('try: a.cartesian_product\nexcept: pass',number=10^5)
100000 loops, best of 3: 1.67 µs per loop
}}}
'''Note'''
Usually, errors are raised with an error message that is given by a
string. It seems that raising it with an instance of
`AttributeErrorMessage` is fine as well - at least it does not seem to
break existing tests. However, perhaps other people have objections.
Therefore I'm informing sage-devel, and I did not override the old patch
but created a new one.
For the patchbot:
Depends on #9944
Apply [attachent:trac11342-AttributeErrorMessage.patch]
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11342#comment:2>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
