Make Cthulhu.jl a weakdep, implement CthulhuExt.jl (#381)

timholy · Apr 16, 2024 · ae295bc · ae295bc
1 parent 91d3685
commit ae295bc
Show file tree

Hide file tree

Showing 10 changed files with 52 additions and 39 deletions.
diff --git a/Project.toml b/Project.toml
@@ -5,7 +5,6 @@ version = "2.10.8"
 
 [deps]
 AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c"
-Cthulhu = "f68482b8-f384-11e8-15f7-abe071a5a75f"
 FlameGraphs = "08572546-2f56-4bcf-ba4e-bab62c3a3f89"
 InteractiveUtils = "b77e0a4c-d291-57a0-90e8-8db25a27a240"
 OrderedCollections = "bac558e1-5e72-5ebc-8fee-abe8a469f55d"
@@ -18,10 +17,12 @@ SnoopCompileCore = "e2b509da-e806-4183-be48-004708413034"
 YAML = "ddb6d928-2868-570f-bddf-ab3f9cf99eb6"
 
 [weakdeps]
+Cthulhu = "f68482b8-f384-11e8-15f7-abe071a5a75f"
 JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
 
 [extensions]
-JETExt = "JET"
+CthulhuExt = "Cthulhu"
+JETExt = ["JET", "Cthulhu"]
 
 [compat]
 AbstractTrees = "0.3, 0.4"
@@ -35,6 +36,7 @@ julia = "1"
 
 [extras]
 ColorTypes = "3da002f7-5984-5a60-b8a6-cbb66c0b333f"
+Cthulhu = "f68482b8-f384-11e8-15f7-abe071a5a75f"
 Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
 FixedPointNumbers = "53c48c17-4a7d-5ca2-90c5-79b7896eea93"
 JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
@@ -45,4 +47,4 @@ Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [targets]
-test = ["ColorTypes", "PrettyTables", "Documenter", "FixedPointNumbers", "JET", "MethodAnalysis", "Pkg", "Random", "Test"]
+test = ["ColorTypes", "Cthulhu", "PrettyTables", "Documenter", "FixedPointNumbers", "JET", "MethodAnalysis", "Pkg", "Random", "Test"]
diff --git a/docs/src/jet.md b/docs/src/jet.md
@@ -91,7 +91,7 @@ As always, you need to do the data collection in a fresh session where the calls
 After restarting Julia, we can do this:
 
 ```julia
-julia> using SnoopCompile
+julia> using SnoopCompile, JET, Cthulhu
 
 julia> using JET # this is necessary to enable the integration
 
@@ -137,5 +137,5 @@ Because SnoopCompile collected the runtime-dispatched `sum` call, we can pass it
 `report_callees` filters those calls which generate JET reports, allowing you to focus on potential errors.
 
 !!! note
-    JET integration is enabled only if JET.jl has been loaded into your main session.
-    This is why there's the `using JET` statement included in the example given.
+    JET integration is enabled only if JET.jl _and_ Cthulhu.jl have been loaded into your main session.
+    This is why there's the `using JET, Cthulhu` statement included in the example given.
diff --git a/docs/src/snoopi.md b/docs/src/snoopi.md
@@ -343,7 +343,7 @@ store its precompiled form.
 
 How to fix this?
 Fundamentally, the problem is that `vcat` call: if we can write `index_midsum` in a way so that inference succeeds, then all these problems go away.
-(You can use `ascend(mi)`, where `mi` was obtained above, to discover that `__cat` gets called from `vcat`. See [ascend](@ref) for more information.)
+(You can use `ascend(mi)`, with Cthulhu.jl, where `mi` was obtained above, to discover that `__cat` gets called from `vcat`. See [`Cthulhu.ascend`](@ref ascend-itrig) for more information.)
 It turns out that `vcat` is inferrable if all the arguments have the same type, so just changing `vcat(0, a)` to `vcat([zero(eltype(a))], a)` fixes the problem.
 (Alternatively, you could make a copy and then use `pushfirst!`.)
 In a fresh Julia session:

diff --git a/docs/src/snoopi_deep_analysis.md b/docs/src/snoopi_deep_analysis.md
@@ -159,7 +159,7 @@ julia> mtrig.itrigs[1]
 Inference triggered to call MethodInstance for (::Base.var"#cat_t##kw")(::NamedTuple{(:dims,), Tuple{Val{1}}}, ::typeof(Base.cat_t), ::Type{Int64}, ::UnitRange{Int64}, ::Vararg{Any, N} where N) from _cat (./abstractarray.jl:1630) inlined into MethodInstance for makeobjects() (/home/tim/.julia/dev/SnoopCompile/examples/OptimizeMe.jl:37)
 ```
 
-This is useful if you want to analyze a method via [`ascend`](@ref ascend-itrig).
+This is useful if you want to analyze a method via [`Cthulhu.ascend`](@ref ascend-itrig).
 `Method`-based triggers, which may aggregate many different individual triggers, are particularly useful mostly because tools like [Cthulhu.jl](https://github.com/JuliaDebug/Cthulhu.jl) show you the inference results for the entire `MethodInstance`, allowing you to fix many different inference problems at once.
 
 ### Trigger trees
@@ -281,10 +281,10 @@ MethodInstance for combine_eltypes(::Type, ::Tuple{Vector{Any}})
 
 julia> suggest(itree.children[2])
 ./broadcast.jl:905: regular invoke (perhaps precompile lotsa_containers() at OptimizeMe.jl:14)
-├─ ./broadcast.jl:740: I've got nothing to say for MethodInstance for return_type(::Any, ::Any) consider `stacktrace(itrig)` or `ascend(itrig)`
-├─ ./broadcast.jl:740: I've got nothing to say for MethodInstance for return_type(::Any, ::Any, ::UInt64) consider `stacktrace(itrig)` or `ascend(itrig)`
-├─ ./broadcast.jl:740: I've got nothing to say for MethodInstance for return_type(::Core.Compiler.NativeInterpreter, ::Any, ::Any) consider `stacktrace(itrig)` or `ascend(itrig)`
-├─ ./broadcast.jl:740: I've got nothing to say for MethodInstance for contains_is(::Core.SimpleVector, ::Any) consider `stacktrace(itrig)` or `ascend(itrig)`
+├─ ./broadcast.jl:740: I've got nothing to say for MethodInstance for return_type(::Any, ::Any) consider `stacktrace(itrig)` or `Cthulhu.ascend(itrig)`
+├─ ./broadcast.jl:740: I've got nothing to say for MethodInstance for return_type(::Any, ::Any, ::UInt64) consider `stacktrace(itrig)` or `Cthulhu.ascend(itrig)`
+├─ ./broadcast.jl:740: I've got nothing to say for MethodInstance for return_type(::Core.Compiler.NativeInterpreter, ::Any, ::Any) consider `stacktrace(itrig)` or `Cthulhu.ascend(itrig)`
+├─ ./broadcast.jl:740: I've got nothing to say for MethodInstance for contains_is(::Core.SimpleVector, ::Any) consider `stacktrace(itrig)` or `Cthulhu.ascend(itrig)`
 └─ ./broadcast.jl:740: non-inferrable call, perhaps annotate combine_eltypes(f, args::Tuple) in Base.Broadcast at broadcast.jl:740 with type MethodInstance for promote_typejoin_union(::Type{Main.OptimizeMe.Container})
    If a noninferrable argument is a type or function, Julia's specialization heuristics may be responsible.
    immediate caller(s):
@@ -721,7 +721,7 @@ Later, we'll see how `parcel` can generate such precompile directives automatica
 
 Another `show` `MethodInstance`, `show(::IOContext{IOBuffer}, ::Tuple{String, Int64})`, seems too specific to be worth worrying about, so we call it quits here.
 
-### [Advanced analysis: `ascend`](@id ascend-itrig)
+### [Advanced analysis: `Cthulhu.ascend`](@id ascend-itrig)
 
 One thing that hasn't yet been covered is that when you really need more insight, you can use `ascend`:
 

diff --git a/ext/CthulhuExt.jl b/ext/CthulhuExt.jl
@@ -0,0 +1,24 @@
+module CthulhuExt
+    import Cthulhu
+    using Core: MethodInstance
+    using SnoopCompile: InstanceNode, TriggerNode, Suggested, InferenceTrigger, countchildren
+
+
+    # Originally from invalidations.jl
+    Cthulhu.backedges(node::InstanceNode) = sort(node.children; by=countchildren, rev=true)
+    Cthulhu.method(node::InstanceNode) = Cthulhu.method(node.mi)
+    Cthulhu.specTypes(node::InstanceNode) = Cthulhu.specTypes(node.mi)
+    Cthulhu.instance(node::InstanceNode) = node.mi
+
+    # Originally from parcel_snoopi_deep.jl
+
+    Cthulhu.descend(itrig::InferenceTrigger; kwargs...) = descend(callerinstance(itrig); kwargs...)
+    Cthulhu.instance(itrig::InferenceTrigger) = MethodInstance(itrig.node)
+    Cthulhu.method(itrig::InferenceTrigger) = Method(itrig.node)
+    Cthulhu.specTypes(itrig::InferenceTrigger) = Cthulhu.specTypes(Cthulhu.instance(itrig))
+    Cthulhu.backedges(itrig::InferenceTrigger) = (itrig.callerframes,)
+    Cthulhu.nextnode(itrig::InferenceTrigger, edge) = (ret = callingframe(itrig); return isempty(ret.callerframes) ? nothing : ret)
+
+    Cthulhu.ascend(node::TriggerNode) = ascend(node.itrig)
+    Cthulhu.ascend(s::Suggested) = ascend(s.itrig)
+end
diff --git a/ext/JETExt.jl b/ext/JETExt.jl
@@ -3,12 +3,12 @@ module JETExt
 @static if isdefined(Base, :get_extension)
     import SnoopCompile: report_callee, report_caller, report_callees
     using SnoopCompile: SnoopCompile, InferenceTrigger, callerinstance
-    using SnoopCompile.Cthulhu: specTypes
+    using Cthulhu: specTypes
     using JET: report_call, get_reports
 else
     import ..SnoopCompile: report_callee, report_caller, report_callees
     using ..SnoopCompile: SnoopCompile, InferenceTrigger, callerinstance
-    using ..SnoopCompile.Cthulhu: specTypes
+    using ..Cthulhu: specTypes
     using ..JET: report_call, get_reports
 end
 

diff --git a/src/SnoopCompile.jl b/src/SnoopCompile.jl
@@ -25,7 +25,7 @@ you should prefer them above the more limited tools available on earlier version
 - `invalidation_trees`: organize invalidation data into trees
 - `filtermod`: select trees that invalidate methods in particular modules
 - `findcaller`: find a path through invalidation trees reaching a particular method
-- `ascend`: interactive analysis of an invalidation tree
+- `ascend`: interactive analysis of an invalidation tree (with Cthulhu.jl)
 
 ### LLVM
 
@@ -40,7 +40,7 @@ you should prefer them above the more limited tools available on earlier version
 - `accumulate_by_source`: aggregate list items by their source
 - `inference_triggers`: extract data on the triggers of inference
 - `callerinstance`, `callingframe`, `skiphigherorder`, and `InferenceTrigger`: manipulate stack frames from `inference_triggers`
-- `ascend`: interactive analysis of an inference-triggering call chain
+- `ascend`: interactive analysis of an inference-triggering call chain (with Cthulhu.jl)
 - `runtime_inferencetime`: profile-guided deoptimization
 """
 module SnoopCompile
@@ -110,7 +110,7 @@ export read_snoopl
 
 if isdefined(SnoopCompileCore, Symbol("@snoopr"))
     include("invalidations.jl")
-    export @snoopr, uinvalidated, invalidation_trees, filtermod, findcaller, ascend
+    export @snoopr, uinvalidated, invalidation_trees, filtermod, findcaller
 end
 
 if isdefined(SnoopCompileCore, Symbol("@snoopr")) && isdefined(SnoopCompileCore, Symbol("@snoopi_deep"))
@@ -129,7 +129,10 @@ function __init__()
         @require PrettyTables = "08abe8d2-0d0c-5749-adfa-8a2ac140af0d" include("report_invalidations.jl")
     end
     if isdefined(SnoopCompile, :report_callee) && !isdefined(Base, :get_extension)
-        @require JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b" include("../ext/JETExt.jl")
+        @require Cthulhu = "f68482b8-f384-11e8-15f7-abe071a5a75f" begin
+            include("../ext/CthulhuExt.jl")
+            @require JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b" include("../ext/JETExt.jl")
+        end
     end
     return nothing
 end

diff --git a/src/invalidations.jl b/src/invalidations.jl
@@ -1,6 +1,4 @@
-using Cthulhu
-
-export uinvalidated, invalidation_trees, filtermod, findcaller, ascend
+export uinvalidated, invalidation_trees, filtermod, findcaller
 
 const have_verify_methods = Base.VERSION >= v"1.9.0-DEV.1512" || Base.VERSION >= v"1.8.4"
 
@@ -852,10 +850,3 @@ function findcaller(meth::Method, node::InstanceNode)
 end
 
 findcaller(meth::Method, mi::MethodInstance) = mi.def == meth ? mi : nothing
-
-# Cthulhu integration
-
-Cthulhu.backedges(node::InstanceNode) = sort(node.children; by=countchildren, rev=true)
-Cthulhu.method(node::InstanceNode) = Cthulhu.method(node.mi)
-Cthulhu.specTypes(node::InstanceNode) = Cthulhu.specTypes(node.mi)
-Cthulhu.instance(node::InstanceNode) = node.mi
diff --git a/src/parcel_snoopi_deep.jl b/src/parcel_snoopi_deep.jl
@@ -6,7 +6,6 @@ using AbstractTrees
 using Core.Compiler.Timings: InferenceFrameInfo
 using SnoopCompileCore: InferenceTiming, InferenceTimingNode, inclusive, exclusive
 using Profile
-using Cthulhu
 
 const InferenceNode = Union{InferenceFrameInfo,InferenceTiming,InferenceTimingNode}
 
@@ -717,6 +716,8 @@ julia> itrigs = inference_triggers(tinf)
 ```
 julia> edit(itrigs[1])     # opens an editor at the spot in the caller
 
+julia> using Cthulhu
+
 julia> ascend(itrigs[2])   # use Cthulhu to inspect the stacktrace (caller is the second item in the trace)
 Choose a call for analysis (q to quit):
  >   double(::Float64)
@@ -889,12 +890,6 @@ end
 AbstractTrees.children(tinf::InferenceTimingNode) = tinf.children
 
 InteractiveUtils.edit(itrig::InferenceTrigger) = edit(Location(itrig.callerframes[end]))
-Cthulhu.descend(itrig::InferenceTrigger; kwargs...) = descend(callerinstance(itrig); kwargs...)
-Cthulhu.instance(itrig::InferenceTrigger) = MethodInstance(itrig.node)
-Cthulhu.method(itrig::InferenceTrigger) = Method(itrig.node)
-Cthulhu.specTypes(itrig::InferenceTrigger) = Cthulhu.specTypes(Cthulhu.instance(itrig))
-Cthulhu.backedges(itrig::InferenceTrigger) = (itrig.callerframes,)
-Cthulhu.nextnode(itrig::InferenceTrigger, edge) = (ret = callingframe(itrig); return isempty(ret.callerframes) ? nothing : ret)
 
 # JET integrations are implemented lazily
 "To use `report_caller` do `using JET`"
@@ -982,7 +977,6 @@ end
 
 InteractiveUtils.edit(node::TriggerNode) = edit(node.itrig)
 Base.stacktrace(node::TriggerNode) = stacktrace(node.itrig)
-Cthulhu.ascend(node::TriggerNode) = ascend(node.itrig)
 
 ### tagged trigger lists
 # good for organizing a collection of related triggers
@@ -1323,7 +1317,6 @@ unspec(s::Suggestion) = s ∈ (UnspecCall, UnspecType, CalleeVariable)
 unspec(s::Suggested)  = any(unspec, s.categories)
 
 Base.stacktrace(s::Suggested) = stacktrace(s.itrig)
-Cthulhu.ascend(s::Suggested) = ascend(s.itrig)
 InteractiveUtils.edit(s::Suggested) = edit(s.itrig)
 
 """

diff --git a/test/snoopi_deep.jl b/test/snoopi_deep.jl
@@ -960,7 +960,7 @@ end
 end
 
 if VERSION >= v"1.7"
-    using JET
+    using JET, Cthulhu
 end
 @static if VERSION >= v"1.7"
     @testset "JET integration" begin