#12601: @cached_method does not work for special methods
-------------------------------------------------+-------------------------
Reporter: saraedum | Owner: jason
Type: enhancement | Status:
Priority: minor | needs_review
Component: misc | Milestone: sage-5.12
Keywords: cached_method, cache, | Resolution:
operator, special method | Merged in:
Authors: Simon King | Reviewers:
Report Upstream: Completely fixed; Fix | Work issues:
reported upstream | Commit:
Branch: | Stopgaps:
Dependencies: #15038 |
-------------------------------------------------+-------------------------
Comment (by SimonKing):
Replying to [comment:11 nbruin]:
> Replying to [comment:10 SimonKing]:
> > PS: One thing I worry about is the startup time. After all, for each
cached method, it is now needed to decide whether the function name starts
and ends with two underscores.
>
> If that's a problem, why not just leave them separate decorators?
''If'' that's a problem. I simply don't know yet whether it is noticeable
in startup-time. If it does not create a slow-down, I think it is more
convenient to have a single decorator that works for both kind of methods.
> Plus, not all `__*__` are special method, are they? Since the list of
special methods is short and discrete, I think you should match on the
whole name. Perhaps the list is available in the python library somewhere.
Right. However, if we are talking about a cached non-special double
underscore method for an instance that allows to assign attributes, then
the distinction between `CachedMethod` and `CachedSpecialMethod` is
irrelevant! Namely, if attribute assignment works, then the `__get__`
method will be called precisely once. There are only three reasons why the
`__get__` method could be called repeatedly:
- Attribute assignment does not work, hence, stuff will be stored in (and
retrieved from) `inst.__cached_methods`.
- We have a special method, so that Python won't look into
`inst.__dict__`.
- The user does `del inst.method_name`.
So, I think it is a non-issue. But of course, we could do a look-up in a
finite list of special methods. I just fear that this would create an even
bigger slow-down (''if'' there is any slow-down noticeable).
> Incidentally, I suspect that this will not work for cdef classes at all,
due to the special way in which special methods get compiled (and the fact
they end up in slots)
Sure. That's why I wrote "(special methods can't be wrapped in Cython, so
I can't wrap `__hash__`)" in comment:9.
In any case, cdef classes do not play totally nicely with cached methods
anyway. They quite often don't allow attribute assignment, and that's why
I introduced `inst.__cached_methods` in the first place, a couple of years
ago. Without it, cached methods on cdef classes would not only be even
slower, but they wouldn't be cached at all!
> By the way, have you done timings on `ArgumentFixer` overhead?
I didn't change argument fixer here. There was some change in #15038,
namely: Postpone creation of the argument fixer. I think it was #8611 and
#11115 when I last worked on speeding up the argument fixer.
> Calling a cached function with arguments has considerable overhead
(easily 4 times as expensive). As a result, instantiating a
`CachedRepresentation` no-op class with `__init__` arguments is much more
expensive than instantiating the corresponding normal no-op class. I think
this can really affect things like creating matrices from a list of
arguments (the default way!) because the parent, the matrix algebra, must
get created.
That's clearly for a different ticket. Actually I have played with the
idea of using an `ArgumentFixer` in `CachedRepresentation.__classcall__`
so that different equivalent ways of providing arguments (by position or
by name) result in the same instance of the class. And then, further
optimisations (e.g., for singleton classes) would easily available.
> `ArgumentFixer` can definitely be improved: several lists there get
intensively rewritten and some methods get looked up where straight
`PyList_...` python library calls could get used. I'm not sure to what
extent that would improve the situation and how much we lose to this
overhead in practice, but it's definitely a possible source:
`ArgumentFixer` is not definitely not without cost and it gets used for
any cached method/function call that has arguments.
Again, that's a different ticket.
--
Ticket URL: <http://trac.sagemath.org/ticket/12601#comment:12>
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 http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
