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

Regression in deepcopy of nested structures #39534

Closed
mohamed82008 opened this issue Feb 5, 2021 · 11 comments
Closed

Regression in deepcopy of nested structures #39534

mohamed82008 opened this issue Feb 5, 2021 · 11 comments

Comments

@mohamed82008
Copy link
Contributor

Hi,

The following seems like a regression when deepcopying nested structures.

using BenchmarkTools
a = (a = rand(1000),);
@btime deepcopy($a);

# Julia Version 1.5.3 - commit 788b2c77c1 (2020-11-09 13:37 UTC)
# 700.047 ns (6 allocations: 8.42 KiB)

# Julia Version 1.6.0-beta1.91 - commit e02764ca43* (2021-02-02 14:43 UTC)
# 797.800 ns (6 allocations: 8.42 KiB)

# Julia Version 1.7.0-DEV.429 - commit 11de85aa56* (2021-02-02 16:48 UTC)
#  843.463 ns (6 allocations: 8.42 KiB)

I am running Julia for Linux Arm on a Ubuntu 20 virtual machine on an Apple M1 processor so my platform prints:

Platform Info:
  OS: Linux (aarch64-unknown-linux-gnu)
  CPU: unknown
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, generic)
@JeffBezanson
Copy link
Member

Interesting; I can't reproduce this on x86_64. Is there also a regression in e.g. a = rand(1000); @btime copy(a)? Would be good to try to narrow it down as much as possible.

@mohamed82008
Copy link
Contributor Author

mohamed82008 commented Feb 6, 2021

The vector case has not regressed. Also the named tuple case seems to only regress when I load Pumas. The vector case doesn't regress even with Pumas loaded.

julia> using BenchmarkTools

julia> a = (a = rand(1000),);

julia> @btime deepcopy($a);
  466.235 ns (6 allocations: 8.42 KiB)

julia> using Revise

julia> @btime deepcopy($a);
  479.694 ns (6 allocations: 8.42 KiB)

julia> using Pumas

julia> @btime deepcopy($a);
  886.905 ns (6 allocations: 8.42 KiB)

The above benchmarks were using the nightly build of Julia from Feb 2. But the slowdown after loading Pumas seems to happen in all the Julia versions I tested. But some get slower than others. This slowdown is also happening outside the VM on MacOS Julia on Rosetta 2. So that may rule out VM weirdness. I still need to see if any of the deps of Pumas causes this individually.

@mohamed82008
Copy link
Contributor Author

A similar slowdown is observed when loading Catalyst.jl instead of Pumas.

@mohamed82008
Copy link
Contributor Author

Reduced to Catlab.jl. But none of Catlab's dependencies causes the slowdown. So the culprit is probably Catlab.

@mohamed82008
Copy link
Contributor Author

mohamed82008 commented Feb 6, 2021

The culprit is this definition in Catlab's theories/Schema.jl file (https://github.com/AlgebraicJulia/Catlab.jl/blob/master/src/theories/Schema.jl#L64):

function Base.getproperty(AD::Type{T},i::Symbol) where
  {Ob,Hom,Dom,Codom,T<:CatDesc{Ob,Hom,Dom,Codom}}
  @match i begin
    :ob => Ob
    :hom => Hom
    :dom => Dom
    :codom => Codom
    _ => getfield(AD,i)
  end
end

@vtjnash
Copy link
Member

vtjnash commented Feb 6, 2021

Type piracy is indeed highly discouraged, particularly over getproperty and for Type{T}

@mohamed82008
Copy link
Contributor Author

But it's not type piracy because Catlab owns CatDesc, no?

@KristofferC
Copy link
Member

KristofferC commented Feb 6, 2021

You also override it for Union{}:

julia> struct MyType end

julia> foo(::Type{T}) where T <: MyType = print("foo")
foo (generic function with 1 method)

julia> foo(MyType)
foo

julia> foo(Union{})
foo

Also you don't own Type which is the type which you are overriding things for. Defining this method will invalidate every method that was inferred with just a DataType type as the first argument to getproperty.

@mohamed82008
Copy link
Contributor Author

You also override it for Union{}:

Aha so with that logic, all uses of Type{<:T} in the dispatch of functions that I don't own is type piracy, for any T. I don't think this is common knowledge.

@timholy
Copy link
Member

timholy commented Feb 6, 2021

You can detect & fix this by measuring invalidations. Catlab is pretty bad: 1753 invalidations for me. The last few entries are:

 inserting getproperty(AD::Type{T}, i::Symbol) where {CD, Data, Attr, ADom, ACodom, T<:Catlab.Theories.AttrDesc{CD, Data, Attr, ADom, ACodom}} in Catlab.Theories at /home/tim/.julia/packages/Catlab/WCY8I/src/theories/Schema.jl:132 invalidated:
   backedges:  1: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{IO}, ::Symbol) (1 children)
               2: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{Any, Any, Bool, Function}, ::Symbol) (1 children)
               3: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{Int64, Bool, Function}, ::Symbol) (1 children)
               4: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{IOContext}, ::Symbol) (1 children)
               5: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{Any, Bool}, ::Symbol) (1 children)
               6: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{Any, Any, Bool, Pkg.Types.var"#25#28"}, ::Symbol) (1 children)
               7: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{Any, Any, Bool, typeof(identity)}, ::Symbol) (1 children)
               8: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{Any, Any, Bool, Pkg.API.var"#5#7"}, ::Symbol) (1 children)
               9: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{Exception}, ::Symbol) (1 children)
              10: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{T} where T<:Tuple{Tuple{Any, Vector{Union{Ptr{Nothing}, Base.InterpreterIP}}}}, ::Symbol) (1 children)
              11: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{var"#s433"} where var"#s433"<:Tuple{Any}, ::Symbol) (1 children)
              12: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{var"#s437"} where var"#s437"<:Tuple{Pair{String, String}, Vararg{Pair, N} where N}, ::Symbol) (1 children)
              13: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{var"#s437"} where var"#s437"<:AbstractArray{T, N} where N where T, ::Symbol) (3 children)
              14: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{var"#s437"} where var"#s437"<:Function, ::Symbol) (4 children)
              15: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{var"#s437"} where var"#s437"<:Integer, ::Symbol) (6 children)
              16: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{var"#s828"} where var"#s828"<:AbstractString, ::Symbol) (7 children)
              17: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type{var"#s437"} where var"#s437"<:Core.Builtin, ::Symbol) (12 children)
              18: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Union, ::Symbol) (78 children)
              19: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::Type, ::Symbol) (303 children)
              20: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::UnionAll, ::Symbol) (602 children)
              21: superseding getproperty(x::Type, f::Symbol) in Base at Base.jl:28 with MethodInstance for getproperty(::DataType, ::Symbol) (1263 children)

https://julialang.org/blog/2020/08/invalidations/

JeffBezanson added a commit that referenced this issue Feb 8, 2021
JeffBezanson added a commit that referenced this issue Feb 8, 2021
JeffBezanson added a commit that referenced this issue May 19, 2021
JeffBezanson added a commit that referenced this issue May 27, 2021
@vtjnash
Copy link
Member

vtjnash commented Feb 8, 2024

Was fixed by AlgebraicJulia/Catlab.jl#382

@vtjnash vtjnash closed this as completed Feb 8, 2024
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

No branches or pull requests

5 participants