Skip to content

Commit 8af6d04

Browse files
timholytimholy
authored
A few changes to the analysis of abstract signatures demo (#17)
1 parent 2918905 commit 8af6d04

File tree

3 files changed

+228
-41
lines changed

3 files changed

+228
-41
lines changed

‎demos/abstract.jl‎

Lines changed: 56 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,23 @@
11
using MethodAnalysis
22

3-
43
"""
5-
atrisktyp(tt)
4+
is_atrisk_type(tt)
65
76
Given a Tuple-type signature (e.g., `Tuple{typeof(sum),Vector{Int}}`), determine whether this signature
87
is "at risk" for invalidation. Essentially it returns `true` if one or more arguments are of abstract type,
98
although there are prominent exceptions:
109
11-
- `Function` is allowed
12-
- any constructor call is allowed
13-
- `convert(X, x)` where `isa(x, X)` is true
14-
- `setindex!` and `push!` methods where the valtype is a subtype of the eltype (likewise keytype for AbstractDicts)
10+
- Constructor calls with arbitrary argument types
11+
- `convert(X, x)` where `isa(x, X)`
12+
- `setindex!` and `push!` methods where the valtype is a subtype of the eltype (for AbstractDicts, likewise for the keytype)
1513
- `getindex`, `length`, `isempty`, and `iterate` on any tuple
14+
15+
All of these are "allowed," meaning that they return `false`.
16+
Moreover, some specific non-concrete argument types---like `Union`s of concrete types and `Function`---
17+
do not trigger a return of `true`, although other at-risk argument types can lead to an overall `true` return
18+
for the signature.
1619
"""
17-
function atrisktype(@nospecialize(typ))
20+
function is_atrisk_type(@nospecialize(typ))
1821
# signatures like `convert(Vector, a)`, `foo(::Vararg{Synbol,N}) where N` do not seem to pose a problem
1922
isa(typ, TypeVar) && return false
2023
# isbits parameters are not a problem
@@ -24,7 +27,7 @@ function atrisktype(@nospecialize(typ))
2427
end
2528
# Exclude signatures with Union{}
2629
typ === Union{} && return false
27-
isa(typ, Union) && return atrisktype(typ.a) | atrisktype(typ.b)
30+
isa(typ, Union) && return is_atrisk_type(typ.a) | is_atrisk_type(typ.b)
2831
# Type{T}: signatures like `convert(::Type{AbstractString}, ::String)` are not problematic
2932
typ <: Type && return false
3033
if typ <: Tuple && length(typ.parameters) >= 1
@@ -38,6 +41,9 @@ function atrisktype(@nospecialize(typ))
3841
p2 = Base.unwrap_unionall(p2)
3942
if isa(p2, DataType) && length(p2.parameters) === 1
4043
T = p2.parameters[1]
44+
if isa(T, TypeVar)
45+
T = T.ub
46+
end
4147
isa(p3, Type) && isa(T, Type) && p3 <: T && return false
4248
end
4349
end
@@ -53,9 +59,9 @@ function atrisktype(@nospecialize(typ))
5359
end
5460
# show(io::IO, x) is OK as long as typeof(x) is safe
5561
elseif p1 === typeof(Base.show) || p1 === typeof(Base.print) || p1 === typeof(Base.println)
56-
# atrisktype(typ.parameters[2]) && return true
62+
# is_atrisk_type(typ.parameters[2]) && return true
5763
for i = 3:length(typ.parameters)
58-
atrisktype(typ.parameters[i]) && return true
64+
is_atrisk_type(typ.parameters[i]) && return true
5965
end
6066
return false
6167
# setindex!(a, x, idx) and push!(a, x) are safe if typeof(x) <: eltype(a)
@@ -75,30 +81,46 @@ function atrisktype(@nospecialize(typ))
7581
isconcretetype(typ) && return false
7682
# ::Function args are excluded
7783
typ === Function && return false
78-
!isempty(typ.parameters) && (any(atrisktype, typ.parameters) || return false)
84+
!isempty(typ.parameters) && (any(is_atrisk_type, typ.parameters) || return false)
7985
return true
8086
end
8187

82-
@assert atrisktype(Tuple{typeof(==),Any,Any})
83-
@assert atrisktype(Tuple{typeof(==),Symbol,Any})
84-
@assert atrisktype(Tuple{typeof(==),Any,Symbol})
85-
@assert !atrisktype(Tuple{typeof(==),Symbol,Symbol})
86-
@assert !atrisktype(Tuple{typeof(convert),Type{Any},Any})
87-
@assert !atrisktype(Tuple{typeof(convert),Type{AbstractString},AbstractString})
88-
@assert !atrisktype(Tuple{typeof(convert),Type{AbstractString},String})
89-
@assert atrisktype(Tuple{typeof(convert),Type{String},AbstractString})
90-
@assert !atrisktype(Tuple{typeof(map),Function,Vector{Any}})
91-
@assert !atrisktype(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Union{String,Int}})
92-
@assert atrisktype(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Any})
93-
@assert !atrisktype(Tuple{Type{BoundsError},Any,Any})
94-
@assert atrisktype(Tuple{typeof(sin),Any})
95-
@assert !atrisktype(Tuple{typeof(length),Tuple{Any,Any}})
96-
@assert atrisktype(Tuple{typeof(setindex!),Vector{Int},Any,Int})
97-
@assert !atrisktype(Tuple{typeof(setindex!),Vector{Any},Any,Int})
98-
@assert atrisktype(Tuple{typeof(push!),Vector{Int},Any})
99-
@assert !atrisktype(Tuple{typeof(push!),Vector{Any},Any})
88+
@assert is_atrisk_type(Tuple{typeof(==),Any,Any})
89+
@assert is_atrisk_type(Tuple{typeof(==),Symbol,Any})
90+
@assert is_atrisk_type(Tuple{typeof(==),Any,Symbol})
91+
@assert !is_atrisk_type(Tuple{typeof(==),Symbol,Symbol})
92+
@assert !is_atrisk_type(Tuple{typeof(convert),Type{Any},Any})
93+
@assert !is_atrisk_type(Tuple{typeof(convert),Type{AbstractString},AbstractString})
94+
@assert !is_atrisk_type(Tuple{typeof(convert),Type{AbstractString},String})
95+
@assert is_atrisk_type(Tuple{typeof(convert),Type{String},AbstractString})
96+
@assert !is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Int})
97+
@assert !is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Int32})
98+
@assert is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Integer})
99+
@assert !is_atrisk_type(Tuple{typeof(convert),Type{T} where T<:Union{Int,Float32},Int})
100+
@assert !is_atrisk_type(Tuple{typeof(map),Function,Vector{Any}})
101+
@assert !is_atrisk_type(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Union{String,Int}})
102+
@assert is_atrisk_type(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Any})
103+
@assert !is_atrisk_type(Tuple{Type{BoundsError},Any,Any})
104+
@assert is_atrisk_type(Tuple{typeof(sin),Any})
105+
@assert !is_atrisk_type(Tuple{typeof(length),Tuple{Any,Any}})
106+
@assert is_atrisk_type(Tuple{typeof(setindex!),Vector{Int},Any,Int})
107+
@assert !is_atrisk_type(Tuple{typeof(setindex!),Vector{Any},Any,Int})
108+
@assert is_atrisk_type(Tuple{typeof(push!),Vector{Int},Any})
109+
@assert !is_atrisk_type(Tuple{typeof(push!),Vector{Any},Any})
110+
111+
# Get the name of a method as written in the code. This strips keyword-method mangling.
112+
function codename(sym::Symbol)
113+
symstr = String(sym)
114+
# Body methods
115+
m = match(r"^#(.*?)#\d+$", symstr)
116+
m !== nothing && return Symbol(only(m.captures))
117+
# kw methods
118+
m = match(r"^(.*?)##kw$", symstr)
119+
m !== nothing && return Symbol(only(m.captures))
120+
return sym
121+
end
100122

101-
isexported(mi::Core.MethodInstance) = isdefined(Main, mi.def.name)
123+
isexported(mi::Core.MethodInstance) = isdefined(Main, codename(mi.def.name))
102124
getfunc(mi::Core.MethodInstance) = getfunc(mi.def)
103125
getfunc(m::Method) = getfield(m.module, m.name)
104126
nmethods(mi::Core.MethodInstance) = length(methods(getfunc(mi)))
@@ -124,7 +146,7 @@ end
124146
const becounter = Dict{Core.MethodInstance,Int}()
125147
visit() do item
126148
if item isa Core.MethodInstance && !fromcc(item.def.module)
127-
if atrisktype(item.specTypes)
149+
if is_atrisk_type(item.specTypes)
128150
becounter[item] = length(all_backedges(item))
129151
end
130152
return false
@@ -141,15 +163,12 @@ open("/tmp/methdata_$VERSION.log", "w") do io
141163
end
142164

143165
# Split into exported & private functions
144-
mtup = (nmethods = 0, nbackedges = 0)
145-
miexp = Pair{Core.MethodInstance,typeof(mtup)}[]
166+
miexp = Pair{Core.MethodInstance,Int}[]
146167
mipriv = similar(miexp)
147168
for (mi, c) in prs
148-
n = nmethods(mi)
149-
pr = mi=>(nmethods=n, nbackedges=c)
150169
if isexported(mi)
151-
push!(miexp, pr)
170+
push!(miexp, mi=>c)
152171
else
153-
push!(mipriv, pr)
172+
push!(mipriv, mi=>c)
154173
end
155174
end

‎demos/abstract_tests.jl‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
atrisk_backedges = with_all_backedges(keys(becounter))
2+
3+
function atrisk_method(m::Method, atrisk_backedges)
4+
for mi in methodinstances(m)
5+
mi ∈ atrisk_backedges && return true
6+
end
7+
return false
8+
end
9+
10+
function atrisk_triggers(m::Method, atrisk_instances)
11+
triggers = Set{Core.MethodInstance}()
12+
for mi in atrisk_instances
13+
if atrisk_method(m, all_backedges(mi))
14+
push!(triggers, mi)
15+
end
16+
end
17+
return triggers
18+
end
19+
20+
# This removes MethodInstances that no one in their right mind should ever invalidate by specialization.
21+
function remove_unlikely_methodinstances(list)
22+
out = Core.MethodInstance[]
23+
for mi in list
24+
mi = mi::Core.MethodInstance # must have MethodInstance elements
25+
# All `continue` statements below omit the MethodInstance
26+
name = codename(mi.def.name)
27+
name ∈ (:invokelatest, :unwrap_unionall, :rewrap_unionall) && continue
28+
name ∈ (:print, :sprint) && length(mi.specTypes.parameters) - mi.def.nkw > 3 && continue
29+
# No one should ever specialize on notify or schedule's `val` argument
30+
name === :notify && !is_atrisk_type(mi.specTypes.parameters[2]) &&
31+
!any(is_atrisk_type, mi.specTypes.parameters[4:end]) && continue
32+
name === :schedule && !any(is_atrisk_type, mi.specTypes.parameters[2:end-1]) && continue
33+
# Add more removal-filters here
34+
35+
# We've decided to keep it
36+
push!(out, mi)
37+
end
38+
return out
39+
end
40+
41+
using Test
42+
43+
# Invalidating the code that loads packages leads to major slowdowns, especially if it happens repeatedly
44+
# in a dependent chain of package loads. Ideally, we'd make this code-path "bulletproof".
45+
for m in methods(Base.require)
46+
@test_broken isempty(remove_unlikely_methodinstances(atrisk_triggers(m, first.(miexp))))
47+
@test_broken isempty(remove_unlikely_methodinstances(atrisk_triggers(m, first.(mipriv))))
48+
end
49+
50+
# Test overall number of atrisk MethodInstances and their average number of backedges
51+
badexp = Set(remove_unlikely_methodinstances(first.(miexp)))
52+
badcounts = filter(pr->pr.first ∈ badexp, miexp)
53+
@test length(badcounts) < 1000
54+
if length(badcounts) < 800
55+
@info "There are now only $(length(badcounts)) at-risk specializations of exported methods, consider dropping the threshold"
56+
end
57+
meancounts = sum(last.(badcounts))/length(badcounts)
58+
@test meancounts < 32
59+
if meancounts < 24
60+
@info "The mean number of at-risk backedges is now only $meancounts, consider dropping the threshold"
61+
end
62+
63+
# Check for inference quality in specific functions.
64+
# This is valid only for functions that should always return a particular type for any valid call of their methods.
65+
function function_returns(@nospecialize(f), @nospecialize(typ); allow_missing_for_missing=true, minargs=0)
66+
for m in methods(f)
67+
sig = Base.unwrap_unionall(m.sig)
68+
for rt in Base.return_types(call_type(Base.unwrap_unionall(m.sig))...)
69+
rt <: typ && continue
70+
if allow_missing_for_missing && any(T->T===Missing, sig.parameters[2:end]) && rt === Missing
71+
continue
72+
end
73+
length(sig.parameters) - 1 < minargs && continue
74+
return false
75+
end
76+
end
77+
return true
78+
end
79+
80+
# All the is* functions
81+
# Not all of the broken cases have been checked carefully; it's possible some of these should return `Union{Bool,Missing}`
82+
# or something.
83+
@test_broken function_returns(isabspath, Bool)
84+
@test function_returns(isabstracttype, Bool)
85+
@test_broken function_returns(isapprox, Bool)
86+
@test_broken function_returns(isascii, Bool)
87+
# @test function_returns(isassigned, Bool)
88+
@test function_returns(isbits, Bool)
89+
@test function_returns(isbitstype, Bool)
90+
@test function_returns(isblockdev, Bool)
91+
@test function_returns(ischardev, Bool)
92+
@test function_returns(iscntrl, Bool)
93+
@test function_returns(isconcretetype, Bool)
94+
@test function_returns(isconst, Bool)
95+
@test function_returns(isdefined, Bool)
96+
@test function_returns(isdigit, Bool)
97+
@test function_returns(isdir, Bool)
98+
@test function_returns(isdirpath, Bool)
99+
@test_broken function_returns(isdisjoint, Bool)
100+
@test function_returns(isdispatchtuple, Bool)
101+
@test_broken function_returns(isempty, Bool)
102+
@test_broken function_returns(isequal, Bool; minargs=2)
103+
@test_broken function_returns(iseven, Bool)
104+
@test function_returns(isexported, Bool)
105+
@test function_returns(isfifo, Bool)
106+
@test function_returns(isfile, Bool)
107+
@test_broken function_returns(isfinite, Bool)
108+
@test_broken function_returns(isinf, Bool)
109+
@test_broken function_returns(isinteger, Bool)
110+
@test function_returns(isinteractive, Bool)
111+
@test_broken function_returns(isless, Bool)
112+
@test function_returns(isletter, Bool)
113+
@test function_returns(islink, Bool)
114+
@test function_returns(islocked, Bool)
115+
@test function_returns(islowercase, Bool)
116+
@test_broken function_returns(ismarked, Bool)
117+
@test function_returns(ismissing, Bool)
118+
@test function_returns(ismount, Bool)
119+
@test function_returns(ismutable, Bool)
120+
@test_broken function_returns(isnan, Bool)
121+
@test function_returns(isnothing, Bool)
122+
@test function_returns(isnumeric, Bool)
123+
@test_broken function_returns(isodd, Bool)
124+
@test_broken function_returns(isone, Bool)
125+
@test_broken function_returns(isopen, Bool)
126+
@test function_returns(ispath, Bool)
127+
@test_broken function_returns(isperm, Bool)
128+
@test function_returns(ispow2, Bool)
129+
@test function_returns(isprimitivetype, Bool)
130+
@test function_returns(isprint, Bool)
131+
@test function_returns(ispunct, Bool)
132+
@test_broken function_returns(isreadable, Bool)
133+
@test_broken function_returns(isreadonly, Bool)
134+
@test_broken function_returns(isready, Bool)
135+
@test_broken function_returns(isreal, Bool)
136+
@test function_returns(issetequal, Bool)
137+
@test function_returns(issetgid, Bool)
138+
@test function_returns(issetuid, Bool)
139+
@test function_returns(issocket, Bool)
140+
@test_broken function_returns(issorted, Bool)
141+
@test function_returns(isspace, Bool)
142+
@test function_returns(issticky, Bool)
143+
@test function_returns(isstructtype, Bool)
144+
@test_broken function_returns(issubnormal, Bool)
145+
@test_broken function_returns(issubset, Bool)
146+
@test function_returns(istaskdone, Bool)
147+
@test function_returns(istaskfailed, Bool)
148+
@test function_returns(istaskstarted, Bool)
149+
@test_broken function_returns(istextmime, Bool)
150+
@test function_returns(isuppercase, Bool)
151+
@test_broken function_returns(isvalid, Bool)
152+
@test_broken function_returns(iswritable, Bool)
153+
@test function_returns(isxdigit, Bool)
154+
@test_broken function_returns(iszero, Bool)
155+
156+
@test_broken function_returns(eof, Bool) # just the Pkg.TOML one is broken
157+
158+
# Check that we never infer certain methodinstances
159+
for f in (==, isequal, <, <=)
160+
for mi in methodinstances(==)
161+
if any(T->T<:Real, mi.specTypes.parameters)
162+
@test !is_atrisk_type(mi.specTypes)
163+
end
164+
end
165+
end

‎src/findcallers.jl‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
export findcallers
22

3-
function get_typed_instances!(srcs, mi::MethodInstance, world, interp)
4-
# This is essentially take from code_typed_by_type
5-
tt = mi.specTypes
3+
function get_typed_instances!(srcs, @nospecialize(tt), method::Method, world, interp)
4+
# This is essentially taken from code_typed_by_type
65
matches = Base._methods_by_ftype(tt, -1, world)
76
if matches === false
87
error("signature $(item.specTypes) does not correspond to a generic function")
98
end
109
for match in matches
11-
match.method == mi.def || continue
10+
match.method == method || continue
1211
meth = Base.func_for_method_checked(match.method, tt, match.sparams)
1312
(src, ty) = isdefined(Core.Compiler, :NativeInterpreter) ?
1413
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)
1716
end
1817
return srcs
1918
end
19+
get_typed_instances!(srcs, mi::MethodInstance, world, interp) = get_typed_instances!(srcs, mi.specTypes, mi.def, world, interp)
2020

2121
defaultinterp(world) = isdefined(Core.Compiler, :NativeInterpreter) ?
2222
Core.Compiler.NativeInterpreter(world) :
@@ -25,6 +25,9 @@ defaultinterp(world) = isdefined(Core.Compiler, :NativeInterpreter) ?
2525
function get_typed_instances(mi::MethodInstance; world=typemax(UInt), interp=defaultinterp(world))
2626
return get_typed_instances!(Tuple{CodeInfo,Core.SimpleVector}[], mi, world, interp)
2727
end
28+
function get_typed_instances(@nospecialize(tt), method::Method; world=typemax(UInt), interp=defaultinterp(world))
29+
return get_typed_instances!(Tuple{CodeInfo,Core.SimpleVector}[], tt, method, world, interp)
30+
end
2831

2932
struct CallMatch
3033
mi::MethodInstance

0 commit comments

Comments
 (0)