From 7c3b0d320d304c35f10450052f5467d7c35ffb6e Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Mon, 26 Sep 2022 05:25:41 -0500 Subject: [PATCH] Fix attribution of "delayed" invalidations (#301) "Delayed" invalidations occur during package loading in response to some event that happened before the package was loaded. Because the trigger is separated in time from its consequences, we did not formerly have a mechanism to clearly attribute these to a cause. Julia 1.9 has recently added more logging to make this more robust; this PR updates SnoopCompile to take advantage of the new information. --- src/invalidations.jl | 91 +++++++++++++------ src/parcel_snoopi_deep.jl | 2 +- test/snoopr.jl | 28 +++++- test/testmodules/Invalidation/Manifest.toml | 16 ++++ .../Invalidation/PkgC/Project.toml | 4 + .../testmodules/Invalidation/PkgC/src/PkgC.jl | 6 ++ .../Invalidation/PkgD/Manifest.toml | 10 ++ .../Invalidation/PkgD/Project.toml | 7 ++ .../testmodules/Invalidation/PkgD/src/PkgD.jl | 11 +++ test/testmodules/Invalidation/Project.toml | 3 + 10 files changed, 147 insertions(+), 31 deletions(-) create mode 100644 test/testmodules/Invalidation/Manifest.toml create mode 100644 test/testmodules/Invalidation/PkgC/Project.toml create mode 100644 test/testmodules/Invalidation/PkgC/src/PkgC.jl create mode 100644 test/testmodules/Invalidation/PkgD/Manifest.toml create mode 100644 test/testmodules/Invalidation/PkgD/Project.toml create mode 100644 test/testmodules/Invalidation/PkgD/src/PkgD.jl create mode 100644 test/testmodules/Invalidation/Project.toml diff --git a/src/invalidations.jl b/src/invalidations.jl index 0a28331d..4f5fbf4c 100644 --- a/src/invalidations.jl +++ b/src/invalidations.jl @@ -286,20 +286,14 @@ julia> trees = invalidation_trees(@snoopr f(::AbstractFloat) = 3) See the documentation for further details. """ function invalidation_trees(list; exclude_corecompiler::Bool=true) - function checkreason(reason, loctag) - if loctag == "jl_method_table_disable" - @assert reason === nothing || reason === :deleting - reason = :deleting - elseif loctag == "jl_method_table_insert" - @assert reason === nothing || reason === :inserting - reason = :inserting - else - error("unexpected reason ", loctag) - end - return reason - end function handle_insert_backedges(list, i, callee) + if Base.VERSION >= v"1.9.0-DEV.1432" + key = (list[i+=1], list[i+=1]) + backedge_table[key] = (callee, list[i+=1]) + return i + end + ncovered = 0 callees = Any[callee] while length(list) >= i+2 && list[i+2] == "insert_backedges_callee" @@ -322,6 +316,7 @@ function invalidation_trees(list; exclude_corecompiler::Bool=true) leaf = nothing mt_backedges, backedges, mt_cache, mt_disable = methinv_storage() reason = nothing + backedge_table = Dict{Tuple{Int32,UInt64},Tuple{Any,Vector{Any}}}() i = 0 while i < length(list) item = list[i+=1] @@ -377,10 +372,32 @@ function invalidation_trees(list; exclude_corecompiler::Bool=true) elseif loctag == "insert_backedges_callee" i = handle_insert_backedges(list, i, mi) elseif loctag == "insert_backedges" - # pre Julia 1.8 - println("insert_backedges for ", mi) - Base.VERSION < v"1.8.0-DEV.368" || error("unexpected failure at ", i) - @assert leaf === nothing + if Base.VERSION >= v"1.9.0-DEV.1432" + key = (list[i+=1], list[i+=1]) + trig, causes = backedge_table[key] + if leaf !== nothing + root = getroot(leaf) + root.mi = mi + if trig isa MethodInstance + oldroot = root + root = InstanceNode(trig, [root]) + oldroot.parent = root + push!(backedges, root) + else + push!(mt_backedges, trig=>root) + end + end + for cause in causes + add_method_trigger!(methodinvs, cause, :inserting, mt_backedges, backedges, mt_cache, mt_disable) + end + mt_backedges, backedges, mt_cache, mt_disable = methinv_storage() + leaf = nothing + reason = nothing + else + # pre Julia 1.8 + Base.VERSION < v"1.8.0-DEV.368" || error("unexpected failure at ", i) + @assert leaf === nothing + end else error("unexpected loctag ", loctag, " at ", i) end @@ -393,18 +410,7 @@ function invalidation_trees(list; exclude_corecompiler::Bool=true) item = list[i+=1] if isa(item, String) reason = checkreason(reason, item) - found = false - for tree in methodinvs - if tree.method == method && tree.reason == reason - join_invalidations!(tree.mt_backedges, mt_backedges) - append!(tree.backedges, backedges) - append!(tree.mt_cache, mt_cache) - append!(tree.mt_disable, mt_disable) - found = true - break - end - end - found || push!(methodinvs, sort!(MethodInvalidations(method, reason, mt_backedges, backedges, mt_cache, mt_disable))) + add_method_trigger!(methodinvs, method, reason, mt_backedges, backedges, mt_cache, mt_disable) mt_backedges, backedges, mt_cache, mt_disable = methinv_storage() leaf = nothing reason = nothing @@ -477,6 +483,22 @@ function invalidation_trees(list; exclude_corecompiler::Bool=true) return sort!(methodinvs; by=countchildren) end +function add_method_trigger!(methodinvs, method::Method, reason::Symbol, mt_backedges, backedges, mt_cache, mt_disable) + found = false + for tree in methodinvs + if tree.method == method && tree.reason == reason + join_invalidations!(tree.mt_backedges, mt_backedges) + append!(tree.backedges, backedges) + append!(tree.mt_cache, mt_cache) + append!(tree.mt_disable, mt_disable) + found = true + break + end + end + found || push!(methodinvs, sort!(MethodInvalidations(method, reason, mt_backedges, backedges, mt_cache, mt_disable))) + return methodinvs +end + function join_invalidations!(list::AbstractVector{<:Pair}, items::AbstractVector{<:Pair}) for (key, root) in items found = false @@ -511,6 +533,19 @@ function join_branches!(to, from) return to end +function checkreason(reason, loctag) + if loctag == "jl_method_table_disable" + @assert reason === nothing || reason === :deleting + reason = :deleting + elseif loctag == "jl_method_table_insert" + @assert reason === nothing || reason === :inserting + reason = :inserting + else + error("unexpected reason ", loctag) + end + return reason +end + """ thinned = filtermod(module, trees::AbstractVector{MethodInvalidations}; recursive=false) diff --git a/src/parcel_snoopi_deep.jl b/src/parcel_snoopi_deep.jl index 6d999ba5..e6d7569e 100644 --- a/src/parcel_snoopi_deep.jl +++ b/src/parcel_snoopi_deep.jl @@ -287,7 +287,7 @@ Precompiles(node::InferenceTimingNode) = Precompiles(InferenceTiming(node).mi_in Core.MethodInstance(pc::Precompiles) = MethodInstance(pc.mi_info) SnoopCompileCore.inclusive(pc::Precompiles) = pc.total_time -precompilable_time(precompiles::Vector{Tuple{Float64,MethodInstance}}) where T = sum(first, precompiles; init=0.0) +precompilable_time(precompiles::Vector{Tuple{Float64,MethodInstance}}) = sum(first, precompiles; init=0.0) precompilable_time(precompiles::Dict{MethodInstance,T}) where T = sum(values(precompiles); init=zero(T)) precompilable_time(pc::Precompiles) = precompilable_time(pc.precompiles) diff --git a/test/snoopr.jl b/test/snoopr.jl index 619ec75e..5e779dfc 100644 --- a/test/snoopr.jl +++ b/test/snoopr.jl @@ -1,4 +1,4 @@ -using SnoopCompile, InteractiveUtils, MethodAnalysis, Test +using SnoopCompile, InteractiveUtils, MethodAnalysis, Pkg, Test const qualify_mi = Base.VERSION >= v"1.7.0-DEV.5" # julia PR #38608 @@ -230,7 +230,31 @@ end end @testset "Delayed invalidations" begin - if Base.VERSION >= v"1.7.0-DEV.254" # julia#39132 (redirect to Pipe) + if Base.VERSION >= v"1.9.0-DEV.1432" # julia#46756 + cproj = Base.active_project() + cd(joinpath(@__DIR__, "testmodules", "Invalidation")) do + Pkg.activate(pwd()) + Pkg.develop(path="./PkgC") + Pkg.develop(path="./PkgD") + Pkg.precompile() + invalidations = @snoopr begin + @eval begin + using PkgC + PkgC.nbits(::UInt8) = 8 + using PkgD + end + end + tree = only(invalidation_trees(invalidations)) + @test tree.reason == :inserting + @test tree.method.file == Symbol(@__FILE__) + @test isempty(tree.backedges) + sig, root = only(tree.mt_backedges) + @test sig.parameters[1] === typeof(PkgC.nbits) + @test sig.parameters[2] === Integer + @test root.mi == only(methods(PkgD.call_nbits)).specializations[1] + end + Pkg.activate(cproj) + elseif Base.VERSION >= v"1.7.0-DEV.254" # julia#39132 (redirect to Pipe) # "Natural" tests are performed in the "Stale" testset of "snoopi_deep.jl" # because they are also used for precompile_blockers. # Here we craft them artificially. diff --git a/test/testmodules/Invalidation/Manifest.toml b/test/testmodules/Invalidation/Manifest.toml new file mode 100644 index 00000000..63d4b287 --- /dev/null +++ b/test/testmodules/Invalidation/Manifest.toml @@ -0,0 +1,16 @@ +# This file is machine-generated - editing it directly is not advised + +julia_version = "1.9.0-DEV" +manifest_format = "2.0" +project_hash = "0bc27949c56990fdb6d1c736921aa6799da2d200" + +[[deps.PkgC]] +path = "PkgC" +uuid = "c8e0c308-2b2c-4e17-bb0b-031502754b83" +version = "0.1.0" + +[[deps.PkgD]] +deps = ["PkgC"] +path = "PkgD" +uuid = "3a5fa9f4-f0a4-4856-859e-2bf0c30232a7" +version = "0.1.0" diff --git a/test/testmodules/Invalidation/PkgC/Project.toml b/test/testmodules/Invalidation/PkgC/Project.toml new file mode 100644 index 00000000..95eca3e3 --- /dev/null +++ b/test/testmodules/Invalidation/PkgC/Project.toml @@ -0,0 +1,4 @@ +name = "PkgC" +uuid = "c8e0c308-2b2c-4e17-bb0b-031502754b83" +authors = ["Tim Holy "] +version = "0.1.0" diff --git a/test/testmodules/Invalidation/PkgC/src/PkgC.jl b/test/testmodules/Invalidation/PkgC/src/PkgC.jl new file mode 100644 index 00000000..aef6090f --- /dev/null +++ b/test/testmodules/Invalidation/PkgC/src/PkgC.jl @@ -0,0 +1,6 @@ +module PkgC + +nbits(::Int8) = 8 +nbits(::Int16) = 16 + +end # module PkgC diff --git a/test/testmodules/Invalidation/PkgD/Manifest.toml b/test/testmodules/Invalidation/PkgD/Manifest.toml new file mode 100644 index 00000000..072fd119 --- /dev/null +++ b/test/testmodules/Invalidation/PkgD/Manifest.toml @@ -0,0 +1,10 @@ +# This file is machine-generated - editing it directly is not advised + +julia_version = "1.9.0-DEV" +manifest_format = "2.0" +project_hash = "605498a297918c5643f56853f77795466548669e" + +[[deps.PkgC]] +path = "../PkgC" +uuid = "c8e0c308-2b2c-4e17-bb0b-031502754b83" +version = "0.1.0" diff --git a/test/testmodules/Invalidation/PkgD/Project.toml b/test/testmodules/Invalidation/PkgD/Project.toml new file mode 100644 index 00000000..238e6e9f --- /dev/null +++ b/test/testmodules/Invalidation/PkgD/Project.toml @@ -0,0 +1,7 @@ +name = "PkgD" +uuid = "3a5fa9f4-f0a4-4856-859e-2bf0c30232a7" +authors = ["Tim Holy "] +version = "0.1.0" + +[deps] +PkgC = "c8e0c308-2b2c-4e17-bb0b-031502754b83" diff --git a/test/testmodules/Invalidation/PkgD/src/PkgD.jl b/test/testmodules/Invalidation/PkgD/src/PkgD.jl new file mode 100644 index 00000000..d25be90e --- /dev/null +++ b/test/testmodules/Invalidation/PkgD/src/PkgD.jl @@ -0,0 +1,11 @@ +module PkgD + +using PkgC + +call_nbits(x::Integer) = PkgC.nbits(x) +map_nbits(list) = map(call_nbits, list) +nbits_list() = map_nbits(Integer[Int8(1), Int16(1)]) + +nbits_list() + +end # module PkgD diff --git a/test/testmodules/Invalidation/Project.toml b/test/testmodules/Invalidation/Project.toml new file mode 100644 index 00000000..b9dc0627 --- /dev/null +++ b/test/testmodules/Invalidation/Project.toml @@ -0,0 +1,3 @@ +[deps] +PkgC = "c8e0c308-2b2c-4e17-bb0b-031502754b83" +PkgD = "3a5fa9f4-f0a4-4856-859e-2bf0c30232a7"