Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Invalidations.yml #91

Merged
merged 1 commit into from
Sep 1, 2022
Merged

Conversation

ranocha
Copy link
Contributor

@ranocha ranocha commented Sep 1, 2022

@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #91 (e048c4d) into master (0be4cdd) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master       #91   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          384       384           
=========================================
  Hits           384       384           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sethaxen
Copy link
Collaborator

sethaxen commented Sep 1, 2022

Thanks! It looks like we have 270 invalidations in this package. Is this a problem, should we do something about that, and if so, what?

@ranocha
Copy link
Contributor Author

ranocha commented Sep 1, 2022

Well, it would of course be great to fix these invalidations. In my experience, this is quite a manual process. I submitted some tens of PRs in the last days to Julia and the stdlibs - all this takes some time. The SnoopCompile.jl docs are a good place to learn about this and how to fix invalidations: https://timholy.github.io/SnoopCompile.jl/stable/snoopr/

@ranocha
Copy link
Contributor Author

ranocha commented Sep 1, 2022

Concerning "is this a problem?": It basically means that methods that have already been inferred will be invalidated, so the latency of many function calls will increase. The full effect of all of this depends on your workloads and the exact invalidations etc. To fix latency issues with JUlia in the long run definitely requires fixing as many invalidations as possible - all of them in an ideal world.

@sethaxen
Copy link
Collaborator

sethaxen commented Sep 1, 2022

Thanks! I'll see if we can get these invalidations down at all.

@sethaxen sethaxen merged commit 7c606f1 into JuliaGeometry:master Sep 1, 2022
@ranocha ranocha deleted the patch-1 branch September 1, 2022 15:04
@ranocha
Copy link
Contributor Author

ranocha commented Sep 1, 2022

Thanks! It would be great if you could fix some of these invalidations. Feel free to ask questions if something is not clear.

@sethaxen
Copy link
Collaborator

sethaxen commented Sep 1, 2022

Hmm, if I'm reading this correctly, our invalidations are all caused by the ChainRulesCore and DualNumbers dependencies, not by us:

julia> using SnoopCompileCore

julia> invalidations = @snoopr using Quaternions

julia> using SnoopCompile

julia> trees = invalidation_trees(invalidations)

julia> trees
4-element Vector{SnoopCompile.MethodInvalidations}:
 inserting *(::Any, ::ChainRulesCore.ZeroTangent) in ChainRulesCore at /home/sethaxen/.julia/packages/ChainRulesCore/ctmSK/src/tangent_arithmetic.jl:105 invalidated:
   mt_backedges: 1: signature Tuple{typeof(*), String, Any} triggered MethodInstance for (::Test.var"#7#9")(::Any) (0 children)
                 2: signature Tuple{typeof(*), String, Any} triggered MethodInstance for (::Test.var"#8#10")(::Any) (0 children)

 inserting isequal(x::Number, z::DualNumbers.Dual) in DualNumbers at /home/sethaxen/.julia/packages/DualNumbers/5knFX/src/dual.jl:179 invalidated:
   backedges: 1: superseding isequal(x, y) in Base at operators.jl:140 with MethodInstance for isequal(::Int64, ::Any) (3 children)
   37 mt_cache

 inserting convert(::Type{<:ChainRulesCore.Thunk}, a::ChainRulesCore.AbstractZero) in ChainRulesCore at /home/sethaxen/.julia/packages/ChainRulesCore/ctmSK/src/tangent_types/thunks.jl:213 invalidated:
   backedges: 1: superseding convert(::Type{Union{}}, x) in Base at essentials.jl:213 with MethodInstance for convert(::Core.TypeofBottom, ::Any) (13 children)

 inserting tail(t::ChainRulesCore.Tangent{<:NamedTuple{<:Any, <:Tuple{}}}) in ChainRulesCore at /home/sethaxen/.julia/packages/ChainRulesCore/ctmSK/src/tangent_types/tangent.jl:110 invalidated:
   mt_backedges: 1: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base._cshp(::Int64, ::Tuple{Bool}, ::Tuple{Int64}, ::Any) (0 children)
                 2: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base._cshp(::Int64, ::Tuple{Bool}, ::Tuple{Any, Vararg{Any}}, ::Any) (0 children)
                 3: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base.Iterators._zip_isdone(::Tuple, ::Any) (0 children)
                 4: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base.Iterators._zip_iterate_some(::Tuple, ::Any, ::Tuple{Missing, Vararg{Any}}, ::Missing) (0 children)
                 5: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base.Iterators._zip_iterate_some(::Tuple, ::Any, ::Tuple{Any, Vararg{Any}}, ::Missing) (0 children)
                 6: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base.Iterators._zip_iterate_some(::Tuple, ::Any, ::Tuple{Bool, Vararg{Any}}, ::Bool) (0 children)
                 7: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base.Iterators._zip_iterate_some(::Tuple, ::Any, ::Tuple{Any, Vararg{Any}}, ::Bool) (0 children)
                 8: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for iterate(::Base.Iterators.Enumerate{Vector{VersionNumber}}, ::Any) (0 children)
                 9: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base.tail(::NamedTuple{names}) where names (335 children)
   15 mt_cache

@ranocha
Copy link
Contributor Author

ranocha commented Sep 1, 2022

Yes, but it doesn't matter who causes the invalidations. If they are caused by some of your dependencies, they will affect the latency of you package, too. Usually, fixing invalidations like these requires fixing some type instabilities in Julia and its stdlibs.

The last one

9: signature Tuple{typeof(Base.tail), Any} triggered MethodInstance for Base.tail(::NamedTuple{names}) where names (335 children)

looks particularly bad (because of the 335 children) - maybe worth trying to fix it or filing an issue at the package causing this?

@sethaxen
Copy link
Collaborator

sethaxen commented Sep 1, 2022

Seems there's already an issue for it: JuliaDiff/ChainRulesCore.jl#576

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants