Skip to content

Commit

Permalink
reduce precompile() failure severity to a warning (JuliaLang#39905)
Browse files Browse the repository at this point in the history
Many users (including Base) are calling `@assert`, despite that this is
not what assert should be used to mark, for many reasons.

This happened to also reveal a small number of errors, so also detect
those (for fixing later).

Refs: JuliaLang@c0f9666#commitcomment-47782674
  • Loading branch information
vtjnash committed Mar 11, 2021
1 parent 3276c11 commit 93b89b9
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 86 deletions.
1 change: 1 addition & 0 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,7 @@ end

function abstract_call_builtin(interp::AbstractInterpreter, f::Builtin, fargs::Union{Nothing,Vector{Any}},
argtypes::Vector{Any}, sv::InferenceState, max_methods::Int)
@nospecialize f
la = length(argtypes)
if f === ifelse && fargs isa Vector{Any} && la == 4
cnd = argtypes[2]
Expand Down
13 changes: 0 additions & 13 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -483,19 +483,6 @@ sizeof(x) = Core.sizeof(x)
# simple Array{Any} operations needed for bootstrap
@eval setindex!(A::Array{Any}, @nospecialize(x), i::Int) = arrayset($(Expr(:boundscheck)), A, x, i)

"""
precompile(f, args::Tuple{Vararg{Any}})
Compile the given function `f` for the argument tuple (of types) `args`, but do not execute it.
"""
function precompile(@nospecialize(f), args::Tuple)
ccall(:jl_compile_hint, Int32, (Any,), Tuple{Core.Typeof(f), args...}) != 0
end

function precompile(argt::Type)
ccall(:jl_compile_hint, Int32, (Any,), argt) != 0
end

"""
esc(e)
Expand Down
29 changes: 23 additions & 6 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1218,11 +1218,9 @@ function include_package_for_output(pkg::PkgId, input::String, depot_path::Vecto
end
end

@assert precompile(include_package_for_output, (PkgId,String,Vector{String},Vector{String},Vector{String},typeof(_concrete_dependencies),Nothing))
@assert precompile(include_package_for_output, (PkgId,String,Vector{String},Vector{String},Vector{String},typeof(_concrete_dependencies),String))

const PRECOMPILE_TRACE_COMPILE = Ref{String}()
function create_expr_cache(pkg::PkgId, input::String, output::String, concrete_deps::typeof(_concrete_dependencies), internal_stderr::IO = stderr, internal_stdout::IO = stdout)
@nospecialize internal_stderr internal_stdout
rm(output, force=true) # Remove file if it exists
depot_path = map(abspath, DEPOT_PATH)
dl_load_path = map(abspath, DL_LOAD_PATH)
Expand Down Expand Up @@ -1261,9 +1259,6 @@ function create_expr_cache(pkg::PkgId, input::String, output::String, concrete_d
return io
end

@assert precompile(create_expr_cache, (PkgId, String, String, typeof(_concrete_dependencies), typeof(stderr), typeof(stdout)))
@assert precompile(create_expr_cache, (PkgId, String, String, typeof(_concrete_dependencies), typeof(stderr), typeof(stdout)))

function compilecache_dir(pkg::PkgId)
entrypath, entryfile = cache_file_entry(pkg)
return joinpath(DEPOT_PATH[1], entrypath)
Expand Down Expand Up @@ -1294,6 +1289,7 @@ This can be used to reduce package load times. Cache files are stored in
for important notes.
"""
function compilecache(pkg::PkgId, internal_stderr::IO = stderr, internal_stdout::IO = stdout)
@nospecialize internal_stderr internal_stdout
path = locate_package(pkg)
path === nothing && throw(ArgumentError("$pkg not found during precompilation"))
return compilecache(pkg, path, internal_stderr, internal_stdout)
Expand All @@ -1302,6 +1298,7 @@ end
const MAX_NUM_PRECOMPILE_FILES = Ref(10)

function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, internal_stdout::IO = stdout)
@nospecialize internal_stderr internal_stdout
# decide where to put the resulting cache file
cachepath = compilecache_dir(pkg)

Expand Down Expand Up @@ -1810,3 +1807,23 @@ macro __DIR__()
_dirname = dirname(String(__source__.file::Symbol))
return isempty(_dirname) ? pwd() : abspath(_dirname)
end

"""
precompile(f, args::Tuple{Vararg{Any}})
Compile the given function `f` for the argument tuple (of types) `args`, but do not execute it.
"""
function precompile(@nospecialize(f), args::Tuple)
precompile(Tuple{Core.Typeof(f), args...})
end

function precompile(argt::Type)
if ccall(:jl_compile_hint, Int32, (Any,), argt) == 0
@warn "Inactive precompile statement" maxlog=100 form=argt _module=nothing _file=nothing _line=0
end
true
end

precompile(include_package_for_output, (PkgId, String, Vector{String}, Vector{String}, Vector{String}, typeof(_concrete_dependencies), Nothing))
precompile(include_package_for_output, (PkgId, String, Vector{String}, Vector{String}, Vector{String}, typeof(_concrete_dependencies), String))
precompile(create_expr_cache, (PkgId, String, String, typeof(_concrete_dependencies), IO, IO))
147 changes: 82 additions & 65 deletions contrib/generate_precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,32 +17,32 @@ DOWN_ARROW = "\e[B"

hardcoded_precompile_statements = """
# used by Revise.jl
@assert precompile(Tuple{typeof(Base.parse_cache_header), String})
@assert precompile(Base.read_dependency_src, (String, String))
precompile(Tuple{typeof(Base.parse_cache_header), String})
precompile(Base.read_dependency_src, (String, String))
# used by Requires.jl
@assert precompile(Tuple{typeof(get!), Type{Vector{Function}}, Dict{Base.PkgId,Vector{Function}}, Base.PkgId})
@assert precompile(Tuple{typeof(haskey), Dict{Base.PkgId,Vector{Function}}, Base.PkgId})
@assert precompile(Tuple{typeof(delete!), Dict{Base.PkgId,Vector{Function}}, Base.PkgId})
@assert precompile(Tuple{typeof(push!), Vector{Function}, Function})
precompile(Tuple{typeof(get!), Type{Vector{Function}}, Dict{Base.PkgId,Vector{Function}}, Base.PkgId})
precompile(Tuple{typeof(haskey), Dict{Base.PkgId,Vector{Function}}, Base.PkgId})
precompile(Tuple{typeof(delete!), Dict{Base.PkgId,Vector{Function}}, Base.PkgId})
precompile(Tuple{typeof(push!), Vector{Function}, Function})
# miscellaneous
@assert precompile(Tuple{typeof(Base.require), Base.PkgId})
@assert precompile(Tuple{typeof(Base.recursive_prefs_merge), Base.Dict{String, Any}})
@assert precompile(Tuple{typeof(isassigned), Core.SimpleVector, Int})
@assert precompile(Tuple{typeof(getindex), Core.SimpleVector, Int})
@assert precompile(Tuple{typeof(Base.Experimental.register_error_hint), Any, Type})
@assert precompile(Tuple{typeof(Base.display_error), MethodError, Vector{Union{Ptr{Nothing}, Base.InterpreterIP}}})
@assert precompile(Tuple{typeof(Base.display_error), ErrorException})
@assert precompile(Tuple{typeof(Base.display_error), BoundsError})
@assert precompile(Tuple{Core.kwftype(typeof(Type)), NamedTuple{(:sizehint,), Tuple{Int64}}, Type{IOBuffer}})
@assert precompile(Base.CoreLogging.current_logger_for_env, (Base.CoreLogging.LogLevel, String, Module))
@assert precompile(Base.CoreLogging.current_logger_for_env, (Base.CoreLogging.LogLevel, Symbol, Module))
precompile(Tuple{typeof(Base.require), Base.PkgId})
precompile(Tuple{typeof(Base.recursive_prefs_merge), Base.Dict{String, Any}})
precompile(Tuple{typeof(isassigned), Core.SimpleVector, Int})
precompile(Tuple{typeof(getindex), Core.SimpleVector, Int})
precompile(Tuple{typeof(Base.Experimental.register_error_hint), Any, Type})
precompile(Tuple{typeof(Base.display_error), MethodError, Vector{Union{Ptr{Nothing}, Base.InterpreterIP}}})
precompile(Tuple{typeof(Base.display_error), ErrorException})
precompile(Tuple{typeof(Base.display_error), BoundsError})
precompile(Tuple{Core.kwftype(typeof(Type)), NamedTuple{(:sizehint,), Tuple{Int}}, Type{IOBuffer}})
precompile(Base.CoreLogging.current_logger_for_env, (Base.CoreLogging.LogLevel, String, Module))
precompile(Base.CoreLogging.current_logger_for_env, (Base.CoreLogging.LogLevel, Symbol, Module))
"""

for T in (Float16, Float32, Float64), IO in (IOBuffer, IOContext{IOBuffer}, Base.TTY, IOContext{Base.TTY})
global hardcoded_precompile_statements
hardcoded_precompile_statements *= "@assert precompile(Tuple{typeof(show), $IO, $T})\n"
hardcoded_precompile_statements *= "precompile(Tuple{typeof(show), $IO, $T})\n"
end

repl_script = """
Expand Down Expand Up @@ -108,7 +108,7 @@ have_repl = haskey(Base.loaded_modules,
Base.PkgId(Base.UUID("3fa0cd96-eef1-5676-8a61-b3b8758bbffb"), "REPL"))
if have_repl
hardcoded_precompile_statements *= """
@assert precompile(Tuple{typeof(getproperty), REPL.REPLBackend, Symbol})
precompile(Tuple{typeof(getproperty), REPL.REPLBackend, Symbol})
"""
end

Expand All @@ -117,9 +117,9 @@ Distributed = get(Base.loaded_modules,
nothing)
if Distributed !== nothing
hardcoded_precompile_statements *= """
@assert precompile(Tuple{typeof(Distributed.remotecall),Function,Int,Module,Vararg{Any, 100}})
@assert precompile(Tuple{typeof(Distributed.procs)})
@assert precompile(Tuple{typeof(Distributed.finalize_ref), Distributed.Future})
precompile(Tuple{typeof(Distributed.remotecall),Function,Int,Module,Vararg{Any, 100}})
precompile(Tuple{typeof(Distributed.procs)})
precompile(Tuple{typeof(Distributed.finalize_ref), Distributed.Future})
"""
# This is disabled because it doesn't give much benefit
# and the code in Distributed is poorly typed causing many invalidations
Expand Down Expand Up @@ -164,9 +164,9 @@ FileWatching = get(Base.loaded_modules,
nothing)
if FileWatching !== nothing
hardcoded_precompile_statements *= """
@assert precompile(Tuple{typeof(FileWatching.watch_file), String, Float64})
@assert precompile(Tuple{typeof(FileWatching.watch_file), String, Int})
@assert precompile(Tuple{typeof(FileWatching._uv_hook_close), FileWatching.FileMonitor})
precompile(Tuple{typeof(FileWatching.watch_file), String, Float64})
precompile(Tuple{typeof(FileWatching.watch_file), String, Int})
precompile(Tuple{typeof(FileWatching._uv_hook_close), FileWatching.FileMonitor})
"""
end

Expand All @@ -184,33 +184,33 @@ Test = get(Base.loaded_modules,
nothing)
if Test !== nothing
hardcoded_precompile_statements *= """
@assert precompile(Tuple{typeof(Test.do_test), Test.ExecutionResult, Any})
@assert precompile(Tuple{typeof(Test.testset_beginend), Tuple{String, Expr}, Expr, LineNumberNode})
@assert precompile(Tuple{Type{Test.DefaultTestSet}, String})
@assert precompile(Tuple{Type{Test.DefaultTestSet}, AbstractString})
@assert precompile(Tuple{Core.kwftype(Type{Test.DefaultTestSet}), Any, Type{Test.DefaultTestSet}, AbstractString})
@assert precompile(Tuple{typeof(Test.finish), Test.DefaultTestSet})
@assert precompile(Tuple{typeof(Test.eval_test), Expr, Expr, LineNumberNode, Bool})
@assert precompile(Tuple{typeof(Test._inferred), Expr, Module})
@assert precompile(Tuple{typeof(Test.push_testset), Test.DefaultTestSet})
@assert precompile(Tuple{typeof(Test.get_alignment), Test.DefaultTestSet, Int})
@assert precompile(Tuple{typeof(Test.get_test_result), Any, Any})
@assert precompile(Tuple{typeof(Test.do_test_throws), Test.ExecutionResult, Any, Any})
@assert precompile(Tuple{typeof(Test.print_counts), Test.DefaultTestSet, Int, Int, Int, Int, Int, Int, Int})
@assert precompile(Tuple{typeof(Test._check_testset), Type, Expr})
@assert precompile(Tuple{typeof(Test.test_expr!), Any, Any})
@assert precompile(Tuple{typeof(Test.test_expr!), Any, Any, Vararg{Any, 100}})
@assert precompile(Tuple{typeof(Test.pop_testset)})
@assert precompile(Tuple{typeof(Test.match_logs), Function, Tuple{Symbol, Regex}})
@assert precompile(Tuple{typeof(Test.match_logs), Function, Tuple{String, Regex}})
@assert precompile(Tuple{typeof(Base.CoreLogging.shouldlog), Test.TestLogger, Base.CoreLogging.LogLevel, Module, Symbol, Symbol})
@assert precompile(Tuple{typeof(Base.CoreLogging.handle_message), Test.TestLogger, Base.CoreLogging.LogLevel, String, Module, Symbol, Symbol, String, Int})
@assert precompile(Tuple{typeof(Core.kwfunc(Base.CoreLogging.handle_message)), typeof((exception=nothing,)), typeof(Base.CoreLogging.handle_message), Test.TestLogger, Base.CoreLogging.LogLevel, String, Module, Symbol, Symbol, String, Int})
@assert precompile(Tuple{typeof(Test.detect_ambiguities), Any})
@assert precompile(Tuple{typeof(Test.collect_test_logs), Function})
@assert precompile(Tuple{typeof(Test.do_broken_test), Test.ExecutionResult, Any})
@assert precompile(Tuple{typeof(Test.record), Test.DefaultTestSet, Union{Test.Error, Test.Fail}})
@assert precompile(Tuple{typeof(Test.filter_errors), Test.DefaultTestSet})
precompile(Tuple{typeof(Test.do_test), Test.ExecutionResult, Any})
precompile(Tuple{typeof(Test.testset_beginend), Tuple{String, Expr}, Expr, LineNumberNode})
precompile(Tuple{Type{Test.DefaultTestSet}, String})
precompile(Tuple{Type{Test.DefaultTestSet}, AbstractString})
precompile(Tuple{Core.kwftype(Type{Test.DefaultTestSet}), Any, Type{Test.DefaultTestSet}, AbstractString})
precompile(Tuple{typeof(Test.finish), Test.DefaultTestSet})
precompile(Tuple{typeof(Test.eval_test), Expr, Expr, LineNumberNode, Bool})
precompile(Tuple{typeof(Test._inferred), Expr, Module})
precompile(Tuple{typeof(Test.push_testset), Test.DefaultTestSet})
precompile(Tuple{typeof(Test.get_alignment), Test.DefaultTestSet, Int})
precompile(Tuple{typeof(Test.get_test_result), Any, Any})
precompile(Tuple{typeof(Test.do_test_throws), Test.ExecutionResult, Any, Any})
precompile(Tuple{typeof(Test.print_counts), Test.DefaultTestSet, Int, Int, Int, Int, Int, Int, Int})
precompile(Tuple{typeof(Test._check_testset), Type, Expr})
precompile(Tuple{typeof(Test.test_expr!), Any, Any})
precompile(Tuple{typeof(Test.test_expr!), Any, Any, Vararg{Any, 100}})
precompile(Tuple{typeof(Test.pop_testset)})
precompile(Tuple{typeof(Test.match_logs), Function, Tuple{Symbol, Regex}})
precompile(Tuple{typeof(Test.match_logs), Function, Tuple{String, Regex}})
precompile(Tuple{typeof(Base.CoreLogging.shouldlog), Test.TestLogger, Base.CoreLogging.LogLevel, Module, Symbol, Symbol})
precompile(Tuple{typeof(Base.CoreLogging.handle_message), Test.TestLogger, Base.CoreLogging.LogLevel, String, Module, Symbol, Symbol, String, Int})
precompile(Tuple{typeof(Core.kwfunc(Base.CoreLogging.handle_message)), typeof((exception=nothing,)), typeof(Base.CoreLogging.handle_message), Test.TestLogger, Base.CoreLogging.LogLevel, String, Module, Symbol, Symbol, String, Int})
precompile(Tuple{typeof(Test.detect_ambiguities), Any})
precompile(Tuple{typeof(Test.collect_test_logs), Function})
precompile(Tuple{typeof(Test.do_broken_test), Test.ExecutionResult, Any})
precompile(Tuple{typeof(Test.record), Test.DefaultTestSet, Union{Test.Error, Test.Fail}})
precompile(Tuple{typeof(Test.filter_errors), Test.DefaultTestSet})
"""
end

Expand All @@ -219,7 +219,7 @@ Profile = get(Base.loaded_modules,
nothing)
if Profile !== nothing
hardcoded_precompile_statements *= """
@assert precompile(Tuple{typeof(Profile.tree!), Profile.StackFrameTree{UInt64}, Vector{UInt64}, Dict{UInt64, Vector{Base.StackTraces.StackFrame}}, Bool, Symbol})
precompile(Tuple{typeof(Profile.tree!), Profile.StackFrameTree{UInt64}, Vector{UInt64}, Dict{UInt64, Vector{Base.StackTraces.StackFrame}}, Bool, Symbol})
"""
end

Expand Down Expand Up @@ -350,32 +350,49 @@ function generate_precompile_statements()
n_succeeded = 0
include_time = @elapsed for statement in sort!(collect(statements))
# println(statement)
# The compiler has problem caching signatures with `Vararg{?, N}`. Replacing
# N with a large number seems to work around it.
# XXX: skip some that are broken. these are caused by issue #39902
occursin("Tuple{Artifacts.var\"#@artifact_str\", LineNumberNode, Module, Any, Any}", statement) && continue
occursin("Tuple{Base.Cartesian.var\"#@ncall\", LineNumberNode, Module, Int64, Any, Vararg{Any}}", statement) && continue
occursin("Tuple{Base.Cartesian.var\"#@ncall\", LineNumberNode, Module, Int32, Any, Vararg{Any}}", statement) && continue
occursin("Tuple{Base.Cartesian.var\"#@nloops\", LineNumberNode, Module, Any, Any, Any, Vararg{Any}}", statement) && continue
occursin("Tuple{Core.var\"#@doc\", LineNumberNode, Module, Vararg{Any}}", statement) && continue
# XXX: this is strange, as this isn't the correct representation of this
occursin("typeof(Core.IntrinsicFunction)", statement) && continue
# XXX: this is strange, as this method should not be getting compiled
occursin(", Core.Compiler.AbstractInterpreter, ", statement) && continue
try
ps = Meta.parse(statement)
if isexpr(ps, :call)
if isexpr(ps.args[end], :curly)
l = ps.args[end]
if length(l.args) == 2 && l.args[1] == :Vararg
push!(l.args, 100)
end
isexpr(ps, :call) || continue
popfirst!(ps.args) # precompile(...)
ps.head = :tuple
l = ps.args[end]
if (isexpr(l, :tuple) || isexpr(l, :curly)) && length(l.args) > 0 # Tuple{...} or (...)
# XXX: precompile doesn't currently handle overloaded Vararg arguments very well.
# Replacing N with a large number works around it.
l = l.args[end]
if isexpr(l, :curly) && length(l.args) == 2 && l.args[1] == :Vararg # Vararg{T}
push!(l.args, 100) # form Vararg{T, 100} instead
end
end
# println(ps)
Core.eval(PrecompileStagingArea, ps)
ps = Core.eval(PrecompileStagingArea, ps)
# XXX: precompile doesn't currently handle overloaded nospecialize arguments very well.
# Skipping them avoids the warning.
ms = length(ps) == 1 ? Base._methods_by_ftype(ps[1], 1, Base.get_world_counter()) : Base.methods(ps...)
ms isa Vector || continue
precompile(ps...)
n_succeeded += 1
print("\rExecuting precompile statements... $n_succeeded/$(length(statements))")
catch
catch ex
# See #28808
# @error "Failed to precompile $statement"
@warn "Failed to precompile expression" form=statement exception=ex _module=nothing _file=nothing _line=0
end
end
println()
if have_repl
# Seems like a reasonable number right now, adjust as needed
# comment out if debugging script
@assert n_succeeded > 1200
n_succeeded > 1200 || @warn "Only $n_succeeded precompile statements"
end

tot_time = time_ns() - start_time
Expand Down
4 changes: 2 additions & 2 deletions test/ambiguous.jl
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ end
## Other ways of accessing functions
# Test that non-ambiguous cases work
let io = IOBuffer()
@test precompile(ambig, (Int, Int)) == true
@test @test_logs precompile(ambig, (Int, Int))
cf = @eval @cfunction(ambig, Int, (Int, Int))
@test ccall(cf, Int, (Int, Int), 1, 2) == 4
@test length(code_lowered(ambig, (Int, Int))) == 1
Expand All @@ -75,7 +75,7 @@ end

# Test that ambiguous cases fail appropriately
let io = IOBuffer()
@test precompile(ambig, (UInt8, Int)) == false
@test @test_logs (:warn,) precompile(ambig, (UInt8, Int))
cf = @eval @cfunction(ambig, Int, (UInt8, Int)) # test for a crash (doesn't throw an error)
@test_throws(MethodError(ambig, (UInt8(1), Int(2)), get_world_counter()),
ccall(cf, Int, (UInt8, Int), 1, 2))
Expand Down

0 comments on commit 93b89b9

Please sign in to comment.