Avoid unnecessary invalidations for invoked fallbacks#45001
Avoid unnecessary invalidations for invoked fallbacks#45001
invoked fallbacks#45001Conversation
While investigating a report in https://discourse.julialang.org/t/package-load-time-regressions-in-v1-8-beta3/78875 it became clear that some precompilation statements were not effective due to invalidation. It turns out that these have the pattern f(arg::SpecificType) = invoke(f, Tuple{GenericType}, arg) When `f` gets compiled for a new subtype of `SpecificType`, the external cached MethodInstance gets invalidated because of the reliance on a less specific method. This is a hack that works around this issue for `copyto!`, but being able to mark calls as having been issued from an `invoke` would be a much better solution.
vtjnash
left a comment
There was a problem hiding this comment.
These worlds are not sortable (they form a complex DAG, not a chain) so this won't be correct. We used to have a world_lowerbound lookup function in this file that we used to validate the lookup. In this case, I think finding a way to tag these as invoke calls would be better.
|
Ping this. |
|
OK, I'm back to thinking about this (though it will be part-time at first). I don't know if I'll make it by julia-1.8.0 but perhaps we can backport it to a later 1.8.x release. @vtjnash, thanks for correcting my mis-impressions. Here's my plan (copied from a slack discussion 😄 with @SimonDanisch), if you disagree feel free to chime in: The solution will be to add another field to a Quite simple in principle, but it may have extensive interacttions with the rest of the compiler. We'll have to store multiple copies of some |
|
Ah, that is a good observation. So it sounds like this is an existing correctness bug, since the stored edge is too narrow. We need to have 2 edges stored and checked here: one from the dispatch signature (which could be represented as a MethodInstance for the invoke signature) and one from the compiled method (which ignores dispatch to it, but tracks changes to all of the code inside). For a regular (non-invoke) call, those 2 edges happen to land on the same method instance signature, so we only needed to record one of them. |
|
It also might fix #45837, if I understand this correctly |
|
Interesting. xref #45051 |
|
@vtjnash, the approach you recommended has been implemented in #46010. Would love a review if you have time. |
While investigating a report in https://discourse.julialang.org/t/package-load-time-regressions-in-v1-8-beta3/78875
it became clear that some precompilation statements were not effective
due to invalidation. It turns out that these have the pattern
When
fgets compiled for a new subtype ofSpecificType, theexternal cached MethodInstance gets invalidated during the check
for validity of new specializations of external methods.
invokeis a specific source of trouble because it can dispatch to a less
specific method, and the test for validity is essentially to check
whether the chosen method is still the one that would be called by
ordinary dispatch (i.e., a test of whether the precompiled method
has been superseded by a more specific one).
This is a hack that works around this issue for
copyto!, exploitingthe fact that if one is calling into a closed module then all of the
methods were available when the precompile was generated and hence
there must have been a reason for the choice. But being
able to mark calls as having been issued from an
invokewouldbe a much better solution. Without that, I don't see a way for a
package-level
invokethat calls aBaseorCore.Compilerfallbackto work properly. (Cthulhu is an example.)