JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.72k stars 5.48k forks source link

Regression 1.9 -> 1.10: `getproperty` 3x longer #55117

Open MarcMush opened 3 months ago

MarcMush commented 3 months ago

with this code that does some "julian inheritance":


mutable struct A
    x::String
end

mutable struct B
    y::Int
    a::A
end

function Base.getproperty(p::B, name::Symbol)
    if hasfield(B, name)
        getfield(p, name)
    else
        getproperty(p.a, name)
    end
end

read(b) = b.x

b = B(1, A("2"))
@btime read($b)

julia 1.9.4: 2.2 ns julia 1.10.4: 7.9 ns julia 1.11.0-rc1: 7.5 ns

the @code_typed is very similar, the only difference being that Base._fieldindex_nothrow is not inlined in 1.10.

removing the @noinline from _fieldindex_nothrow seems to help slightly but not completely solve the issue

#julia 1.9
julia> @code_typed read(b)
CodeInfo(
1 ── %1  = Main.B::Type{B}
│    %2  = $(Expr(:foreigncall, :(:jl_field_index), Int32, svec(Any, Any, Int32), 0, :(:ccall), :(%1), :(:a), 0, 0))::Int32
│    %3  = Base.sext_int(Int64, %2)::Int64
│    %4  = Base.add_int(%3, 1)::Int64
│    %5  = Base.slt_int(0, %4)::Bool
└───       goto #3 if not %5
2 ── %7  = Main.getfield(b, :a)::Union{Int64, A}
└───       goto #4
3 ── %9  = Main.getfield(b, :a)::A
│    %10 = Base.getfield(%9, :a)::String
└───       goto #4
4 ┄─ %12 = φ (#2 => false, #3 => true)::Bool
│    %13 = φ (#2 => %7, #3 => %10)::Union{Int64, String, A}
│    %14 = (isa)(%13, Int64)::Bool
└───       goto #7 if not %14
5 ── %16 = π (%13, Int64)
│          Base.getfield(%16, :x)::Union{}
└───       unreachable
6 ──       unreachable
7 ──       goto #10 if not %12
8 ── %21 = π (%13, String)
│          Base.getfield(%21, :x)::Union{}
└───       unreachable
9 ──       unreachable
10 ─ %25 = (isa)(%13, A)::Bool
└───       goto #12 if not %25
11 ─ %27 = π (%13, A)
│    %28 = Base.getfield(%27, :x)::String
└───       goto #13
12 ─       Core.throw(ErrorException("fatal error in type inference (type bound)"))::Union{}
└───       unreachable
13 ─       goto #14
14 ─       return %28
) => String
#julia 1.10
julia> @code_typed read(b)
CodeInfo(
1 ── %1  = Main.B::Type{B}
└───       goto #3
2 ──       nothing::Nothing
3 ┄─ %4  = invoke Base._fieldindex_nothrow(%1::DataType, :a::Symbol)::Int64
└───       goto #4
4 ── %6  = Base.slt_int(0, %4)::Bool
└───       goto #5
5 ──       goto #7 if not %6
6 ── %9  = Main.getfield(b, :a)::Union{Int64, A}
└───       goto #8
7 ── %11 = Main.getfield(b, :a)::A
│    %12 = Base.getfield(%11, :a)::String
└───       goto #8
8 ┄─ %14 = φ (#6 => false, #7 => true)::Bool
│    %15 = φ (#6 => %9, #7 => %12)::Union{Int64, String, A}
│    %16 = (isa)(%15, Int64)::Bool
└───       goto #11 if not %16
9 ── %18 = π (%15, Int64)
│          Base.getfield(%18, :x)::Union{}
└───       unreachable
10 ─       unreachable
11 ─       goto #14 if not %14
12 ─ %23 = π (%15, String)
│          Base.getfield(%23, :x)::Union{}
└───       unreachable
13 ─       unreachable
14 ─ %27 = (isa)(%15, A)::Bool
└───       goto #16 if not %27
15 ─ %29 = π (%15, A)
│    %30 = Base.getfield(%29, :x)::String
└───       goto #17
16 ─       Core.throw(ErrorException("fatal error in type inference (type bound)"))::Union{}
└───       unreachable
17 ─       goto #18
18 ─       return %30
) => String

The big difference is in @code_llvm

#1.9
julia> @code_llvm read(b)
;  @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:19 within `read`
; Function Attrs: uwtable
define nonnull {}* @julia_read_542({}* noundef nonnull align 8 dereferenceable(16) %0) #0 {
top:
;  @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:20 within `read`
; ┌ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 within `getproperty` @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:13
   %1 = bitcast {}* %0 to i8*
   %2 = getelementptr inbounds i8, i8* %1, i64 8
   %3 = bitcast i8* %2 to {}***
   %4 = load atomic {}**, {}*** %3 unordered, align 8
; │ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 within `getproperty` @ Base.jl:37
   %5 = load atomic {}*, {}** %4 unordered, align 8
; └
  ret {}* %5
}
#1.10
julia> @code_llvm read(b)
;  @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:19 within `read`
; Function Attrs: uwtable
define nonnull {}* @julia_read_573({}* noundef nonnull align 8 dereferenceable(16) %0) #0 {
top:
  %1 = alloca [2 x {}*], align 8
  %gcframe15 = alloca [3 x {}*], align 16
  %gcframe15.sub = getelementptr inbounds [3 x {}*], [3 x {}*]* %gcframe15, i64 0, i64 0
  %2 = bitcast [3 x {}*]* %gcframe15 to i8*
  call void @llvm.memset.p0i8.i64(i8* align 16 %2, i8 0, i64 24, i1 true)
  %3 = call {}*** inttoptr (i64 140734520482496 to {}*** ()*)() #6
;  @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:20 within `read`
; ┌ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 within `getproperty` @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:12
; │┌ @ reflection.jl:197 within `hasfield`
; ││┌ @ reflection.jl:825 within `fieldindex`
; │││┌ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:25 within `_fieldindex_nothrow`
      %4 = bitcast [3 x {}*]* %gcframe15 to i64*
      store i64 4, i64* %4, align 16
      %5 = getelementptr inbounds [3 x {}*], [3 x {}*]* %gcframe15, i64 0, i64 1
      %6 = bitcast {}** %5 to {}***
      %7 = load {}**, {}*** %3, align 8
      store {}** %7, {}*** %6, align 8
      %8 = bitcast {}*** %3 to {}***
      store {}** %gcframe15.sub, {}*** %8, align 8
      %9 = call i32 inttoptr (i64 140734520416368 to i32 ({}*, {}*, i32)*)({}* inttoptr (i64 1985824393104 to {}*), {}* inttoptr (i64 1985762169200 to {}*),
 i32 0)
; ││└└
; ││┌ @ operators.jl:378 within `>`
; │││┌ @ int.jl:83 within `<`
      %10 = icmp slt i32 %9, 0
; │└└└
; │ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 within `getproperty` @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:0
   %11 = bitcast {}* %0 to i8*
   %getfield_addr6 = getelementptr inbounds i8, i8* %11, i64 8
   %12 = bitcast i8* %getfield_addr6 to {}**
   %getfield7 = load atomic {}*, {}** %12 unordered, align 8
; │ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 within `getproperty` @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:12
   br i1 %10, label %L25, label %post_isa

L25:                                              ; preds = %top
   %.sub = getelementptr inbounds [2 x {}*], [2 x {}*]* %1, i64 0, i64 0
   %13 = getelementptr inbounds [3 x {}*], [3 x {}*]* %gcframe15, i64 0, i64 2
   store {}* %getfield7, {}** %13, align 16
; │ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 within `getproperty` @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 @ Base.jl:37
   store {}* %getfield7, {}** %.sub, align 8
   %14 = getelementptr inbounds [2 x {}*], [2 x {}*]* %1, i64 0, i64 1
   store {}* inttoptr (i64 1985762169200 to {}*), {}** %14, align 8
   %15 = call nonnull {}* @jl_f_getfield({}* null, {}** nonnull %.sub, i32 2)
   store {}* %15, {}** %13, align 16
; │ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 within `getproperty` @ Base.jl:37
   store {}* %15, {}** %.sub, align 8
   store {}* inttoptr (i64 1985761965456 to {}*), {}** %14, align 8
   %16 = call nonnull {}* @jl_f_getfield({}* null, {}** nonnull %.sub, i32 2)
   call void @llvm.trap()
   unreachable

L31:                                              ; preds = %post_isa
   %17 = bitcast {}* %getfield7 to {}**
   %getfield4 = load atomic {}*, {}** %17 unordered, align 8
   %18 = load {}*, {}** %5, align 8
   %19 = bitcast {}*** %3 to {}**
   store {}* %18, {}** %19, align 8
; └
  ret {}* %getfield4

L34:                                              ; preds = %post_isa
; ┌ @ C:\Users\Marc\Programmation\Julia\progressmeter_perf\perf.jl:15 within `getproperty`
   call void @ijl_throw({}* inttoptr (i64 140734228577984 to {}*))
   unreachable

post_isa:                                         ; preds = %top
   %20 = bitcast {}* %getfield7 to i64*
   %21 = getelementptr inbounds i64, i64* %20, i64 -1
   %22 = load atomic i64, i64* %21 unordered, align 8
   %23 = and i64 %22, -16
   %24 = inttoptr i64 %23 to {}*
   %.not = icmp eq {}* %24, inttoptr (i64 1985824423312 to {}*)
   br i1 %.not, label %L31, label %L34
; └
}
ericphanson commented 3 months ago

usually it’s better to write these non-recursively:

function Base.getproperty(p::B, name::Symbol)
    if hasfield(B, name)
        getfield(p, name)
    else
        getproperty(getfield(p, :a) name)
    end
end

Here I use getfield in the second branch also.

MarcMush commented 3 months ago

indeed this non-recursive function works efficiently like in 1.9