diff --git a/demos/abstract.jl b/demos/abstract.jl index fb35003..9fe3dd9 100644 --- a/demos/abstract.jl +++ b/demos/abstract.jl @@ -1,20 +1,23 @@ using MethodAnalysis - """ - atrisktyp(tt) + is_atrisk_type(tt) Given a Tuple-type signature (e.g., `Tuple{typeof(sum),Vector{Int}}`), determine whether this signature is "at risk" for invalidation. Essentially it returns `true` if one or more arguments are of abstract type, although there are prominent exceptions: -- `Function` is allowed -- any constructor call is allowed -- `convert(X, x)` where `isa(x, X)` is true -- `setindex!` and `push!` methods where the valtype is a subtype of the eltype (likewise keytype for AbstractDicts) +- Constructor calls with arbitrary argument types +- `convert(X, x)` where `isa(x, X)` +- `setindex!` and `push!` methods where the valtype is a subtype of the eltype (for AbstractDicts, likewise for the keytype) - `getindex`, `length`, `isempty`, and `iterate` on any tuple + +All of these are "allowed," meaning that they return `false`. +Moreover, some specific non-concrete argument types---like `Union`s of concrete types and `Function`--- +do not trigger a return of `true`, although other at-risk argument types can lead to an overall `true` return +for the signature. """ -function atrisktype(@nospecialize(typ)) +function is_atrisk_type(@nospecialize(typ)) # signatures like `convert(Vector, a)`, `foo(::Vararg{Synbol,N}) where N` do not seem to pose a problem isa(typ, TypeVar) && return false # isbits parameters are not a problem @@ -24,7 +27,7 @@ function atrisktype(@nospecialize(typ)) end # Exclude signatures with Union{} typ === Union{} && return false - isa(typ, Union) && return atrisktype(typ.a) | atrisktype(typ.b) + isa(typ, Union) && return is_atrisk_type(typ.a) | is_atrisk_type(typ.b) # Type{T}: signatures like `convert(::Type{AbstractString}, ::String)` are not problematic typ <: Type && return false if typ <: Tuple && length(typ.parameters) >= 1 @@ -38,6 +41,9 @@ function atrisktype(@nospecialize(typ)) p2 = Base.unwrap_unionall(p2) if isa(p2, DataType) && length(p2.parameters) === 1 T = p2.parameters[1] + if isa(T, TypeVar) + T = T.ub + end isa(p3, Type) && isa(T, Type) && p3 <: T && return false end end @@ -53,9 +59,9 @@ function atrisktype(@nospecialize(typ)) end # show(io::IO, x) is OK as long as typeof(x) is safe elseif p1 === typeof(Base.show) || p1 === typeof(Base.print) || p1 === typeof(Base.println) - # atrisktype(typ.parameters[2]) && return true + # is_atrisk_type(typ.parameters[2]) && return true for i = 3:length(typ.parameters) - atrisktype(typ.parameters[i]) && return true + is_atrisk_type(typ.parameters[i]) && return true end return false # setindex!(a, x, idx) and push!(a, x) are safe if typeof(x) <: eltype(a) @@ -75,30 +81,46 @@ function atrisktype(@nospecialize(typ)) isconcretetype(typ) && return false # ::Function args are excluded typ === Function && return false - !isempty(typ.parameters) && (any(atrisktype, typ.parameters) || return false) + !isempty(typ.parameters) && (any(is_atrisk_type, typ.parameters) || return false) return true end -@assert atrisktype(Tuple{typeof(==),Any,Any}) -@assert atrisktype(Tuple{typeof(==),Symbol,Any}) -@assert atrisktype(Tuple{typeof(==),Any,Symbol}) -@assert !atrisktype(Tuple{typeof(==),Symbol,Symbol}) -@assert !atrisktype(Tuple{typeof(convert),Type{Any},Any}) -@assert !atrisktype(Tuple{typeof(convert),Type{AbstractString},AbstractString}) -@assert !atrisktype(Tuple{typeof(convert),Type{AbstractString},String}) -@assert atrisktype(Tuple{typeof(convert),Type{String},AbstractString}) -@assert !atrisktype(Tuple{typeof(map),Function,Vector{Any}}) -@assert !atrisktype(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Union{String,Int}}) -@assert atrisktype(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Any}) -@assert !atrisktype(Tuple{Type{BoundsError},Any,Any}) -@assert atrisktype(Tuple{typeof(sin),Any}) -@assert !atrisktype(Tuple{typeof(length),Tuple{Any,Any}}) -@assert atrisktype(Tuple{typeof(setindex!),Vector{Int},Any,Int}) -@assert !atrisktype(Tuple{typeof(setindex!),Vector{Any},Any,Int}) -@assert atrisktype(Tuple{typeof(push!),Vector{Int},Any}) -@assert !atrisktype(Tuple{typeof(push!),Vector{Any},Any}) +@assert is_atrisk_type(Tuple{typeof(==),Any,Any}) +@assert is_atrisk_type(Tuple{typeof(==),Symbol,Any}) +@assert is_atrisk_type(Tuple{typeof(==),Any,Symbol}) +@assert !is_atrisk_type(Tuple{typeof(==),Symbol,Symbol}) +@assert !is_atrisk_type(Tuple{typeof(convert),Type{Any},Any}) +@assert !is_atrisk_type(Tuple{typeof(convert),Type{AbstractString},AbstractString}) +@assert !is_atrisk_type(Tuple{typeof(convert),Type{AbstractString},String}) +@assert is_atrisk_type(Tuple{typeof(convert),Type{String},AbstractString}) +@assert !is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Int}) +@assert !is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Int32}) +@assert is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Integer}) +@assert !is_atrisk_type(Tuple{typeof(convert),Type{T} where T<:Union{Int,Float32},Int}) +@assert !is_atrisk_type(Tuple{typeof(map),Function,Vector{Any}}) +@assert !is_atrisk_type(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Union{String,Int}}) +@assert is_atrisk_type(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Any}) +@assert !is_atrisk_type(Tuple{Type{BoundsError},Any,Any}) +@assert is_atrisk_type(Tuple{typeof(sin),Any}) +@assert !is_atrisk_type(Tuple{typeof(length),Tuple{Any,Any}}) +@assert is_atrisk_type(Tuple{typeof(setindex!),Vector{Int},Any,Int}) +@assert !is_atrisk_type(Tuple{typeof(setindex!),Vector{Any},Any,Int}) +@assert is_atrisk_type(Tuple{typeof(push!),Vector{Int},Any}) +@assert !is_atrisk_type(Tuple{typeof(push!),Vector{Any},Any}) + +# Get the name of a method as written in the code. This strips keyword-method mangling. +function codename(sym::Symbol) + symstr = String(sym) + # Body methods + m = match(r"^#(.*?)#\d+$", symstr) + m !== nothing && return Symbol(only(m.captures)) + # kw methods + m = match(r"^(.*?)##kw$", symstr) + m !== nothing && return Symbol(only(m.captures)) + return sym +end -isexported(mi::Core.MethodInstance) = isdefined(Main, mi.def.name) +isexported(mi::Core.MethodInstance) = isdefined(Main, codename(mi.def.name)) getfunc(mi::Core.MethodInstance) = getfunc(mi.def) getfunc(m::Method) = getfield(m.module, m.name) nmethods(mi::Core.MethodInstance) = length(methods(getfunc(mi))) @@ -124,7 +146,7 @@ end const becounter = Dict{Core.MethodInstance,Int}() visit() do item if item isa Core.MethodInstance && !fromcc(item.def.module) - if atrisktype(item.specTypes) + if is_atrisk_type(item.specTypes) becounter[item] = length(all_backedges(item)) end return false @@ -141,15 +163,12 @@ open("/tmp/methdata_$VERSION.log", "w") do io end # Split into exported & private functions -mtup = (nmethods = 0, nbackedges = 0) -miexp = Pair{Core.MethodInstance,typeof(mtup)}[] +miexp = Pair{Core.MethodInstance,Int}[] mipriv = similar(miexp) for (mi, c) in prs - n = nmethods(mi) - pr = mi=>(nmethods=n, nbackedges=c) if isexported(mi) - push!(miexp, pr) + push!(miexp, mi=>c) else - push!(mipriv, pr) + push!(mipriv, mi=>c) end end diff --git a/demos/abstract_tests.jl b/demos/abstract_tests.jl new file mode 100644 index 0000000..27f2e9e --- /dev/null +++ b/demos/abstract_tests.jl @@ -0,0 +1,165 @@ +atrisk_backedges = with_all_backedges(keys(becounter)) + +function atrisk_method(m::Method, atrisk_backedges) + for mi in methodinstances(m) + mi ∈ atrisk_backedges && return true + end + return false +end + +function atrisk_triggers(m::Method, atrisk_instances) + triggers = Set{Core.MethodInstance}() + for mi in atrisk_instances + if atrisk_method(m, all_backedges(mi)) + push!(triggers, mi) + end + end + return triggers +end + +# This removes MethodInstances that no one in their right mind should ever invalidate by specialization. +function remove_unlikely_methodinstances(list) + out = Core.MethodInstance[] + for mi in list + mi = mi::Core.MethodInstance # must have MethodInstance elements + # All `continue` statements below omit the MethodInstance + name = codename(mi.def.name) + name ∈ (:invokelatest, :unwrap_unionall, :rewrap_unionall) && continue + name ∈ (:print, :sprint) && length(mi.specTypes.parameters) - mi.def.nkw > 3 && continue + # No one should ever specialize on notify or schedule's `val` argument + name === :notify && !is_atrisk_type(mi.specTypes.parameters[2]) && + !any(is_atrisk_type, mi.specTypes.parameters[4:end]) && continue + name === :schedule && !any(is_atrisk_type, mi.specTypes.parameters[2:end-1]) && continue + # Add more removal-filters here + + # We've decided to keep it + push!(out, mi) + end + return out +end + +using Test + +# Invalidating the code that loads packages leads to major slowdowns, especially if it happens repeatedly +# in a dependent chain of package loads. Ideally, we'd make this code-path "bulletproof". +for m in methods(Base.require) + @test_broken isempty(remove_unlikely_methodinstances(atrisk_triggers(m, first.(miexp)))) + @test_broken isempty(remove_unlikely_methodinstances(atrisk_triggers(m, first.(mipriv)))) +end + +# Test overall number of atrisk MethodInstances and their average number of backedges +badexp = Set(remove_unlikely_methodinstances(first.(miexp))) +badcounts = filter(pr->pr.first ∈ badexp, miexp) +@test length(badcounts) < 1000 +if length(badcounts) < 800 + @info "There are now only $(length(badcounts)) at-risk specializations of exported methods, consider dropping the threshold" +end +meancounts = sum(last.(badcounts))/length(badcounts) +@test meancounts < 32 +if meancounts < 24 + @info "The mean number of at-risk backedges is now only $meancounts, consider dropping the threshold" +end + +# Check for inference quality in specific functions. +# This is valid only for functions that should always return a particular type for any valid call of their methods. +function function_returns(@nospecialize(f), @nospecialize(typ); allow_missing_for_missing=true, minargs=0) + for m in methods(f) + sig = Base.unwrap_unionall(m.sig) + for rt in Base.return_types(call_type(Base.unwrap_unionall(m.sig))...) + rt <: typ && continue + if allow_missing_for_missing && any(T->T===Missing, sig.parameters[2:end]) && rt === Missing + continue + end + length(sig.parameters) - 1 < minargs && continue + return false + end + end + return true +end + +# All the is* functions +# Not all of the broken cases have been checked carefully; it's possible some of these should return `Union{Bool,Missing}` +# or something. +@test_broken function_returns(isabspath, Bool) +@test function_returns(isabstracttype, Bool) +@test_broken function_returns(isapprox, Bool) +@test_broken function_returns(isascii, Bool) +# @test function_returns(isassigned, Bool) +@test function_returns(isbits, Bool) +@test function_returns(isbitstype, Bool) +@test function_returns(isblockdev, Bool) +@test function_returns(ischardev, Bool) +@test function_returns(iscntrl, Bool) +@test function_returns(isconcretetype, Bool) +@test function_returns(isconst, Bool) +@test function_returns(isdefined, Bool) +@test function_returns(isdigit, Bool) +@test function_returns(isdir, Bool) +@test function_returns(isdirpath, Bool) +@test_broken function_returns(isdisjoint, Bool) +@test function_returns(isdispatchtuple, Bool) +@test_broken function_returns(isempty, Bool) +@test_broken function_returns(isequal, Bool; minargs=2) +@test_broken function_returns(iseven, Bool) +@test function_returns(isexported, Bool) +@test function_returns(isfifo, Bool) +@test function_returns(isfile, Bool) +@test_broken function_returns(isfinite, Bool) +@test_broken function_returns(isinf, Bool) +@test_broken function_returns(isinteger, Bool) +@test function_returns(isinteractive, Bool) +@test_broken function_returns(isless, Bool) +@test function_returns(isletter, Bool) +@test function_returns(islink, Bool) +@test function_returns(islocked, Bool) +@test function_returns(islowercase, Bool) +@test_broken function_returns(ismarked, Bool) +@test function_returns(ismissing, Bool) +@test function_returns(ismount, Bool) +@test function_returns(ismutable, Bool) +@test_broken function_returns(isnan, Bool) +@test function_returns(isnothing, Bool) +@test function_returns(isnumeric, Bool) +@test_broken function_returns(isodd, Bool) +@test_broken function_returns(isone, Bool) +@test_broken function_returns(isopen, Bool) +@test function_returns(ispath, Bool) +@test_broken function_returns(isperm, Bool) +@test function_returns(ispow2, Bool) +@test function_returns(isprimitivetype, Bool) +@test function_returns(isprint, Bool) +@test function_returns(ispunct, Bool) +@test_broken function_returns(isreadable, Bool) +@test_broken function_returns(isreadonly, Bool) +@test_broken function_returns(isready, Bool) +@test_broken function_returns(isreal, Bool) +@test function_returns(issetequal, Bool) +@test function_returns(issetgid, Bool) +@test function_returns(issetuid, Bool) +@test function_returns(issocket, Bool) +@test_broken function_returns(issorted, Bool) +@test function_returns(isspace, Bool) +@test function_returns(issticky, Bool) +@test function_returns(isstructtype, Bool) +@test_broken function_returns(issubnormal, Bool) +@test_broken function_returns(issubset, Bool) +@test function_returns(istaskdone, Bool) +@test function_returns(istaskfailed, Bool) +@test function_returns(istaskstarted, Bool) +@test_broken function_returns(istextmime, Bool) +@test function_returns(isuppercase, Bool) +@test_broken function_returns(isvalid, Bool) +@test_broken function_returns(iswritable, Bool) +@test function_returns(isxdigit, Bool) +@test_broken function_returns(iszero, Bool) + +@test_broken function_returns(eof, Bool) # just the Pkg.TOML one is broken + +# Check that we never infer certain methodinstances +for f in (==, isequal, <, <=) + for mi in methodinstances(==) + if any(T->T<:Real, mi.specTypes.parameters) + @test !is_atrisk_type(mi.specTypes) + end + end +end diff --git a/src/findcallers.jl b/src/findcallers.jl index 3fa2236..dc964a9 100644 --- a/src/findcallers.jl +++ b/src/findcallers.jl @@ -1,14 +1,13 @@ export findcallers -function get_typed_instances!(srcs, mi::MethodInstance, world, interp) - # This is essentially take from code_typed_by_type - tt = mi.specTypes +function get_typed_instances!(srcs, @nospecialize(tt), method::Method, world, interp) + # This is essentially taken from code_typed_by_type matches = Base._methods_by_ftype(tt, -1, world) if matches === false error("signature $(item.specTypes) does not correspond to a generic function") end for match in matches - match.method == mi.def || continue + match.method == method || continue meth = Base.func_for_method_checked(match.method, tt, match.sparams) (src, ty) = isdefined(Core.Compiler, :NativeInterpreter) ? Core.Compiler.typeinf_code(interp, meth, match.spec_types, match.sparams, false) : @@ -17,6 +16,7 @@ function get_typed_instances!(srcs, mi::MethodInstance, world, interp) end return srcs end +get_typed_instances!(srcs, mi::MethodInstance, world, interp) = get_typed_instances!(srcs, mi.specTypes, mi.def, world, interp) defaultinterp(world) = isdefined(Core.Compiler, :NativeInterpreter) ? Core.Compiler.NativeInterpreter(world) : @@ -25,6 +25,9 @@ defaultinterp(world) = isdefined(Core.Compiler, :NativeInterpreter) ? function get_typed_instances(mi::MethodInstance; world=typemax(UInt), interp=defaultinterp(world)) return get_typed_instances!(Tuple{CodeInfo,Core.SimpleVector}[], mi, world, interp) end +function get_typed_instances(@nospecialize(tt), method::Method; world=typemax(UInt), interp=defaultinterp(world)) + return get_typed_instances!(Tuple{CodeInfo,Core.SimpleVector}[], tt, method, world, interp) +end struct CallMatch mi::MethodInstance