ranocha commented 2 years ago

I'm new to fixing invalidations, so please help me understand whether I'm doing it correctly.

I ran the following code on Julia v1.8.0:

julia> using Pkg; Pkg.activate(temp=true); Pkg.add("ArrayInterface")

julia> using SnoopCompileCore

julia> invalidations = @snoopr using ArrayInterface;

julia> using SnoopCompile

julia> length(uinvalidated(invalidations))
1962

julia> trees = invalidation_trees(invalidations);

julia> trees[end-1]
inserting (::Colon)(L::Integer, ::Static.StaticInt{U}) where U in ArrayInterface at /home/hendrik/.julia/packages/ArrayInterface/qyd0m/src/ranges.jl:157 invalidated:
   mt_backedges:  1: signature Tuple{Colon, Int64, Any} triggered MethodInstance for InteractiveUtils.recursive_dotcalls!(::Expr, ::Vector{Any}, ::Int64) (0 children)
                  2: signature Tuple{Colon, Int64, Any} triggered MethodInstance for Base.Sort.sort_int_range!(::AbstractVector{<:Integer}, ::Any, ::Any, ::typeof(identity)) (1 children)
                  3: signature Tuple{Colon, Int64, Any} triggered MethodInstance for Base.Sort.sort_int_range!(::Vector{_A} where _A<:Integer, ::Any, ::Any, ::typeof(identity)) (1 children)
                  4: signature Tuple{Colon, Int64, Any} triggered MethodInstance for Printf.fmt(::Vector{UInt8}, ::Int64, ::Any, ::Printf.Spec{Val{'s'}}) (2 children)
                  5: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, Tuple{Bool}, _B, Pair{DataType, ArgTools.var"#5#15"}} where {_A, _B})(::Int64) (3 children)
                  6: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, Tuple{Bool}, _B, Pair{DataType, ArgTools.var"#7#17"}} where {_A, _B})(::Int64) (3 children)
                  7: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, Tuple{Bool}, _B, Pair{DataType, ArgTools.var"#25#35"}} where {_A, _B})(::Int64) (3 children)
                  8: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, Tuple{Bool}, _B, Pair{DataType, ArgTools.var"#27#37"}} where {_A, _B})(::Int64) (3 children)
                  9: signature Tuple{Colon, Int64, Any} triggered MethodInstance for Tar.var"#read_data#63"(::Int64, ::Vector{UInt8}, ::Base.DevNull, ::typeof(Tar.read_data), ::Base.Process, ::IOBuffer) (3 children)
                 10: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, Tuple{Bool}, _B, Vector{Base.PkgId}} where {_A, _B})(::Int64) (3 children)
                 11: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, Tuple{Bool}, _B, Pair{DataType, ArgTools.var"#23#33"}} where {_A, _B})(::Int64) (4 children)
                 12: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, Tuple{Bool}, _B, Pair{DataType, ArgTools.var"#3#13"}} where {_A, _B})(::Int64) (9 children)
                 13: signature Tuple{Colon, Int64, Any} triggered MethodInstance for Tar.var"#read_data#63"(::Int64, ::Vector{UInt8}, ::Base.DevNull, ::typeof(Tar.read_data), ::Base.Process, ::IOStream) (9 children)
                 14: signature Tuple{Colon, Int64, Any} triggered MethodInstance for InteractiveUtils.recursive_dotcalls!(::Any, ::Vector{Any}, ::Int64) (10 children)
                 15: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, _B, _C, Vector{Union{Int64, Symbol}}} where {_A, _B, _C})(::Int64) (13 children)
                 16: signature Tuple{Colon, Int64, Any} triggered MethodInstance for Tar.var"#read_standard_header#60"(::Vector{UInt8}, ::Base.DevNull, ::typeof(Tar.read_standard_header), ::Base.Process) (16 children)
                 17: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Base.var"#150#152"{_A, Tuple{Bool}} where _A)(::Int64) (21 children)
                 18: signature Tuple{Colon, Int64, Any} triggered MethodInstance for Tar.var"#write_data#19"(::Int64, ::Vector{UInt8}, ::typeof(Tar.write_data), ::IOStream, ::IO) (42 children)
                 19: signature Tuple{Colon, Int64, Any} triggered MethodInstance for (::Type{NamedTuple{_A}} where _A)(::NamedTuple) (117 children)

julia> root = trees[end-1].mt_backedges[end-1][2]
MethodInstance for Tar.var"#write_data#19"(::Int64, ::Vector{UInt8}, ::typeof(Tar.write_data), ::IOStream, ::IO) at depth 0 with 42 children

julia> ascend(root)
Choose a call for analysis (q to quit):
 >   #write_data#19(::Int64, ::Vector{UInt8}, ::typeof(Tar.write_data), ::IOStream, ::IO)
       (::Tar.var"#write_data##kw")(::NamedTuple{(:size, :buf), Tuple{Int64, Vector{UInt8}}}, ::typeof(Tar.write_
         #write_tarball#15(::Vector{UInt8}, ::typeof(Tar.write_tarball), ::IOStream, ::Tar.Header, ::Any)
           (::Tar.var"#write_tarball##kw")(::NamedTuple{(:buf,), Tuple{Vector{UInt8}}}, ::typeof(Tar.write_tarbal
             #write_tarball#14(::Vector{UInt8}, ::typeof(Tar.write_tarball), ::Function, ::IOStream, ::Any, ::Str
               (::Tar.var"#write_tarball##kw")(::NamedTuple{(:buf,), Tuple{Vector{UInt8}}}, ::typeof(Tar.write_ta
                 #write_tarball#14(::Vector{UInt8}, ::typeof(Tar.write_tarball), ::Function, ::IOStream, ::String
                   (::Tar.var"#write_tarball##kw")(::NamedTuple{(:buf,), Tuple{Vector{UInt8}}}, ::typeof(Tar.writ
                     #write_tarball#14(::Vector{UInt8}, ::typeof(Tar.write_tarball), ::Tar.var"#6#7"{Tar.var"#1#2
v                      (::Tar.var"#write_tarball##kw")(::NamedTuple{(:buf,), Tuple{Vector{UInt8}}}, ::typeof(Tar.
var"#write_data#19"(size::Integer, buf::Vector{UInt8}, ::typeof(Tar.write_data), tar::IO, data::IO) in Tar at /home/hendrik/Software/julia-1.8.0/share/julia/stdlib/v1.8/Tar/src/create.jl:284
Variables
  #write_data#19::Core.Const(Tar.var"#write_data#19")
  size@_2::Int64
  buf::Vector{UInt8}
  @_4::Core.Const(Tar.write_data)
  tar::IOStream
  data::IO
  t::Any
  w::Any
  n::Any
  b::Any
  size@_11::Any
  @_12::Any
...

As far as I understand,

the invalidations happen since n is only inferred to Any
telling the compiler that readbytes! returns an Integer should make it infer n::Integer
this should fix the invalidations since Static.StaticInt is not an Integer (anymore in version v0.7)

codecov[bot] commented 2 years ago

Codecov Report

Merging #138 (cde7d3a) into master (5606269) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #138   +/-   ##
=======================================
  Coverage   97.29%   97.29%           
=======================================
  Files           4        4           
  Lines         775      775           
=======================================
  Hits          754      754           
  Misses         21       21

Impacted Files	Coverage Δ
src/create.jl	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ranocha commented 2 years ago

I suggest the labels latency and backport-1.8 for this PR.

ranocha commented 2 years ago

Bump. Can we get this into Julia v1.8.1?

ranocha commented 2 years ago

Bump. Can we get this into Julia v1.8.1?

staticfloat commented 2 years ago

Looks good to me, thanks @ranocha!

JuliaIO / Tar.jl

hopefully fix invalidations from ArrayInterface.jl #138

Codecov Report