search

timholy / SnoopCompile.jl

Provide insights about latency (TTFX) for Julia packages
https://timholy.github.io/SnoopCompile.jl/dev/
Other
309 stars 48 forks source link

JET integration #253

Closed timholy closed 2 years ago

timholy commented 3 years ago

This allows one to generate reports on both the callee and caller of an inference trigger.

timholy commented 3 years ago

Demo:

julia> f(c) = sum(c[1])
f (generic function with 1 method)

julia> c = Any[Any[1,2,3]]
1-element Vector{Any}:
 Any[1, 2, 3]

julia> using SnoopCompile
[ Info: Precompiling SnoopCompile [aa65fe97-06da-5843-b5b1-d5d13cad87d2]

julia> tinf = @snoopi_deep f(c)
InferenceTimingNode: 0.032447/0.037888 on Core.Compiler.Timings.ROOT() with 2 direct children

julia> using JET

julia> @report_call f(c)
No errors !
(Any, 0)

julia> itrigs = inference_triggers(tinf)
2-element Vector{InferenceTrigger}:
 Inference triggered to call f(::Vector{Any}) from eval (./boot.jl:373) inlined into REPL.eval_user_input(::Any, ::REPL.REPLBackend) (/home/tim/src/julia-master/usr/share/julia/stdlib/v1.8/REPL/src/REPL.jl:150)
 Inference triggered to call sum(::Vector{Any}) from f (./REPL[1]:1) with specialization f(::Vector{Any})

julia> report_callee(itrigs[1])
No errors !
(Any, 0)

julia> report_callee(itrigs[2])
═════ 1 possible error found ═════
┌ @ reducedim.jl:889 Base.#sum#738(Base.:, Base.pairs(Core.NamedTuple()), #self#, a)
│┌ @ reducedim.jl:889 Base._sum(a, dims)
││┌ @ reducedim.jl:893 Base.#_sum#740(Base.pairs(Core.NamedTuple()), #self#, a, _3)
│││┌ @ reducedim.jl:893 Base._sum(Base.identity, a, Base.:)
││││┌ @ reducedim.jl:894 Base.#_sum#741(Base.pairs(Core.NamedTuple()), #self#, f, a, _4)
│││││┌ @ reducedim.jl:894 Base.mapreduce(f, Base.add_sum, a)
││││││┌ @ reducedim.jl:322 Base.#mapreduce#731(Base.:, Base._InitialValue(), #self#, f, op, A)
│││││││┌ @ reducedim.jl:322 Base._mapreduce_dim(f, op, init, A, dims)
││││││││┌ @ reducedim.jl:330 Base._mapreduce(f, op, Base.IndexStyle(A), A)
│││││││││┌ @ reduce.jl:402 Base.mapreduce_empty_iter(f, op, A, Base.IteratorEltype(A))
││││││││││┌ @ reduce.jl:353 Base.reduce_empty_iter(Base.MappingRF(f, op), itr, ItrEltype)
│││││││││││┌ @ reduce.jl:357 Base.reduce_empty(op, Base.eltype(itr))
││││││││││││┌ @ reduce.jl:331 Base.mapreduce_empty(Base.getproperty(op, :f), Base.getproperty(op, :rf), _)
│││││││││││││┌ @ reduce.jl:345 Base.reduce_empty(op, T)
││││││││││││││┌ @ reduce.jl:322 Base.reduce_empty(Base.+, _)
│││││││││││││││┌ @ reduce.jl:313 Base.zero(_)
││││││││││││││││┌ @ missing.jl:106 Base.throw(Base.MethodError(Base.zero, Core.tuple(Base.Any)))
│││││││││││││││││ MethodError: no method matching zero(::Type{Any})
││││││││││││││││└──────────────────
(Any, 1)

julia> report_caller(itrigs[2])
No errors !
(Any, 0)

CC @aviatesk

I might leave this up long enough for folks to experiment with it before merging. An alternative approach is to split out @snoopi_deep and a small portion of its infrastructure into a new package, ProfileInference.jl, which could then be imported by both SnoopCompile & JET. @aviatesk, I'm perfectly happy with either approach, please let me know what you prefer.

codecov[bot] commented 3 years ago

Codecov Report

Merging #253 (0ef71df) into master (2ff6c69) will increase coverage by 0.12%. The diff coverage is 90.90%.

:exclamation: Current head 0ef71df differs from pull request most recent head 9469a1c. Consider uploading reports for the commit 9469a1c to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
+ Coverage   86.82%   86.95%   +0.12%     
==========================================
  Files          16       16              
  Lines        2012     2023      +11     
==========================================
+ Hits         1747     1759      +12     
+ Misses        265      264       -1
Impacted Files Coverage Δ
SnoopCompileCore/src/snoopi_deep.jl 92.00% <ø> (ø)
src/SnoopCompile.jl 100.00% <ø> (ø)
src/parcel_snoopi_deep.jl 89.77% <90.90%> (+0.22%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2ff6c69...9469a1c. Read the comment docs.

aviatesk commented 3 years ago

Great ! I think this would be a simple but very nice first step toward analyzing Julia programs using both static/dynamic information.

An alternative approach is to split out @snoopi_deep and a small portion of its infrastructure into a new package, ProfileInference.jl, which could then be imported by both SnoopCompile & JET

Hmm, I'm still not too sure how SnoopCompile and JET want to use "ProfileInference". My naive understanding is, "ProfileInference" (or SnoopCompile if we go with the current approach) will provide user-facing utilities on top of functionalities of SnoopCompile and JET, and so there is no need for those two packages to load "ProfileInference" ?

timholy commented 3 years ago

One option would be possible to extract:

and put it in a standalone package. It might need a few more things, but if you just want to capture the raw data and then write your own handling, that's the minimum. But it seems likely that you'd soon want https://github.com/timholy/SnoopCompile.jl/blob/1abf0a2ae1f64fb8eb93175b9aa7adf16b9530f8/src/parcel_snoopi_deep.jl#L590-L877, and at that point it might make more sense to just have SnoopCompile be a wrapper. It's already a snoop/Cthulhu/AbstractTrees integration package, so this would not be weird.

Anyway, the point is, let me know what you think makes the most sense.

aviatesk commented 3 years ago

Thanks so much for your explanation.

Okay, so to me it seems more natural to have this integration in SnoopCompile. From JET's viewpoint, it's much simpler if it just focuses on its (half-)static analysis feature, while it just makes sense to have external package (SnoopCompile) that combines JET's static analysis and runtime information. If you think you want to make SnoopCompile focus on "inference timing" tools, then it also makes sense to have a separate package that is dedicated to execute a program with collecting entry points for static analysis, but as far as I understand, SnoopCompile is already such a package that combines both static and runtime information, right ?

Just a random thought, in the long run, we may want another, more automated way to combine static analysis and runtime execution, e.g. we can use JuliaInterpreter/Cassette/compiler-plugin to automatically invoke "static" analysis provided by SnoopCompile/JET/whatsoever in the middle of program execution and collect some information. But also, we know (basically-)interactive inspections like SnoopCompile/Cthulhu are the most accurate and efficient tools we have at this moment.