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

Add `report_invalidations` to tabulate invalidations summary #303

Closed charleskawczynski closed 1 year ago

charleskawczynski commented 2 years ago

Hello πŸ‘‹πŸ»! There has been a lot of activity around hunting down invalidations lately, for example:

As mentioned in this comment of the discourse thread, I've added a function called report_invalidations to ReportMetrics.jl which summarizes the output of SnoopCompileCore.@snoopr with quantitative and actionable information.

I'd like to call this function from the julia-invalidations github action. I thought it might be cleaner to add this functionality to SnoopCompile directly (rather than a separate package) since it's relatively light weight (barring PrettyTables) and uses SnoopCompile internals.

The ReportMetrics.jl version of the printed table looks like this:

[ Info: Number of invalidations for SomeWork: 7
[ Info: SomeWork: 6 total method invalidations
β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”
β”‚ <file name>:<line number>         β”‚ Method Name β”‚ Invalidations β”‚ Invalidations % β”‚
β”‚                                   β”‚             β”‚    Number     β”‚     (xα΅’/βˆ‘x)     β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ SnoopCompile.jlirrationals.jl:182 β”‚  BigFloat   β”‚       1       β”‚       50        β”‚
β”‚ SnoopCompile.jlirrationals.jl:182 β”‚  BigFloat   β”‚       1       β”‚       50        β”‚
β”‚ SnoopCompile.jlirrationals.jl:192 β”‚   Float32   β”‚       0       β”‚        0        β”‚
β”‚ SnoopCompile.jlirrationals.jl:191 β”‚   Float64   β”‚       0       β”‚        0        β”‚
β”‚ SnoopCompile.jlirrationals.jl:192 β”‚   Float32   β”‚       0       β”‚        0        β”‚
β”‚ SnoopCompile.jlirrationals.jl:191 β”‚   Float64   β”‚       0       β”‚        0        β”‚
β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜
codecov[bot] commented 2 years ago

Codecov Report

Base: 84.08% // Head: 83.70% // Decreases project coverage by -0.38% :warning:

Coverage data is based on head (5b4a42f) compared to base (dcae62d). Patch coverage: 87.50% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #303 +/- ## ========================================== - Coverage 84.08% 83.70% -0.39% ========================================== Files 17 18 +1 Lines 2162 2221 +59 ========================================== + Hits 1818 1859 +41 - Misses 344 362 +18 ``` | [Impacted Files](https://codecov.io/gh/timholy/SnoopCompile.jl/pull/303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy) | Coverage Ξ” | | |---|---|---| | [src/invalidations.jl](https://codecov.io/gh/timholy/SnoopCompile.jl/pull/303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy#diff-c3JjL2ludmFsaWRhdGlvbnMuamw=) | `74.06% <85.71%> (-3.74%)` | :arrow_down: | | [src/report\_invalidations.jl](https://codecov.io/gh/timholy/SnoopCompile.jl/pull/303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy#diff-c3JjL3JlcG9ydF9pbnZhbGlkYXRpb25zLmps) | `86.66% <86.66%> (ΓΈ)` | | | [src/SnoopCompile.jl](https://codecov.io/gh/timholy/SnoopCompile.jl/pull/303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy#diff-c3JjL1Nub29wQ29tcGlsZS5qbA==) | `100.00% <100.00%> (ΓΈ)` | | | [src/parcel\_snoopi\_deep.jl](https://codecov.io/gh/timholy/SnoopCompile.jl/pull/303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy#diff-c3JjL3BhcmNlbF9zbm9vcGlfZGVlcC5qbA==) | `90.01% <0.00%> (+0.10%)` | :arrow_up: | | [src/parcel\_snoopi.jl](https://codecov.io/gh/timholy/SnoopCompile.jl/pull/303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy#diff-c3JjL3BhcmNlbF9zbm9vcGkuamw=) | `95.00% <0.00%> (+0.85%)` | :arrow_up: | | [SnoopPrecompile/src/SnoopPrecompile.jl](https://codecov.io/gh/timholy/SnoopCompile.jl/pull/303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy#diff-U25vb3BQcmVjb21waWxlL3NyYy9Tbm9vcFByZWNvbXBpbGUuamw=) | `94.11% <0.00%> (+6.61%)` | :arrow_up: | | [SnoopCompileCore/src/snoopi\_deep.jl](https://codecov.io/gh/timholy/SnoopCompile.jl/pull/303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy#diff-U25vb3BDb21waWxlQ29yZS9zcmMvc25vb3BpX2RlZXAuamw=) | `100.00% <0.00%> (+8.00%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tim+Holy)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

charleskawczynski commented 1 year ago

bump πŸ™‚

charleskawczynski commented 1 year ago

Bump! πŸ™‚

timholy commented 1 year ago

I think we could have this, but I'd rather it be loaded under a @require PrettyTables = ... block. Often the best practice is to define the function

"""
Lotsa docs

Using `tabulated_invalidations` requires that you first load the PrettyTables package.
"""
function tabulated_invalidations end      # this defines a function with no methods, but the docstring is available.

as a "stub" in the main package, and then in __init__()

function __init__()
    # other stuff
    @require PettyTables = sha include("tabulated.jl")
end

and put the tabulated_invalidations methods in "tabulated.jl".

charleskawczynski commented 1 year ago

Ok, this now works I think as expected, here's a mwe (assuming some packages are loadable):

import SnoopCompileCore
invalidations = SnoopCompileCore.@snoopr begin
   using ForwardDiff
end;
import SnoopCompile
using PrettyTables # needed for report_invalidations to be defined
SnoopCompile.report_invalidations(;
   invalidations,
   process_filename = x -> last(split(x, ".julia/packages/"))
)

Which produces

β”Œ Warning: Could not attribute the following delayed invalidations:
β”” @ SnoopCompile ~/Dropbox/Caltech/work/dev/clones/forks/SnoopCompile.jl/src/invalidations.jl:552
Any[MethodInstance for eltype(::Type{A} where A<:(NamedTuple{(:init,), _A} where _A<:Tuple{AbstractDict})), MethodInstance for eltype(::Type{T} where T<:Tuple{AbstractDict}), MethodInstance for eltype(::Type{A} where A<:(NamedTuple{(:maxlog, :form), _A} where _A<:Tuple{Int64, Type})), MethodInstance for eltype(::Type{T} where T<:Tuple{Int64, Type})] invalidated Core.MethodInstance[MethodInstance for StaticArrays.var"#s25#170"(::Any, ::Any, ::Any, ::Any, ::Any, ::Any, ::Any, ::Any), MethodInstance for StaticArrays._precompile_()]
Any[MethodInstance for eltype(::Type{T} where T<:Tuple{AbstractDict}), MethodInstance for eltype(::Type{A} where A<:(NamedTuple{(:init,), _A} where _A<:Tuple{AbstractDict}))] invalidated MethodInstance for MacroTools.allbindings(::Any, ::Vector{Any})
[ Info: 233 methods invalidated for 6 functions
β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”
β”‚ <file name>:<line number>                   β”‚ Function Name β”‚ Invalidations β”‚ Invalidations % β”‚
β”‚                                             β”‚               β”‚               β”‚     (xα΅’/βˆ‘x)     β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ ForwardDiff/QdStj/src/dual.jl:427           β”‚ promote_rule  β”‚      209      β”‚       82        β”‚
β”‚ StaticArrays/x7lS0/src/abstractarray.jl:150 β”‚    similar    β”‚      18       β”‚        7        β”‚
β”‚ StaticArrays/x7lS0/src/SizedArray.jl:88     β”‚    convert    β”‚      15       β”‚        6        β”‚
β”‚ StaticArrays/x7lS0/src/mapreduce.jl:253     β”‚   foldl##kw   β”‚       8       β”‚        3        β”‚
β”‚ ForwardDiff/QdStj/src/dual.jl:145           β”‚    isequal    β”‚       3       β”‚        1        β”‚
β”‚ StaticArrays/x7lS0/src/SOneTo.jl:57         β”‚  getproperty  β”‚       3       β”‚        1        β”‚
β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜
charleskawczynski commented 1 year ago

Based on https://github.com/JuliaDiff/ForwardDiff.jl/issues/620#issuecomment-1370119947 and https://github.com/JuliaDiff/ForwardDiff.jl/issues/620#issuecomment-1370142934, should we update how report_invalidations works? cc @ChrisRackauckas @timholy

charleskawczynski commented 1 year ago

I think one last thing that we could do, that would really put this over the top in terms of ease of use, would be if we followed through with https://github.com/CliMA/ReportMetrics.jl/issues/7#issuecomment-1015618744, and made the file / line number hypertext links through PrettyTables (which I think is supported here).

I think this can be a followup feature, though. Thumbs up to open an issue πŸ™‚

timholy commented 1 year ago

Thanks! This is a nice addition. Release of SnoopCompile 2.10.0 is underway.