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 1.9 -> 1.10: getproperty 3x longer #55117

Open
MarcMush opened this issue Jul 13, 2024 · 2 comments
Open

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

MarcMush opened this issue Jul 13, 2024 · 2 comments
Labels
performance Must go faster regression 1.10 Regression in the 1.10 release regression 1.11 Regression in the 1.11 release regression 1.12 Regression in the 1.12 release

Comments

@MarcMush
Copy link
Sponsor Contributor

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
14return %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
18return %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
; └
}
@nsajko nsajko added performance Must go faster regression 1.10 Regression in the 1.10 release regression 1.11 Regression in the 1.11 release regression 1.12 Regression in the 1.12 release labels Jul 13, 2024
@ericphanson
Copy link
Contributor

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
Copy link
Sponsor Contributor Author

indeed this non-recursive function works efficiently like in 1.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster regression 1.10 Regression in the 1.10 release regression 1.11 Regression in the 1.11 release regression 1.12 Regression in the 1.12 release
Projects
None yet
Development

No branches or pull requests

3 participants