Skip to content

Commit

Permalink
Add a generic code-inferrability test
Browse files Browse the repository at this point in the history 
Closes #36393
  • Loading branch information
@timholy
timholy committed Nov 8, 2020
1 parent 07fd900 commit c8127ea
Show file tree
Hide file tree
Showing 4 changed files with 494 additions and 1 deletion.
15 changes: 15 additions & 0 deletions base/reflection.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,13 @@ function signature_type(@nospecialize(f), @nospecialize(args))
end
end

function call_type(@nospecialize(tt))
ft = tt.parameters[1]
argt = Tuple{tt.parameters[2:end]...}
name = Symbol(String(ft.name.name)[2:end]) # strip off leading '#'
return (getfield(ft.name.module, name), argt)
end

"""
code_lowered(f, types; generated=true, debuginfo=:default)
Expand Down Expand Up @@ -929,6 +936,14 @@ function visit(f, d::Core.TypeMapEntry)
end
nothing
end
function visit(f, d::SimpleVector)
for i = 1:length(d)
if isassigned(d, i)
f(d[i])
end
end
nothing
end

function length(mt::Core.MethodTable)
n = 0
Expand Down
4 changes: 3 additions & 1 deletion test/choosetests.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function choosetests(choices = [])
"boundscheck", "error", "ambiguous", "cartesian", "osutils",
"channels", "iostream", "secretbuffer", "specificity",
"reinterpretarray", "syntax", "corelogging", "missing", "asyncmap",
"smallarrayshrink"
"smallarrayshrink", "inference_qa"
]

tests = []
Expand Down Expand Up @@ -102,6 +102,8 @@ function choosetests(choices = [])
"compiler/ssair", "compiler/irpasses", "compiler/codegen",
"compiler/inline", "compiler/contextual"])
filtertests!(tests, "stdlib", STDLIBS)
# do inference_qa very early to avoid trouble from specializations introduced by running other tests
filtertests!(tests, "inference_qa")
# do ambiguous first to avoid failing if ambiguities are introduced by other tests
filtertests!(tests, "ambiguous")

Expand Down
216 changes: 216 additions & 0 deletions test/inference_qa.jl
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

# The aim of these tests is to enforce a minimum quality of inferrability of Julia's
# Base and specifically its precompiled methods. Passing these tests does not
# indicate that Julia has no inference problems, but they are designed to catch some
# inadvertent problems. While `@inferred` only tests the return type, the tests
# in this file are designed to check the overall inference quality of method *internals*.

# If you fail tests here, you can usually fix problems using `@code_warntype` or Cthulhu.jl's `@descend`.

using Test

isdefined(@__MODULE__, :is_atrisk_type) || include("testhelpers/methodanalysis.jl")

## Test the testing tools

@testset "is_atrisk_type" begin
@test is_atrisk_type(Tuple{typeof(==),Any,Any})
@test is_atrisk_type(Tuple{typeof(==),Symbol,Any})
@test is_atrisk_type(Tuple{typeof(==),Any,Symbol})
@test !is_atrisk_type(Tuple{typeof(==),Symbol,Symbol})
@test !is_atrisk_type(Tuple{typeof(convert),Type{Any},Any})
@test !is_atrisk_type(Tuple{typeof(convert),Type{AbstractString},AbstractString})
@test !is_atrisk_type(Tuple{typeof(convert),Type{AbstractString},String})
@test is_atrisk_type(Tuple{typeof(convert),Type{String},AbstractString})
@test !is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Int})
@test !is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Int32})
@test is_atrisk_type(Tuple{typeof(convert),Type{Union{Int,Float32}},Integer})
@test !is_atrisk_type(Tuple{typeof(convert),Type{T} where T<:Union{Int,Float32},Int})
@test !is_atrisk_type(Tuple{typeof(map),Function,Vector{Any}})
@test !is_atrisk_type(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Union{String,Int}})
@test is_atrisk_type(Tuple{typeof(getindex),Dict{Union{String,Int},Any},Any})
@test !is_atrisk_type(Tuple{Type{BoundsError},Any,Any})
@test is_atrisk_type(Tuple{typeof(sin),Any})
@test !is_atrisk_type(Tuple{typeof(length),Tuple{Any,Any}})
@test is_atrisk_type(Tuple{typeof(setindex!),Vector{Int},Any,Int})
@test !is_atrisk_type(Tuple{typeof(setindex!),Vector{Any},Any,Int})
@test is_atrisk_type(Tuple{typeof(push!),Vector{Int},Any})
@test !is_atrisk_type(Tuple{typeof(push!),Vector{Any},Any})
@test !is_atrisk_type(Tuple{typeof(push!),Vector{T} where T,Any})
end

## Prepwork

# Count # of backedges for MethodInstances with abstract types

function get_atrisk_methodinstances()
mivulnerabilities = Pair{MethodInstance,Int}[]
for mi in methodinstances()
if !fromcc(mi.def.module)
if is_atrisk_type(mi.specTypes)
push!(mivulnerabilities, mi => length(all_backedges(mi)))
end
end
end
return sort!(mivulnerabilities; by=last)
end
mivulnerabilities = get_atrisk_methodinstances()

# Get all the things that depends on these at-risk methodinstances
const atrisk_methodinstances = Set{MethodInstance}()
for (mi, c) in mivulnerabilities
isdefined(mi, :backedges) && all_backedges!(atrisk_methodinstances, mi)
push!(atrisk_methodinstances, mi)
end

# Split into exported & private functions
mivulnerabilities_exported = Pair{MethodInstance,Int}[]
mivulnerabilities_private = similar(mivulnerabilities_exported)
for (mi, c) in mivulnerabilities
if isexported(mi)
push!(mivulnerabilities_exported, mi=>c)
else
push!(mivulnerabilities_private, mi=>c)
end
end

## Tests

@testset "Base.require vulnerabilities" begin
# 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 isempty(remove_unlikely_methodinstances(atrisk_triggers(m, first.(mivulnerabilities_exported))))

# It's far less important to protect against invalidation of private functions,
# since generally packages should not extend these functions. Nevertheless it wouldn't
# be a bad thing.
@test_broken isempty(remove_unlikely_methodinstances(atrisk_triggers(m, first.(mivulnerabilities_private))))
end
end

# If you fail these tests, it may or may not be your fault---you may just be the
# one that pushed one of these tests over the edge. Check `badcounts` and `meancounts`
# before and after your changes; if the change is quite small, it's unlikely that your changes
# are the root cause. In such cases, failures here should be investigated or reported,
# but if inference problems can be fixed elsewhere you may not have to change your PR.
# Conversely, if your changes *do* increase these numbers substantially, your changes have likely
# triggered widespread inference problems---you should fix them before merging your PR.
#
# Never increase the thresholds here without public discussion. Indeed, the goal is to
# shrink them as much as possible.
@testset "Aggregate at-risk methodinstances" begin
# Test overall number of atrisk MethodInstances and their average number of backedges
badexp = Set(remove_unlikely_methodinstances(first.(mivulnerabilities_exported)))
badcounts = filter(pr->pr.first ∈ badexp, mivulnerabilities_exported)
@test length(badcounts) < 1250 # 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 < 33 # 32
if meancounts < 24
@info "The mean number of at-risk backedges is now only $meancounts, consider dropping the threshold"
end
end

@testset "Specific return types" begin
# 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 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 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_broken 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 function_returns(eof, Bool)
end

@testset "Never inferred" begin
# Check that we never infer certain methodinstances,
# It would be great to broaden this beyond Real, but this is a good start.
# If you fail these tests, it means an internal operation forced
# the compiler to infer one of these methods for a problematic combination of types.
function subtype_real(@nospecialize T)
while isa(T, TypeVar)
T = T.ub
end
return T<:Real
end
for f in (==, isequal, <, <=)
for mi in methodinstances(f)
if any(subtype_real, Base.unwrap_unionall(mi.specTypes).parameters)
@test !is_atrisk_type(mi.specTypes)
end
end
end
end
Loading

0 comments on commit c8127ea

Please sign in to comment.