Invalidations

[timholy]

I checked this to see how ready this might be to include in ImageCore. Here are the invalidations from this package on Julia 1.6-rc1:

julia> trees
6-element Vector{SnoopCompile.MethodInvalidations}:
 inserting promote_rule(::Type{Bool}, ::Type{var"#s12"} where var"#s12"<:ArrayInterface.StaticBool) in ArrayInterface at /home/tim/.julia/dev/ArrayInterface/src/static.jl:251 invalidated:
   backedges: 1: superseding promote_rule(::Type{Bool}, ::Type{T}) where T<:Number in Base at bool.jl:4 with MethodInstance for promote_rule(::Type{Bool}, ::Type{var"#s444"} where var"#s444"<:Integer) (1 children)
   18 mt_cache

 inserting step(::ArrayInterface.OptionallyStaticUnitRange) in ArrayInterface at /home/tim/.julia/dev/ArrayInterface/src/ranges.jl:102 invalidated:
   backedges: 1: superseding step(r::AbstractUnitRange{T}) where T in Base at range.jl:544 with MethodInstance for step(::AbstractUnitRange{Int64}) (1 children)

 inserting eachindex(r::Union{var"#s12", var"#s9"} where {var"#s12"<:ArrayInterface.OptionallyStaticUnitRange, var"#s9"<:ArrayInterface.OptionallyStaticStepRange}) in ArrayInterface at /home/tim/.julia/dev/ArrayInterface/src/ranges.jl:401 invalidated:
   backedges: 1: superseding eachindex(A::AbstractVector{T} where T) in Base at abstractarray.jl:256 with MethodInstance for eachindex(::AbstractVector{T} where T) (1 children)

 inserting convert(::Type{T}, ::ArrayInterface.StaticInt{N}) where {T<:Number, N} in ArrayInterface at /home/tim/.julia/dev/ArrayInterface/src/static.jl:22 invalidated:
   backedges: 1: superseding convert(::Type{T}, x::Number) where T<:Number in Base at number.jl:7 with MethodInstance for convert(::Type{UInt64}, ::Integer) (2 children)

 inserting step(r::ArrayInterface.OptionallyStaticStepRange) in ArrayInterface at /home/tim/.julia/dev/ArrayInterface/src/ranges.jl:190 invalidated:
   mt_backedges: 1: signature Tuple{typeof(step), OrdinalRange{Int64, Int64}} triggered MethodInstance for (::Base.IteratorsMD.var"#21#23")(::OrdinalRange{Int64, Int64}) (3 children)

 inserting (::Type{T})(x::ArrayInterface.StaticInt{N}) where {T<:Integer, N} in ArrayInterface at /home/tim/.julia/dev/ArrayInterface/src/static.jl:26 invalidated:
   mt_backedges:  1: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for _array_for(::Type{Any}, ::Any, ::Base.HasLength) (0 children)
                  2: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for to_shape(::Integer) (0 children)
                  3: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for findnext(::Test.var"#3#5", ::AbstractString, ::Integer) (0 children)
                  4: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for findnext(::Test.var"#4#6", ::AbstractString, ::Integer) (0 children)
                  5: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for _array_for(::Type{T}, ::Any, ::Base.HasLength) where T (0 children)
                  6: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for _array_for(::Type{String}, ::Any, ::Base.HasLength) (0 children)
                  7: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for _array_for(::Type{Expr}, ::Any, ::Base.HasLength) (0 children)
                  8: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for _similar_for(::Vector{T} where T, ::Type, ::Base.Generator{_A, Test.var"#24#25"{Int64}} where _A, ::Base.HasLength) (0 children)
                  9: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for _similar_for(::Vector{T} where T, ::DataType, ::Base.Generator{_A, Test.var"#24#25"{Int64}} where _A, ::Base.HasLength) (0 children)
                 10: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for findnext(::Pkg.Types.var"#21#22"{String}, ::AbstractString, ::Integer) (0 children)
                 11: signature Tuple{Type{UInt64}, Integer} triggered MethodInstance for convert(::Type{UInt64}, ::Integer) (0 children)
                 12: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for var"#handle_message#2"(::Base.Iterators.Pairs{Symbol, _A, Tuple{Symbol}, _B} where {_A, _B}, ::typeof(Base.CoreLogging.handle_message), ::Base.CoreLogging.SimpleLogger, ::Base.CoreLogging.LogLevel, ::String, ::Nothing, ::Symbol, ::Symbol, ::Nothing, ::Int64) (1 children)
                 13: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for var"#handle_message#2"(::Base.Iterators.Pairs{Symbol, _A, Tuple{Symbol}, _B} where {_A, _B}, ::typeof(Base.CoreLogging.handle_message), ::Logging.ConsoleLogger, ::Base.CoreLogging.LogLevel, ::String, ::Module, ::Symbol, ::Symbol, ::String, ::Int64) (1 children)
                 14: signature Tuple{Type{UInt32}, Integer} triggered MethodInstance for VersionNumber(::Integer, ::Int64, ::Int64, ::Tuple{}, ::Tuple{}) (1 children)
                 15: signature Tuple{Type{Int32}, Integer} triggered MethodInstance for Int32(::Enum{T2} where T2<:Integer) (1 children)
                 16: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for var"#handle_message#2"(::Base.Iterators.Pairs{Symbol, _A, Tuple{Symbol}, _B} where {_A, _B}, ::typeof(Base.CoreLogging.handle_message), ::Base.CoreLogging.SimpleLogger, ::Base.CoreLogging.LogLevel, ::String, ::Module, ::Symbol, ::Symbol, ::String, ::Int64) (2 children)
                 17: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for rpad(::String, ::Integer, ::String) (5 children)
                 18: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for var"#handle_message#2"(::Any, ::typeof(Base.CoreLogging.handle_message), ::Logging.ConsoleLogger, ::Base.CoreLogging.LogLevel, ::Any, ::Any, ::Any, ::Any, ::Any, ::Any) (772 children)

Only the last one is bad, but it's pretty bad. (Among other things, it invalidates Base.require which means the next package to load has to recompile it.) The vulnerability arises in https://github.com/JuliaLang/julia/blob/b1fbe7f135f1b7b0f0c80a6484ea090eef1f1e72/stdlib/Logging/src/ConsoleLogger.jl#L104-L105; since all it knows is that maxlog isa Integer, it's possible that the (::Type{T})(x::ArrayInterface.StaticInt{N}) where {T<:Integer, N} method is applicable and more specific.

The only really good fix I can see is to restrict maxlog support to specific integer types, like the ones Julia ships with. This would be a fix for Base but I am filing this here as a reminder. When working on a package it's worth checking this periodically as it could be a deal-breaker for inclusion in other things.

[Tokazama]

I can't remember if (::Type{T})(x::ArrayInterface.StaticInt{N}) where {T<:Integer, N} is necessary to prevent ambiguities. I'll check on this.

[Tokazama]

It looks like we could instead define Int(::StaticInt) and UInt(::StaticInt). Should we define all of the Integer types for this (Int32, UInt32, etc.)?

EDIT: actually it looks like the invalidations still happen if we just define Int(::StaticInt). The only way I could resolve this is if StaticInt isn't a subtype of Integer.

[timholy]

Yeah, the problem is that it calls Int(::Integer). I'm testing a fix now; it will restrict maxlog to being one of the Core.BuiltinInts, but hopefully folks will be OK with that?

I think the 1.6 backport window may still be sufficiently open to get this in, but we'll see.

[ChrisRackauckas]

Yes, if that improves compilation and doesn't break anything here then 👍 I like it.

[Tokazama]

I guess we could just not subtype Integer, but I assume this is still a problem for other packages with Integer subtypes. There shouldn't be a ton of new Integer subtypes but a know there are a few others out there.

[timholy]

I think you probably want to subtype Integer. You didn't do anything wrong here, except perhaps that in an ideal world we'd all remember to check invalidations in packages we care about when Julia merge windows are about to close.

The logging framework has been wickedly untyped; it's one of the few places where we've had to resort to using invoke just to sever backedges to keep too much stuff from getting invalidated. But handle_message isn't one of the ones we've previously had to protect this way.

Keep an eye on JuliaLang/julia#39689

[Tokazama]

You didn't do anything wrong here, except perhaps that in an ideal world we'd all remember to check invalidations in packages we care about when Julia merge windows are about to close.

Will do. Thank you.

[chriselrod]

EDIT: actually it looks like the invalidations still happen if we just define Int(::StaticInt). The only way I could resolve this is if StaticInt isn't a subtype of Integer.

I think once upon a time we didn't define Int(::StaticInt) for that reason, but it's a convenient method to have, so I'd rather not forgo it.

I could live with something like:

int(x) = Int(x)
int(::StaticInt{N}) = N

if we didn't want to wait until Julia 1.7 for most users to be relieved of the invalidations.

But taking out Int would be a breaking change.

The list of invalidations also shows:

18: signature Tuple{Type{Int64}, Integer} triggered MethodInstance for var"#handle_message#2"(::Any, ::typeof(Base.CoreLogging.handle_message), ::Logging.ConsoleLogger, ::Base.CoreLogging.LogLevel, ::Any, ::Any, ::Any, ::Any, ::Any, ::Any) (772 children)

So we do see it is a (::Type{Int64})(::Integer) signature that we need to be less specific than, which as you note isn't possible as long as StaticInt is an Integer.

[Tokazama]

Note that we are probably moving most of this stuff to https://github.com/SciML/Static.jl.

[timholy]

If it's all array traits, why not JuliaArrays? This stuff should be useful across many domains.

[Tokazama]

I meant the StaticInt, StaticSymbol, StaticBool stuff is being moved to Static.jl. I think it makes sense to move ArrayInterface.jl to JuliaArrays but I'd prefer someone with more seniority than me make the final decision on that.