#20686: Refactor getattr_from_other_class() for lookup of methods in categories
-------------------------------------+-------------------------------------
Reporter: nthiery | Owner:
Type: enhancement | Status: needs_work
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:
-------------------------------------+-------------------------------------
Changes (by embray):
* commit: 8a4f48d047a06ef9fcb2a0e01b5198f7ef439f9b =>
7140ee389ac63921f356d5abc4cdc94a6bf48580
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://u3351942.ct.sendgrid.net/wf/click?upn=aTs-2BwUSKwq20U-2FVxpZle9avE-2F22ScxT2g945hnZa-2Bd7NL7-2F6-2FLkuo-2B7kqyyL9WQ-2BflpBhAeERnAiA6fWpy4e3Q-3D-3D_gXX0YPkjCa6kfMda2NWALp0MQ-2FOvmULrxPdhd2nGLCZmcKY4a-2BIhFTvpT0CPyjp0jWY3yT0c7u8vxR2iodY8CFi3jlNHRi3-2FXWjm5CoeOjgWoS-2BDBFumOGZxSKCc53TMGORG2vZQts9Z1YZF1btJ2-2BlSLtlrlDkXBTyAOoFVnA2PA4kVXrUvTDRNKnPjufPu4YjqgHymnqUJ79ulzEEN-2BeATQZ5jwLxcwH9d0gGRe5M-3D
>
> Test
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. 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://u3351942.ct.sendgrid.net/wf/click?upn=aTs-2BwUSKwq20U-2FVxpZle9avE-2F22ScxT2g945hnZa-2Bd7NL7-2F6-2FLkuo-2B7kqyyL9WQ-2BflpBhAeERnAiA6fWpy4e3Q-3D-3D_gXX0YPkjCa6kfMda2NWALp0MQ-2FOvmULrxPdhd2nGLCZmcKY4a-2BIhFTvpT0CPyjp0jWY3yT0c7u8vxR2iodY8CA1dSnHdkziWNKXp6LiZCAqHTPlvTz26lKeiA7V8rBQ3B91vmeCwE1-2FM6JzygR2nCEmXhoXOUz2Ir9ZN-2FWO-2BxjqHu8YpkwfcPnuSFNEXj1YHsXPGPFRBOg7Xe1f-2Bx1j-2FXHMLFJ1hoou64Ki3hfZyqys-3D
--
Comment:
New commits:
||[https://u3351942.ct.sendgrid.net/wf/click?upn=aTs-2BwUSKwq20U-2FVxpZle9X5Q5HkyL4BAcZIINAQIANwXv2zxXdpqI2sgTMRb-2BsFd4055yXvUU0GbywcvoO6S2zUNcl16UWMqk3ZnUKSmCDAY3CSPmQj6VlPQMvwNZjjK_gXX0YPkjCa6kfMda2NWALp0MQ-2FOvmULrxPdhd2nGLCZmcKY4a-2BIhFTvpT0CPyjp0jWY3yT0c7u8vxR2iodY8CEBdUAFUTE1YKqDJpcMw0qtjGWgyIRrGk9gGSNotdjzcpeNWjl-2F-2FnDB-2F-2ByML7EQ1kdQoavItqxAmoQEFp4k7M1gT9guaAjEOjEN2zG29sFyly0T3bZoOD40oFRMw7C7sRMunZo-2FSgwCPYsmEc37L5Cc-3D
88d9757]||{{{Comment that getattr_from_other_class is cached}}}||
||[https://u3351942.ct.sendgrid.net/wf/click?upn=aTs-2BwUSKwq20U-2FVxpZle9X5Q5HkyL4BAcZIINAQIANwXv2zxXdpqI2sgTMRb-2BsFd8h5Kyit0x-2Bu8TemCO5VbWyJZgy11uhVs3DFSDG8plFnvlPYunkIV4JyMv2sCpJjl_gXX0YPkjCa6kfMda2NWALp0MQ-2FOvmULrxPdhd2nGLCZmcKY4a-2BIhFTvpT0CPyjp0jWY3yT0c7u8vxR2iodY8CH74oUYCKujQ9RnMr0eB32v-2F0-2F-2Bhi-2FuMyAxl23jYyNtlZvIXvIp9sOFa8vDxKTtNCOGTsgXmEONfjJnApuYeU0X1TQT-2FJ2yprhAcyOizuaI-2F9ls9iUbKQhz-2FC7oTkZr6mC0y2YhIGAfa3AhM8q6OG1Y-3D
7140ee3]||{{{Prefer absolute imports}}}||
--
Ticket URL:
<https://u3351942.ct.sendgrid.net/wf/click?upn=aTs-2BwUSKwq20U-2FVxpZle9V7rZPHNFdCZn9IqCcBPbg5D9KijS1M0T4BLYvm9PfIs-2Bk8cs-2FfrNE1Or-2Bex7P5i3w-3D-3D_gXX0YPkjCa6kfMda2NWALp0MQ-2FOvmULrxPdhd2nGLCZmcKY4a-2BIhFTvpT0CPyjp0jWY3yT0c7u8vxR2iodY8CHJnOtA-2BG3Yk7fAfiUuc2tjvKMZfdMX5Dyg-2BIXp3v-2BsbZeMYJUM9Khe1EmZ5yusmJro9CruDwxqAmM4ERsdHKXq3gdawsM8p71Z9eWxIBwZ-2F966gTq79pCz02FvK0Tqaa5KHeQlIWv-2B602H3-2B-2FZGefQ-3D>
Sage
<https://u3351942.ct.sendgrid.net/wf/click?upn=jm4cvpnHFskDUI5PLE4HCGcqpDNkng8vhBVTwprYF6Q-3D_gXX0YPkjCa6kfMda2NWALp0MQ-2FOvmULrxPdhd2nGLCZmcKY4a-2BIhFTvpT0CPyjp0jWY3yT0c7u8vxR2iodY8CCqmv-2BhRDBf8qDw5-2FBQVg9QxplXuPV2scrW-2FC-2Fewvaz98FiAj9eF9kiN6n8RiNn9dr4goMGjqKCS0Q0lQg3yKyuJIp1JNwE7GdlU-2BHM7AzU-2FZ5viFh7S-2BZIwcUraKDXMqsw4LN6DMFBFpiIxrRYgH78-3D>
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.
