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

Fix some invalidations from extending ==(::SomeType, ::Any) #36255

Merged
merged 4 commits into from
Jun 13, 2020

Conversation

timholy
Copy link
Member

@timholy timholy commented Jun 12, 2020

I spent some time investigating #36005 as a way of exercising the new tools that I'm in the process of releasing. This PR, plus some unsubmitted fixes to JuliaInterpreter, LoweredCodeUtils, and Revise (which were loaded when the tests below were run) fixes more than 1000 invalidations, but some of the trickier ones remain to be addressed.

First, let's see where we started:

julia> using SnoopCompile

julia> trees = invalidation_trees(@snoopr using Plumber)
2-element Array{SnoopCompile.MethodInvalidations,1}:
 insert ==(expr, capture::ExprManipulation.AbstractCapture) in ExprManipulation at /home/tim/.julia/packages/ExprManipulation/NLUoZ/src/capture.jl:49 invalidated:
   backedges:  1: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Array{Any,1}, ::Any) (1 children) more specific
               2: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Tuple{Union{Ptr{Nothing}, Base.InterpreterIP},Symbol}, ::Any) (1 children) more specific
               3: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::typeof(display), ::Any) (2 children) more specific
               4: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::typeof(show), ::Any) (2 children) more specific
               5: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Revise.RelocatableExpr, ::Any) (2 children) more specific
               6: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::VersionNumber, ::Any) (4 children) more specific
               7: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::GlobalRef, ::Any) (4 children) more specific
               8: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Pkg.BinaryPlatforms.Platform, ::Any) (5 children) more specific
               9: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Base.SHA1, ::Any) (5 children) more specific
              10: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Array{Char,1}, ::Any) (5 children) more specific
              11: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Dict, ::Any) (5 children) more specific
              12: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::REPL.LineEdit.var"#74#105"{REPL.LineEdit.HistoryPrompt}, ::Any) (6 children) more specific
              13: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::REPL.LineEdit.var"#75#106"{REPL.LineEdit.HistoryPrompt}, ::Any) (6 children) more specific
              14: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Dict{String,Union{Base.SHA1, String}}, ::Any) (6 children) more specific
              15: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Regex, ::Any) (7 children) more specific
              16: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Array{String,1}, ::Any) (12 children) more specific
              17: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Module, ::Any) (17 children) more specific
              18: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Base.UUID, ::Any) (17 children) more specific
              19: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::REPL.REPLCompletions.Completion, ::Any) (17 children) more specific
              20: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Function, ::Any) (18 children) more specific
              21: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Method, ::Any) (23 children) more specific
              22: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Core.TypeName, ::Any) (26 children) more specific
              23: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Distributed.RRID, ::Any) (50 children) more specific
              24: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Nothing, ::Any) (100 children) more specific
              25: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Symbol, ::Any) (327 children) more specific

 insert ==(capture::ExprManipulation.AbstractCapture, expr) in ExprManipulation at /home/tim/.julia/packages/ExprManipulation/NLUoZ/src/capture.jl:48 invalidated:
   backedges:  1: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Dict{Base.UUID,Dict{String,Union{Base.SHA1, String}}}) (1 children) more specific
               2: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::JuliaInterpreter.SSAValue) (2 children) more specific
               3: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Core._abstracttype)) (2 children) more specific
               4: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(+)) (3 children) more specific
               5: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(-)) (3 children) more specific
               6: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(pairs)) (3 children) more specific
               7: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(iterate)) (3 children) more specific
               8: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Core.apply_type)) (3 children) more specific
               9: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Core.kwfunc)) (3 children) more specific
              10: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(merge)) (3 children) more specific
              11: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::JuliaInterpreter.SlotNumber) (3 children) more specific
              12: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Distributed.RRID) (4 children) more specific
              13: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Dict{Base.SHA1,Union{Base.SHA1, String}}) (4 children) more specific
              14: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::REPL.LineEdit.Prompt) (4 children) more specific
              15: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::REPL.REPLCompletions.Completion) (4 children) more specific
              16: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Dict) (4 children) more specific
              17: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Array{String,1}) (5 children) more specific
              18: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::VersionNumber) (5 children) more specific
              19: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Base.SHA1) (7 children) more specific
              20: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Pkg.BinaryPlatforms.Linux) (7 children) more specific
              21: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Pkg.BinaryPlatforms.MacOS) (7 children) more specific
              22: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Pkg.BinaryPlatforms.Windows) (7 children) more specific
              23: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(getproperty)) (7 children) more specific
              24: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Core._typebody!)) (10 children) more specific
              25: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Revise.swap_watch_package)) (10 children) more specific
              26: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Module) (12 children) more specific
              27: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Core._setsuper!)) (13 children) more specific
              28: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Core._primitivetype)) (15 children) more specific
              29: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Core.iterate)) (17 children) more specific
              30: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::LibGit2.Error.Code) (18 children) more specific
              31: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(Core.Typeof)) (19 children) more specific
              32: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Base.UUID) (20 children) more specific
              33: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::typeof(tuple)) (41 children) more specific
              34: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Distributed.WorkerState) (83 children) more specific
              35: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Nothing) (235 children) more specific
              36: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Any, ::Symbol) (1764 children) more specific
   4 mt_cache

So, all invalidations stem from these two methods. Methods like that show up fairly frequently, so this is not an atypical case.

This PR (and the ones in JuliaInterpreter etc) focus on the big culprit, ==(::Any, ::Symbol), reducing the total number of children from 1764 to 93. So, major progress.

It does not, however, fix the issue mentioned in #36005. That turns out to be a consequence of invalidating Base.require(::Module, ::Symbol). You can see the call chain that leads here like this (requires the just-tagged SnoopCompile 1.4):

julia> m = which(Base.require, (Module, Symbol))
require(into::Module, mod::Symbol) in Base at loading.jl:887

julia> findcaller(m, trees)
insert ==(expr, capture::ExprManipulation.AbstractCapture) in ExprManipulation at /home/tim/.julia/packages/ExprManipulation/NLUoZ/src/capture.jl:49 invalidated:
   backedges: 1: superseding ==(x, y) in Base at operators.jl:83 with MethodInstance for ==(::Symbol, ::Any) (16 children) more specific


julia> node = ans.backedges[1]
MethodInstance for ==(::Symbol, ::Any) at depth 0 with 16 children

julia> show(node; maxdepth=100, minchildren=0)
MethodInstance for ==(::Symbol, ::Any) (16 children)
 MethodInstance for isequal(::Symbol, ::Any) (15 children)
  MethodInstance for ht_keyindex2!(::Dict{Any,Int64}, ::Symbol) (14 children)
   MethodInstance for get!(::Base.var"#137#138"{Int64}, ::Dict{Any,Int64}, ::Symbol) (13 children)
    MethodInstance for get!(::Dict{Any,Int64}, ::Symbol, ::Int64) (12 children)
     MethodInstance for #handle_message#2(::Int64, ::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::typeof(Base.CoreLogging.handle_message), ::Base.CoreLogging.SimpleLogger, ::Base.CoreLogging.LogLevel, ::String, ::Module, ::Symbol, ::Symbol, ::String, ::Int64) (11 children)
      MethodInstance for (::Base.CoreLogging.var"#handle_message##kw")(::NamedTuple{(:maxlog,),Tuple{Int64}}, ::typeof(Base.CoreLogging.handle_message), ::Base.CoreLogging.SimpleLogger, ::Base.CoreLogging.LogLevel, ::String, ::Module, ::Symbol, ::Symbol, ::String, ::Int64) (10 children)
       MethodInstance for Base.FileRedirect(::String, ::Bool) (9 children)
        MethodInstance for Base.FileRedirect(::AbstractString, ::Bool) (8 children)
         MethodInstance for redir_err_append(::Cmd, ::AbstractString) (7 children)
          MethodInstance for #pipeline#590(::Nothing, ::Nothing, ::Any, ::Bool, ::typeof(pipeline), ::Cmd) (6 children)
           MethodInstance for (::Base.var"#pipeline##kw")(::NamedTuple{(:stderr,),_A} where _A<:Tuple, ::typeof(pipeline), ::Cmd) (5 children)
            MethodInstance for create_expr_cache(::String, ::String, ::Array{Pair{Base.PkgId,UInt64},1}, ::Nothing) (4 children)
             MethodInstance for compilecache(::Base.PkgId, ::String) (3 children)
              MethodInstance for _require(::Base.PkgId) (2 children)
               MethodInstance for require(::Base.PkgId) (1 children)
                MethodInstance for require(::Module, ::Symbol) (0 children)

(To clarify, this is just one "path" stemming from ==(::Symbol, ::Any), accounting for 16 of the 327 children. It's just the path that first reaches require.)

Lots of similar invalidations pass through ht_keyindex and ht_keyindex2. Anything that uses a Dict{Any,K}() and then uses a Symbol as a key is vulnerable to this. I am not quite sure how to fix this, if we indeed want to fix it. Here are some options:

  1. Enforce runtime-dispatch with atsign-invoke #35901 or an automated version (like that described in Summary of non-ambiguous patterns of invalidation #35922)
  2. a trait-system that "freezes" == and isequal for certain types. One would define a trait and dispatch hierarchy that declares that ==(::Any, ::Symbol) and ==(::Symbol, ::Any) always pass through === without the possibility of calling ==.
  3. magic

My favorite is option 3, followed by 1. Option 2 makes me think "yuck."

This helps reduce the footprint for invalidations
Fixes some invalidations
Avoids invalidations
Fixes some invalidations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:latency Compiler latency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants