From 8b6bc60a04ad5f3ba86851a2ece1ede3a3a8595e Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 19 Aug 2021 10:18:45 -0500 Subject: [PATCH] Improve attribution of delayed invalidations (#261) Improves on #260 by collecting all callees and callers and then doing the attribution at once (finding at least one identifiable callee). This also adds the test's Manifest.toml, which was missing on #260 but not detected because the necessary Julia version had not yet hit nightly. --- src/invalidations.jl | 50 ++++++++++++++++------------ test/snoopr.jl | 50 ++++++++++++++++++++++++++++ test/testmodules/Stale/Manifest.toml | 21 ++++++++++++ 3 files changed, 100 insertions(+), 21 deletions(-) create mode 100644 test/testmodules/Stale/Manifest.toml diff --git a/src/invalidations.jl b/src/invalidations.jl index 9e0b8b17..0cbbbb68 100644 --- a/src/invalidations.jl +++ b/src/invalidations.jl @@ -187,7 +187,7 @@ function Base.show(io::IO, methinvs::MethodInvalidations) sig = root.first if isa(sig, MethodInstance) # "insert_backedges_callee"/"insert_backedges" (delayed) invalidations - printstyled(io, which(sig.specTypes), color = :light_cyan) + printstyled(io, try which(sig.specTypes) catch _ "(unavailable)" end, color = :light_cyan) print(io, " (formerly ", sig.def, ')') else # `sig` (immediate) invalidations @@ -290,27 +290,24 @@ function invalidation_trees(list; exclude_corecompiler::Bool=true) function handle_insert_backedges(list, i, callee) ncovered = 0 + callees = Any[callee] while length(list) >= i+2 && list[i+2] == "insert_backedges_callee" - if isa(callee, Type) - newcallee = list[i+1] - if isa(newcallee, MethodInstance) - callee = newcallee - end - end + push!(callees, list[i+1]) i += 2 end + callers = MethodInstance[] while length(list) >= i+2 && list[i+2] == "insert_backedges" - caller = list[i+=1] - i += 1 - push!(delayed, callee => caller) + push!(callers, list[i+1]) + i += 2 ncovered += 1 end + push!(delayed, callees => callers) @assert ncovered > 0 return i end methodinvs = MethodInvalidations[] - delayed = Pair{Any,MethodInstance}[] # from "insert_backedges" invalidations + delayed = Pair{Vector{Any},Vector{MethodInstance}}[] # from "insert_backedges" invalidations leaf = nothing mt_backedges, backedges, mt_cache, mt_disable = methinv_storage() reason = nothing @@ -418,20 +415,31 @@ function invalidation_trees(list; exclude_corecompiler::Bool=true) end end end - trouble = similar(delayed, 0) - for (callee, caller) in delayed - if isa(callee, MethodInstance) - idx = get(callee2idx, callee.def, nothing) - if idx !== nothing - push!(methodinvs[idx].mt_backedges, callee => caller) - continue + solved = Int[] + for (i, (callees, callers)) in enumerate(delayed) + for callee in callees + if isa(callee, MethodInstance) + idx = get(callee2idx, callee.def, nothing) + if idx !== nothing + for caller in callers + push!(methodinvs[idx].mt_backedges, callee => caller) + end + push!(solved, i) + break + end end end - push!(trouble, callee => caller) end - if !isempty(trouble) + deleteat!(delayed, solved) + if !isempty(delayed) @warn "Could not attribute the following delayed invalidations:" - display(trouble) + for (callees, callers) in delayed + @assert !isempty(callees) # this shouldn't ever happen + printstyled(length(callees) == 1 ? callees[1] : callees; color = :light_cyan) + print(" invalidated ") + printstyled(length(callers) == 1 ? callers[1] : callers; color = :light_yellow) + println() + end end return sort!(methodinvs; by=countchildren) end diff --git a/test/snoopr.jl b/test/snoopr.jl index 87794949..510a17e3 100644 --- a/test/snoopr.jl +++ b/test/snoopr.jl @@ -212,3 +212,53 @@ end @test isempty(filtermod(Outer, trees)) @test length(filtermod(Outer, trees; recursive=true)) == 1 end + +@testset "Delayed invalidations" begin + if 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. + M = @eval Module() begin + fake1(x) = 1 + fake2(x) = fake1(x) + fake3() = nothing + foo() = nothing + @__MODULE__ + end + M.fake2('a') + M.fake3() + callee = methodinstance(M.fake1, (Char,)) + caller = methodinstance(M.fake2, (Char,)) + othercallee = methodinstance(M.fake3, ()) + # failed attribution (no invalidations occurred prior to the backedges invalidations) + invalidations = Any[callee, "insert_backedges_callee", othercallee, "insert_backedges_callee", caller, "insert_backedges"] + pipe = Pipe() + redirect_stdout(pipe) do + @test_logs (:warn, "Could not attribute the following delayed invalidations:") begin + trees = invalidation_trees(invalidations) + @test isempty(trees) + end + end + close(pipe.in) + str = read(pipe.out, String) + @test occursin(r"fake1\(::Char\).*invalidated.*fake2\(::Char\)", str) + + m = which(M.foo, ()) + invalidations = Any[Any[caller, Int32(1), callee, "jl_method_table_insert", m, "jl_method_table_insert"]; invalidations] + tree = @test_nowarn only(invalidation_trees(invalidations)) + @test tree.method == m + @test tree.reason == :inserting + mi1, mi2 = tree.mt_backedges[1] + @test mi1 == callee + @test mi2 == caller + @test Core.MethodInstance(tree.backedges[1]) == callee + io = IOBuffer() + print(io, tree) + str = String(take!(io)) + @test occursin(r"fake1\(x\) in.*formerly fake1\(x\) in", str) + Base.delete_method(callee.def) + print(io, tree) + str = String(take!(io)) + @test occursin(r"\(unavailable\).*formerly fake1\(x\) in", str) + end +end diff --git a/test/testmodules/Stale/Manifest.toml b/test/testmodules/Stale/Manifest.toml new file mode 100644 index 00000000..9ef716b6 --- /dev/null +++ b/test/testmodules/Stale/Manifest.toml @@ -0,0 +1,21 @@ +# This file is machine-generated - editing it directly is not advised + +julia_version = "1.6.3-pre.1" +manifest_format = "2.0" + +[[deps.StaleA]] +path = "StaleA" +uuid = "daf834c3-b832-4a67-a95b-01ec1ffe9b4d" +version = "0.1.0" + +[[deps.StaleB]] +deps = ["StaleA", "StaleC"] +path = "StaleB" +uuid = "af730a9e-e668-4d07-a0f0-de54196c2067" +version = "0.1.0" + +[[deps.StaleC]] +deps = ["StaleA"] +path = "StaleC" +uuid = "f6b5ece7-60fa-49fc-ba7e-b783050e37f1" +version = "0.1.0"