-
Notifications
You must be signed in to change notification settings - Fork 22
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
Avoid generated when defining setproperties #22
Conversation
Codecov Report
@@ Coverage Diff @@
## master #22 +/- ##
=====================================
- Coverage 100% 80% -20%
=====================================
Files 1 1
Lines 23 30 +7
=====================================
+ Hits 23 24 +1
- Misses 0 6 +6
Continue to review full report at Codecov.
|
I think before merging this, we should have a suite of performance and maybe IR tests. Setfield has something like this for inspiration: https://github.com/jw3126/Setfield.jl/blob/master/test/perf.jl |
Setting up IR tests first sounds like a good idea. This PR was just meant to be a PoC. Let's re-open this if we decide to do this in #21. |
This has bad performance for structs with more then two fields. I get struct S
a::Int;b::Int;c::Int
end
o = S(1,2,3)
p = (a=2,)
@btime setproperties($o, $p) # 67.924 ns (3 allocations: 80 bytes) |
It seems that this approach not only has performance problem, but it also compiles slow: # This codes measures compile times of variants of setproperties.
# It patches a struct with various permutations of a NamedTuple
# and measures time
# we wrap the code in a gensymed module, so it can be easily recompiled in a jupyter notebook
module_name = gensym("SetpropertiesDefinitions")
@eval module $module_name
using Base: tail
@generated __fieldnames__(::Type{T}) where T = fieldnames(T)
@inline _issubset(::Tuple{}, _) = true
@inline _issubset(xs::Tuple, ys) = inargs(xs[1], ys...) && _issubset(tail(xs), ys)
@inline inargs(x) = false
@inline inargs(x, y, ys...) = x === y || inargs(x, ys...)
@inline foldlargs(op, x) = x
@inline foldlargs(op, x1, x2, xs...) = foldlargs(op, op(x1, x2), xs...)
@inline function setproperties_rec(obj, patch::NamedTuple{pnames}) where pnames
fnames = __fieldnames__(typeof(obj))
if !_issubset(pnames, fnames)
setproperties_unknown_field_error(obj, patch)
end
fields = foldlargs((), fnames...) do fields, name
if inargs(name, pnames...)
(fields..., patch[name])
else
(fields..., getfield(obj, name))
end
end
return constructorof(typeof(obj))(fields...)
end
@generated function setproperties_gen(obj, patch::NamedTuple)
if issubset(fieldnames(patch), fieldnames(obj))
args = map(fieldnames(obj)) do fn
if fn in fieldnames(patch)
:(patch.$fn)
else
:(obj.$fn)
end
end
return Expr(:block,
Expr(:meta, :inline),
Expr(:call,:(constructorof($obj)), args...)
)
else
:(setproperties_unknown_field_error(obj, patch))
end
end
function setproperties_unknown_field_error(obj, patch)
O = typeof(obj)
P = typeof(patch)
msg = """
Failed to assign properties $(fieldnames(P)) to object with fields $(fieldnames(O)).
You may want to overload
ConstructionBase.setproperties(obj::$O, patch::NamedTuple)
"""
throw(ArgumentError(msg))
end
@generated function constructorof(::Type{T}) where T
getfield(parentmodule(T), nameof(T))
end
constructorof(::Type{<:Tuple}) = tuple
constructorof(::Type{<:NamedTuple{names}}) where names =
NamedTupleConstructor{names}()
struct NamedTupleConstructor{names} end
@generated function (::NamedTupleConstructor{names})(args...) where names
quote
Base.@_inline_meta
$(NamedTuple{names, Tuple{args...}})(args)
end
end
end # end of module
M = @eval $module_name
using BenchmarkTools
### define struct Ints1, Ints2 with up to N fields
d = Dict()
N = 10
for n in 0:N
name = Symbol("Ints", n)
fields = [Expr(:(::), Symbol("field", i), Int) for i in 1:n]
@eval begin
struct $(name)
$(fields...)
end
$(name)() = $(name)((1:$n)...)
d[$n] = $(name)
end
end
Ints(i) = d[i]
function doit(setprop, args)
for (obj, patch) in args
setprop(obj, patch)
end
end
using Combinatorics
nstructfields = 4
npatchfields = 4
@assert npatchfields <= nstructfields
inds = permutations(1:npatchfields)
default_pairs = [Symbol(:field, i) => i for i in 1:npatchfields]
args = map(inds) do ind
pairs = default_pairs[ind]
patch = (;pairs...)
o = Ints(nstructfields)()
(o, patch)
end
@show npatchfields
@show nstructfields
@show last(args)
@show length(args)
GC.gc()
@time doit(M.setproperties_gen, args)
GC.gc()
@time doit(M.setproperties_rec, args)
|
Usually this can be solved by using Before: julia> @btime setproperties($o, $p)
53.838 ns (3 allocations: 80 bytes)
S(2, 2, 3)
julia> @code_typed setproperties(o, p)
CodeInfo(
1 ── goto #3 if not false
2 ── nothing::Nothing
3 ┄─ %3 = (getfield)((:a, :b, :c), 1)::Core.Compiler.Const(:a, false)
│ %4 = (getfield)((:a, :b, :c), 2)::Core.Compiler.Const(:b, false)
│ %5 = (getfield)((:a, :b, :c), 3)::Core.Compiler.Const(:c, false)
│ %6 = Base.getfield(patch, %3)::Int64
│ %7 = ConstructionBase.getfield(obj, %4)::Int64
│ %8 = (getfield)((:a,), 1)::Symbol
│ %9 = (%5 === %8)::Bool
└─── goto #5 if not %9
4 ── goto #6
5 ── goto #6
6 ┄─ %13 = φ (#4 => %9, #5 => false)::Bool
└─── goto #8 if not %13
7 ── %15 = Base.getfield(patch, %5)::Int64
└─── goto #9
8 ── %17 = ConstructionBase.getfield(obj, %5)::Int64
└─── goto #9
9 ┄─ %19 = φ (#7 => %6, #8 => %6)::Int64
│ %20 = φ (#7 => %7, #8 => %7)::Int64
│ %21 = φ (#7 => %15, #8 => %17)::Int64
└─── goto #10
10 ─ goto #11
11 ─ goto #12
12 ─ %25 = %new(Main.S, %19, %20, %21)::S
└─── return %25
) => S After: julia> @btime setproperties($o, $p)
1.641 ns (0 allocations: 0 bytes)
S(2, 2, 3)
julia> @code_typed setproperties(o, p)
CodeInfo(
1 ─ goto #3 if not false
2 ─ nothing::Nothing
3 ┄ %3 = Base.getfield(patch, :a)::Int64
│ %4 = ConstructionBase.getfield(obj, :b)::Int64
│ %5 = ConstructionBase.getfield(obj, :c)::Int64
│ %6 = %new(Main.S, %3, %4, %5)::S
└── return %6
) => S Although it only works up to five fields: julia> o = (a=1,b=2,c=3,d=4,e=5)
(a = 1, b = 2, c = 3, d = 4, e = 5)
julia> @code_typed setproperties(o, p)
CodeInfo(
1 ─ goto #3 if not false
2 ─ nothing::Nothing
3 ┄ %3 = Base.getfield(patch, :a)::Int64
│ %4 = ConstructionBase.getfield(obj, :b)::Int64
│ %5 = ConstructionBase.getfield(obj, :c)::Int64
│ %6 = ConstructionBase.getfield(obj, :d)::Int64
│ %7 = ConstructionBase.getfield(obj, :e)::Int64
│ %8 = %new(NamedTuple{(:a, :b, :c, :d, :e),NTuple{5,Int64}}, %3, %4, %5, %6, %7)::NamedTuple{(:a, :b, :c, :d, :e),NTuple{5,Int64}}
└── return %8
) => NamedTuple{(:a, :b, :c, :d, :e),NTuple{5,Int64}}
julia> o = (a=1,b=2,c=3,d=4,e=5,f=6);
julia> @code_typed setproperties(o, p)
CodeInfo(
1 ─ %1 = ConstructionBase.Val::Core.Compiler.Const(Val, false)
│ %2 = (getfield)((:a, :b, :c, :d, :e, :f), 3)::Symbol
│ %3 = (getfield)((:a, :b, :c, :d, :e, :f), 4)::Symbol
│ %4 = (getfield)((:a, :b, :c, :d, :e, :f), 5)::Symbol
│ %5 = (getfield)((:a, :b, :c, :d, :e, :f), 6)::Symbol
│ %6 = invoke %1(%2::Symbol)::Val{_A} where _A
│ %7 = invoke %1(%3::Symbol)::Val{_A} where _A
│ %8 = invoke %1(%4::Symbol)::Val{_A} where _A
│ %9 = invoke %1(%5::Symbol)::Val{_A} where _A
└── goto #3 if not false
2 ─ nothing::Nothing
3 ┄ %12 = %new(ConstructionBase.var"##21#22"{NamedTuple{(:a, :b, :c, :d, :e, :f),NTuple{6,Int64}},NamedTuple{(:a,),Tuple{Int64}},Tuple{Val{:a}}}, obj, patch, (Val{:a}(),))::ConstructionBase.var"##21#22"{NamedTuple{(:a, :b, :c, :d, :e, :f),NTuple{6,Int64}},NamedTuple{(:a,),Tuple{Int64}},Tuple{Val{:a}}}
│ %13 = Base.getfield(patch, :a)::Int64
│ %14 = ConstructionBase.getfield(obj, :b)::Int64
│ %15 = Core.tuple(%13, %14)::Tuple{Int64,Int64}
│ %16 = (%12)(%15, %6)::Tuple{Int64,Int64,Int64}
│ %17 = ConstructionBase.foldlargs(%12, %16, %7, %8, %9)::NTuple{6,Int64}
│ %18 = (getfield)(%17, 1)::Int64
│ %19 = (getfield)(%17, 2)::Int64
│ %20 = (getfield)(%17, 3)::Int64
│ %21 = (getfield)(%17, 4)::Int64
│ %22 = (getfield)(%17, 5)::Int64
│ %23 = (getfield)(%17, 6)::Int64
│ %24 = %new(NamedTuple{(:a, :b, :c, :d, :e, :f),NTuple{6,Int64}}, %18, %19, %20, %21, %22, %23)::NamedTuple{(:a, :b, :c, :d, :e, :f),NTuple{6,Int64}}
└── return %24
) => NamedTuple{(:a, :b, :c, :d, :e, :f),NTuple{6,Int64}} |
Cool, thanks for sharing the |
Implementation of #21.