Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method invalidations due to superseding convert(::Type{Union{}}, x) in Base, related to Base.MappingRF #45837

Closed
fingolfin opened this issue Jun 28, 2022 · 0 comments · Fixed by #46000

Comments

@fingolfin
Copy link
Member

While trying to use SnoopCompile by @timholy to reduce invalidations in packages I work on, there is one invalidation I run into again and again, and in the end gave up trying to address because it just is introduced by too many packages.

The typical cause seems to be that one writes code like this:

struct Foo
  x::Int
end

convert(::Type{T}, x::Foo) where {T <: Integer} = T(x.x)

without realizing that Union{} also matches for T -- see #33780 and #40301 for related issues, and PR #39434 for a related invalidation fix.

Here's an example using Mongoc, but that's just arbitrary, I found many more (e.g. in ChainRulesCore, CxxWrap, Nemo, Singular, ...)

julia> using SnoopCompileCore ; invalidations = @snoopr using Mongoc ;

julia> using SnoopCompile ; trees = invalidation_trees(invalidations)
1-element Vector{SnoopCompile.MethodInvalidations}:
 inserting convert(::Type{T}, t::Union{Mongoc.FindAndModifyFlags, Mongoc.QueryFlags}) where T<:Number in Mongoc at /Users/mhorn/.julia/packages/Mongoc/gXfqP/src/types.jl:62 invalidated:
   backedges: 1: superseding convert(::Type{Union{}}, x) in Base at essentials.jl:282 with MethodInstance for convert(::Core.TypeofBottom, ::Any) (2573 children)
   3 mt_cache

julia> method_invalidations = trees[end] ; root = method_invalidations.backedges[end]
MethodInstance for convert(::Core.TypeofBottom, ::Any) at depth 0 with 2573 children

julia> show(root)
MethodInstance for convert(::Core.TypeofBottom, ::Any) (2573 children)
 MethodInstance for Base.MappingRF(::Any, ::Base.BottomRF) (56 children)
 ⋮
 MethodInstance for Base.MappingRF(::Function, ::Function) (2337 children)
  MethodInstance for Base.mapreduce_empty_iter(::Function, ::Function, ::Vector{Int64}, ::Base.HasEltype) (102 children)
  ⋮
  MethodInstance for Base.mapreduce_empty_iter(::Function, ::Function, ::Vector{Any}, ::Base.HasEltype) (746 children)
   MethodInstance for Base._mapreduce(::typeof(Base.operator_precedence), ::typeof(min), ::IndexLinear, ::Vector{Any}) (745 children)
    MethodInstance for Base._mapreduce_dim(::typeof(Base.operator_precedence), ::typeof(min), ::Base._InitialValue, ::Vector{Any}, ::Colon) (744 children)
     MethodInstance for Base.var"#mapreduce#776"(::Colon, ::Base._InitialValue, ::typeof(mapreduce), ::typeof(Base.operator_precedence), ::typeof(min), ::Vector{Any}) (743 children)
     ⋮
  MethodInstance for Base.mapreduce_empty_iter(::Function, ::Function, ::Vector{UInt16}, ::Base.HasEltype) (1326 children)
   MethodInstance for Base._mapreduce(::typeof(identity), ::typeof(max), ::IndexLinear, ::Vector{UInt16}) (1325 children)
    MethodInstance for Base._mapreduce_dim(::typeof(identity), ::typeof(max), ::Base._InitialValue, ::Vector{UInt16}, ::Colon) (1324 children)
     MethodInstance for Base.var"#mapreduce#776"(::Colon, ::Base._InitialValue, ::typeof(mapreduce), ::typeof(identity), ::typeof(max), ::Vector{UInt16}) (1318 children)
     ⋮
    ⋮
 ⋮
⋮

As it is, I am not even sure how much of an issue this actually is, as I cannot (yet?) quantify how much impact these invalidations really have... I've been trying to squish the many many invalidations that plague the Oscar package (and its dependencies, Singular, Polymake, Hecke, Nemo...) and made some tiny progress but after resolving causes for this invalidation in a ton of places (sometimes with questionable hacks) I gave up for now as whenever I cut off one head of this hydra, two more seem to appear...

vtjnash added a commit that referenced this issue Jul 11, 2022
This should make it impossible to accidentally define or call this
method on foreign types.

Refs: #31602
Fixes: #45837
Closes: #45051
KristofferC pushed a commit that referenced this issue Jul 18, 2022
This should make it impossible to accidentally define or call this
method on foreign types.

Refs: #31602
Fixes: #45837
Closes: #45051
ffucci pushed a commit to ffucci/julia that referenced this issue Aug 11, 2022
This should make it impossible to accidentally define or call this
method on foreign types.

Refs: JuliaLang#31602
Fixes: JuliaLang#45837
Closes: JuliaLang#45051
pcjentsch pushed a commit to pcjentsch/julia that referenced this issue Aug 18, 2022
This should make it impossible to accidentally define or call this
method on foreign types.

Refs: JuliaLang#31602
Fixes: JuliaLang#45837
Closes: JuliaLang#45051
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant