diff --git a/Project.toml b/Project.toml index 30f428127..e926c5889 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "SnoopCompile" uuid = "aa65fe97-06da-5843-b5b1-d5d13cad87d2" author = ["Tim Holy "] -version = "2.5.2" +version = "2.6.0" [deps] Cthulhu = "f68482b8-f384-11e8-15f7-abe071a5a75f" diff --git a/SnoopCompileCore/src/snoopi.jl b/SnoopCompileCore/src/snoopi.jl index 2ff0dbd9d..8e204cd99 100644 --- a/SnoopCompileCore/src/snoopi.jl +++ b/SnoopCompileCore/src/snoopi.jl @@ -91,11 +91,11 @@ function __init__() # We do this in __init__ to make sure it gets compiled to native code # (the *.ji file stores only the inferred code) if isdefined(Core.Compiler, :Params) - @assert precompile(typeinf_ext_timed, (Core.MethodInstance, Core.Compiler.Params)) - @assert precompile(typeinf_ext_timed, (Core.MethodInstance, UInt)) + precompile(typeinf_ext_timed, (Core.MethodInstance, Core.Compiler.Params)) || error("precompilation of typeinf modifiers is not allowed to fail") + precompile(typeinf_ext_timed, (Core.MethodInstance, UInt)) || error("precompilation of typeinf modifiers is not allowed to fail") else - @assert precompile(typeinf_ext_timed, (Core.Compiler.NativeInterpreter, Core.MethodInstance)) - @assert precompile(typeinf_ext_timed, (Core.MethodInstance, UInt)) + precompile(typeinf_ext_timed, (Core.Compiler.NativeInterpreter, Core.MethodInstance)) || error("precompilation of typeinf modifiers is not allowed to fail") + precompile(typeinf_ext_timed, (Core.MethodInstance, UInt)) || error("precompilation of typeinf modifiers is not allowed to fail") end precompile(start_timing, ()) precompile(stop_timing, ()) diff --git a/SnoopCompileCore/src/snoopi_deep.jl b/SnoopCompileCore/src/snoopi_deep.jl index 8c43a7531..46f86555b 100644 --- a/SnoopCompileCore/src/snoopi_deep.jl +++ b/SnoopCompileCore/src/snoopi_deep.jl @@ -96,7 +96,6 @@ end # These are okay to come at the top-level because we're only measuring inference, and # inference results will be cached in a `.ji` file. -@assert precompile(Core.Compiler.Timings.reset_timings, ()) -@assert precompile(start_deep_timing, ()) -@assert precompile(stop_deep_timing, ()) -@assert precompile(finish_snoopi_deep, ()) +precompile(start_deep_timing, ()) +precompile(stop_deep_timing, ()) +precompile(finish_snoopi_deep, ()) diff --git a/docs/src/snoopi.md b/docs/src/snoopi.md index b96550b1a..a5722c666 100644 --- a/docs/src/snoopi.md +++ b/docs/src/snoopi.md @@ -184,15 +184,18 @@ inference on those methods, because it used the inference results from the cache methods that appear in your `"precompile.jl"` file. This will *not* result in an error; by default `precompile` fails silently. If you want to be certain that your precompile directives don't go stale, - preface each with an `@assert`. - Note that this forces you to update your precompile directives as you modify your package, - which may or may not be desirable. + you can check that `precompile` returns `true` and otherwise issue a warning. + By default, [`SnoopCompile.write`](@ref) generates + a macro, `@warnpcfail`, and you can use it by + changing `precompile(args...)` to `@warnpcfail precompile(args...)`. + If you find that some precompile directives are ineffective (they appear in a new `@snoopi` despite being precompiled) and their inference time is substantial, sometimes a bit of manual investigation of the callees can lead to insights. For example, you might be able to introduce a precompile in a dependent package that can mitigate the total time. +(`@snoopi_deep` makes the analysis and resolution of these issues more straightforward.) ## Producing precompile directives manually @@ -278,8 +281,7 @@ This will cause Julia to infer this method for the given argument types. If you !!! note The `true` indicates that Julia was successfully able to find a method supporting this signature and precompile it. - Some people put `@assert` in front of their package's `precompile` statements--this way, if you delete or modify methods, "stale" - `precompile` directives will trigger an error, thus notifying you that they need to be updated. + See the note about `@warnpcfail` above for ways to exploit this in your package. But if you execute these lines in the REPL, and then check how well it worked, you might see something like the following: diff --git a/src/SnoopCompile.jl b/src/SnoopCompile.jl index b82e07247..42119fab4 100644 --- a/src/SnoopCompile.jl +++ b/src/SnoopCompile.jl @@ -64,6 +64,21 @@ const kwbodyrex = r"^##(\w[^#]*)#\d+" # detect keyword body methods const genrex = r"^##s\d+#\d+$" # detect generators for @generated functions const innerrex = r"^#[^#]+#\d+" # detect inner functions +# This is for SnoopCompile's own directives. You don't want to call this from packages because then +# SnoopCompile becomes a dependency of your package. Instead, make sure that `writewarnpcfail` is set to `true` +# in `SnoopCompile.write` and a copy of this macro will be placed at the top +# of your precompile files. +macro warnpcfail(ex::Expr) + modl = __module__ + file = __source__.file === nothing ? "?" : String(__source__.file) + line = __source__.line + quote + $(esc(ex)) || @warn """precompile directive + $($(Expr(:quote, ex))) + failed. Please report an issue in $($modl) (after checking for duplicates) or remove this directive.""" _file=$file _line=$line + end +end + # Parcel include("parcel_snoopc.jl") diff --git a/src/parcel_snoopi_deep.jl b/src/parcel_snoopi_deep.jl index 7ed56368c..36646fe0f 100644 --- a/src/parcel_snoopi_deep.jl +++ b/src/parcel_snoopi_deep.jl @@ -1804,12 +1804,12 @@ end for IO in (IOContext{Base.TTY}, IOContext{IOBuffer}, IOBuffer) for T = (InferenceTimingNode, InferenceTrigger, Precompiles, MethodLoc, MethodTriggers, Location, LocationTriggers) - @assert precompile(show, (IO, T)) + @warnpcfail precompile(show, (IO, T)) end end -@assert precompile(flamegraph, (InferenceTimingNode,)) -@assert precompile(inference_triggers, (InferenceTimingNode,)) -@assert precompile(flatten, (InferenceTimingNode,)) -@assert precompile(accumulate_by_source, (Vector{InferenceTiming},)) -@assert precompile(isprecompilable, (MethodInstance,)) -@assert precompile(parcel, (InferenceTimingNode,)) +@warnpcfail precompile(flamegraph, (InferenceTimingNode,)) +@warnpcfail precompile(inference_triggers, (InferenceTimingNode,)) +@warnpcfail precompile(flatten, (InferenceTimingNode,)) +@warnpcfail precompile(accumulate_by_source, (Vector{InferenceTiming},)) +@warnpcfail precompile(isprecompilable, (MethodInstance,)) +@warnpcfail precompile(parcel, (InferenceTimingNode,)) diff --git a/src/write.jl b/src/write.jl index ee72d0101..287670460 100644 --- a/src/write.jl +++ b/src/write.jl @@ -1,20 +1,35 @@ # Write precompiles for userimg.jl -function write(io::IO, pc::Vector{<:AbstractString}) +const warnpcfail_str = """ +# Use +# @warnpcfail precompile(args...) +# if you want to be warned when a precompile directive fails +macro warnpcfail(ex::Expr) + modl = __module__ + file = __source__.file === nothing ? "?" : String(__source__.file) + line = __source__.line + quote + \$(esc(ex)) || @warn \"\"\"precompile directive + \$(\$(Expr(:quote, ex))) + failed. Please report an issue in \$(\$modl) (after checking for duplicates) or remove this directive.\"\"\" _file=\$file _line=\$line + end +end +""" + +function write(io::IO, pc::Vector{<:AbstractString}; writewarnpcfail::Bool=true) + writewarnpcfail && println(io, warnpcfail_str, '\n') for ln in pc println(io, ln) end end -function write(filename::AbstractString, pc::Vector) +function write(filename::AbstractString, pc::Vector; kwargs...) path, fn = splitdir(filename) if !isdir(path) mkpath(path) end - ret = nothing - open(filename, "w") do io - ret = write(io, pc) + return open(filename, "w") do io + write(io, pc) end - return ret end """ @@ -31,6 +46,7 @@ function write(prefix::AbstractString, pc::Dict; always::Bool = false) end for (k, v) in pc open(joinpath(prefix, "precompile_$k.jl"), "w") do io + println(io, warnpcfail_str, '\n') if any(str->occursin("__lookup", str), v) println(io, lookup_kwbody_str) end diff --git a/test/runtests.jl b/test/runtests.jl index a8b11c7b3..748f01220 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -18,6 +18,7 @@ end using SnoopCompile +@testset "Miscellaneous" begin # issue #26 logfile = joinpath(tempdir(), "anon.log") @snoopc logfile begin @@ -46,6 +47,22 @@ data = SnoopCompile.read(logfile) pc = SnoopCompile.parcel(reverse!(data[2])) @test any(startswith.(pc[:IsDef], "isdefined")) +@testset "Warning for failures to precompile" begin + pcs = ["@warnpcfail precompile(fsimple, (Char,))", + "@warnpcfail precompile(fsimple, (Char, Char))", + "@warnpcfail precompile(fsimple, (Char, Char, Char))", + ] + fn = tempname() * ".jl" + open(fn, "w") do io + println(io, "fsimple(x, y) = 1") + SnoopCompile.write(io, pcs; writewarnpcfail=true) + end + @test_logs (:warn, r"precompile\(fsimple, \(Char,\)\)") (:warn, r"precompile\(fsimple, \(Char, Char, Char\)\)") include(fn) + rm(fn) +end + +end + #= # Simple call let str = "sum"