#20686: Refactor getattr_from_other_class() for lookup of methods in categories
-------------------------------------+-------------------------------------
Reporter: nthiery | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-7.3
Component: categories | Resolution:
Keywords: | Merged in:
Authors: Jeroen Demeyer | Reviewers:
Report Upstream: Fixed upstream, | Work issues:
but not in a stable release. |
Branch: | Commit:
u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes|
7140ee389ac63921f356d5abc4cdc94a6bf48580
Dependencies: #20825 | Stopgaps:
-------------------------------------+-------------------------------------
Description changed by jdemeyer:
Old description:
> For parents and elements implemented in Cython, category lookup is
> emulated in `__getattr__` using `getattr_from_other_class`. In this
> ticket, this is refactored a bit:
>
> 1. We should not pick up special attributes from `type`, there is no
> point in returning the type name of the category here (similarly for
> `__dict__`, `__mro__`, ...):
> {{{
> sage: ZZ(1).__name__
> 'JoinCategory.element_class'
> sage: ZZ.__name__
> 'JoinCategory.parent_class'
> }}}
> This change causes a few failures which need to be fixed.
>
> 2. The implementation of `getattr_from_other_class` is not very robust.
> For example, static methods are not supported. We fix this by using low-
> level Python functions to get the attribute and we manually call the
> descriptor `__get__` if needed.
>
> 3. The lookup using `getattr_from_other_class` is about 9 times slower
> than a normal method lookup::
> {{{
> sage: def foo(self): return
> sage: Sets().element_class.foo = foo
> sage: def g(x):
> ....: for i in range(1000):
> ....: x.foo()
>
> sage: x = Semigroups().example().an_element()
> sage: y = 1
>
> sage: %timeit g(x)
> 10000 loops, best of 3: 115 µs per loop
>
> sage: %timeit g(y)
> 1000 loops, best of 3: 1.18 ms per loop
> }}}
> We improve on this by roughly a factor 2 (but even then, it's still a lot
> slower than usual method lookup).
>
> NOTE: This needs a trivial Cython patch (accepted by upstream):
> https://github.com/cython/cython/pull/522
New description:
For parents and elements implemented in Cython, category lookup is
emulated in `__getattr__` using `getattr_from_other_class`. In this
ticket, this is refactored a bit:
1. We should not pick up special attributes from `type`, there is no point
in returning the type name of the category here (similarly for `__dict__`,
`__mro__`, ...):
{{{
sage: ZZ(1).__name__
'JoinCategory.element_class'
sage: ZZ.__name__
'JoinCategory.parent_class'
}}}
This change causes a few failures which need to be fixed.
2. The implementation of `getattr_from_other_class` is not very robust.
For example, static methods are not supported. We fix this by using low-
level Python functions to get the attribute and we manually call the
descriptor `__get__` if needed.
3. We shouldn't do anything special with double-underscore private
attributes: in plain Python, this is implemented by the parser and not by
`getattr()`. So `__getattr__` would already receive the mangled private
name.
4. Some of the changes broke `_sage_src_lines_`, so we also change that:
currently, `_sage_src_lines_()` is used both to get the source lines of a
class (e.g. dynamic classes) and an instance (e.g. cached functions). We
change this to always mean the source lines of an instance, which makes
things clearer.
5. The lookup using `getattr_from_other_class` is about 9 times slower
than a normal method lookup::
{{{
sage: def foo(self): return
sage: Sets().element_class.foo = foo
sage: def g(x):
....: for i in range(1000):
....: x.foo()
sage: x = Semigroups().example().an_element()
sage: y = 1
sage: %timeit g(x)
10000 loops, best of 3: 115 µs per loop
sage: %timeit g(y)
1000 loops, best of 3: 1.18 ms per loop
}}}
We improve on this by roughly a factor 2 (but even then, it's still a lot
slower than usual method lookup).
NOTE: This needs a trivial Cython patch (accepted by upstream):
https://github.com/cython/cython/pull/522
--
--
Ticket URL: <https://trac.sagemath.org/ticket/20686#comment:37>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
