Closed dpinol closed 1 year ago
Hi @dpinol,
nice detective work! As you can tell from your print statements, caching was done for the unconverted object instead of the converted one.
@JonasIsensee thanks to you for your quick PR! Do you have an estimation on a release date of the next JLD2 version? :smiley:
By the way, initially the PR didn't avoid the duplications on my real project, and I found out that it happened when the duplicated reference was in a Set and I was serializing through a struct
instead of a Dict
. I finally realized that the problem is solved if I change my FS
struct to be mutable
. Maybe it's worth mentioning it in the documentation to avoid other people to get into the same gotcha.
using JLD2
abstract type AT end
Base.@kwdef mutable struct T1 <: AT
f
end
Base.@kwdef mutable struct T2 <: AT
t1s::Set{T1}
t1::T1
end
struct FS # <= we get duplicated instances unless it's a mutable struct
fields::Dict{Symbol, Any}
end
(JLD2.writeas(::Type{T})) where {T <: AT} = FS
function Base.convert(::Type{FS}, t::AT)
@info "saving $(typeof(t))"
return FS(Dict(f => getproperty(t, f) for f in fieldnames(typeof(t))))
end
function Base.convert(::Type{T}, fs::FS) where {T <: AT}
t = T(; fs.fields...)
return t
end
t1 = T1(1)
t2 = T2(Set([t1]), t1)
save_object("kk.jld2", t2)
t2 = load_object("kk.jld2")
@info "t1s" t2.t1 === only(t2.t1s)
I can tag a release later today but I want to understand your second example first. I want to see if this can also be fixed.
thanks For the record, in my full project I get a segmentation fault when loading the FS struct mutable. It looks like I get it with #undef contents.
[ Info: loading FS(#undef)
signal (11): Segmentation fault
in expression starting at /proj/test/core/core.serialization.test.jl:7
typekeyvalue_hash at /julia-release-1-dot-8/src/jltypes.c:1184 [inlined]
lookup_typevalue at /julia-release-1-dot-8/src/jltypes.c:748
jl_inst_arg_tuple_type at /julia-release-1-dot-8/src/jltypes.c:1627
arg_type_tuple at /julia-release-1-dot-8/src/gf.c:1903 [inlined]
jl_lookup_generic_ at /julia-release-1-dot-8/src/gf.c:2493 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2545
#createFromFields#105 at /proj/src/utils/serialization.jl:40
createFromFields at /proj/src/utils/serialization.jl:35 [inlined]
convert at /proj/src/core/model/engine.jl:97
unknown function (ip: 0x7fc3bcb6f0e3)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
rconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/custom_serialization.jl:10
unknown function (ip: 0x7fc3bcb745b6)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
macro expansion at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/reconstructing_datatypes.jl:560 [inlined]
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/reconstructing_datatypes.jl:525
read_scalar at /home/dani/.julia/packages/JLD2/sf1Q4/src/dataio.jl:37
unknown function (ip: 0x7fc3bcb74551)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:238
unknown function (ip: 0x7fc3bcb6ae9d)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:194
load_dataset at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:125
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/writing_datatypes.jl:305 [inlined]
macro expansion at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/reconstructing_datatypes.jl:606 [inlined]
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/reconstructing_datatypes.jl:525
read_scalar at /home/dani/.julia/packages/JLD2/sf1Q4/src/dataio.jl:37
unknown function (ip: 0x7fc3bcb74253)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:238
unknown function (ip: 0x7fc3bcb6ae9d)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:194
load_dataset at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:125
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/writing_datatypes.jl:305 [inlined]
macro expansion at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/reconstructing_datatypes.jl:606 [inlined]
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/reconstructing_datatypes.jl:525
read_scalar at /home/dani/.julia/packages/JLD2/sf1Q4/src/dataio.jl:37
unknown function (ip: 0x7fc3bcb73b64)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:238
unknown function (ip: 0x7fc3bcb6ae9d)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:194
load_dataset at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:125
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/writing_datatypes.jl:305 [inlined]
macro expansion at /home/dani/.julia/packages/JLD2/sf1Q4/src/dataio.jl:70 [inlined]
macro expansion at ./simdloop.jl:77 [inlined]
read_array! at /home/dani/.julia/packages/JLD2/sf1Q4/src/dataio.jl:68
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_array at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:406
unknown function (ip: 0x7fc3bcb7344a)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:272
macro expansion at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:224 [inlined]
macro expansion at /home/dani/.julia/packages/JLD2/sf1Q4/src/datatypes.jl:105 [inlined]
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:211
load_dataset at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:125
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/writing_datatypes.jl:305 [inlined]
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/custom_serialization.jl:47
read_scalar at /home/dani/.julia/packages/JLD2/sf1Q4/src/dataio.jl:37
unknown function (ip: 0x7fc3bcb71e21)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:238
unknown function (ip: 0x7fc3bcb6ae9d)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:194
load_dataset at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:125
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/writing_datatypes.jl:305 [inlined]
macro expansion at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/reconstructing_datatypes.jl:560 [inlined]
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/reconstructing_datatypes.jl:525
jlconvert at /home/dani/.julia/packages/JLD2/sf1Q4/src/data/custom_serialization.jl:47
read_scalar at /home/dani/.julia/packages/JLD2/sf1Q4/src/dataio.jl:37
unknown function (ip: 0x7fc3bcb6f311)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:238
unknown function (ip: 0x7fc3bcb6ae9d)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
read_data at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:194
load_dataset at /home/dani/.julia/packages/JLD2/sf1Q4/src/datasets.jl:125
getindex at /home/dani/.julia/packages/JLD2/sf1Q4/src/groups.jl:109
getindex at /home/dani/.julia/packages/JLD2/sf1Q4/src/JLD2.jl:451 [inlined]
#77 at /home/dani/.julia/packages/JLD2/sf1Q4/src/loadsave.jl:214
unknown function (ip: 0x7fc3bcb5e982)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
#jldopen#70 at /home/dani/.julia/packages/JLD2/sf1Q4/src/loadsave.jl:4
jldopen at /home/dani/.julia/packages/JLD2/sf1Q4/src/loadsave.jl:1 [inlined]
load_object at /home/dani/.julia/packages/JLD2/sf1Q4/src/loadsave.jl:210
unknown function (ip: 0x7fc3bcb4fc52)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
jl_apply at /julia-release-1-dot-8/src/julia.h:1839 [inlined]
do_call at /julia-release-1-dot-8/src/interpreter.c:126
eval_value at /julia-release-1-dot-8/src/interpreter.c:215
eval_body at /julia-release-1-dot-8/src/interpreter.c:467
eval_body at /julia-release-1-dot-8/src/interpreter.c:522
eval_body at /julia-release-1-dot-8/src/interpreter.c:522
eval_body at /julia-release-1-dot-8/src/interpreter.c:522
eval_body at /julia-release-1-dot-8/src/interpreter.c:522
jl_interpret_toplevel_thunk at /julia-release-1-dot-8/src/interpreter.c:750
jl_toplevel_eval_flex at /julia-release-1-dot-8/src/toplevel.c:906
jl_toplevel_eval_flex at /julia-release-1-dot-8/src/toplevel.c:850
ijl_toplevel_eval_in at /julia-release-1-dot-8/src/toplevel.c:965
eval at ./boot.jl:368 [inlined]
include_string at ./loading.jl:1428
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
_include at ./loading.jl:1488
include at ./Base.jl:419
jfptr_include_55781.clone_1 at /opt/julia/julia-1.8.2/lib/julia/sys.so (unknown line)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
jl_apply at /julia-release-1-dot-8/src/julia.h:1839 [inlined]
jl_f__call_in_world at /julia-release-1-dot-8/src/builtins.c:793
#invoke_in_world#3 at ./essentials.jl:763 [inlined]
invoke_in_world at ./essentials.jl:760 [inlined]
include at /proj/test/runtests.jl:98
include_test_file at /home/dani/.julia/packages/Jive/8tjab/src/runtests.jl:185
jive_lets_dance at /home/dani/.julia/packages/Jive/8tjab/src/runtests.jl:245 [inlined]
normal_run at /home/dani/.julia/packages/Jive/8tjab/src/runtests.jl:209
#runtests#26 at /home/dani/.julia/packages/Jive/8tjab/src/runtests.jl:164
runtests##kw at /home/dani/.julia/packages/Jive/8tjab/src/runtests.jl:135
unknown function (ip: 0x7fc434144c91)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
runJive at /proj/test/runtests.jl:115
run at /proj/test/runtests.jl:126
unknown function (ip: 0x7fc43412f794)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
jl_apply at /julia-release-1-dot-8/src/julia.h:1839 [inlined]
do_call at /julia-release-1-dot-8/src/interpreter.c:126
eval_value at /julia-release-1-dot-8/src/interpreter.c:215
eval_stmt_value at /julia-release-1-dot-8/src/interpreter.c:166 [inlined]
eval_body at /julia-release-1-dot-8/src/interpreter.c:612
jl_interpret_toplevel_thunk at /julia-release-1-dot-8/src/interpreter.c:750
jl_toplevel_eval_flex at /julia-release-1-dot-8/src/toplevel.c:906
jl_toplevel_eval_flex at /julia-release-1-dot-8/src/toplevel.c:850
ijl_toplevel_eval_in at /julia-release-1-dot-8/src/toplevel.c:965
eval at ./boot.jl:368 [inlined]
include_string at ./loading.jl:1428
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
_include at ./loading.jl:1488
include at ./client.jl:476
unknown function (ip: 0x7fc4340a3032)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
jl_apply at /julia-release-1-dot-8/src/julia.h:1839 [inlined]
do_call at /julia-release-1-dot-8/src/interpreter.c:126
eval_value at /julia-release-1-dot-8/src/interpreter.c:215
eval_stmt_value at /julia-release-1-dot-8/src/interpreter.c:166 [inlined]
eval_body at /julia-release-1-dot-8/src/interpreter.c:612
jl_interpret_toplevel_thunk at /julia-release-1-dot-8/src/interpreter.c:750
jl_toplevel_eval_flex at /julia-release-1-dot-8/src/toplevel.c:906
jl_toplevel_eval_flex at /julia-release-1-dot-8/src/toplevel.c:850
ijl_toplevel_eval_in at /julia-release-1-dot-8/src/toplevel.c:965
eval at ./boot.jl:368 [inlined]
include_string at ./loading.jl:1428
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
_include at ./loading.jl:1488
include at ./client.jl:476 [inlined]
reviseLoop at /proj/ci/revise.jl:26
unknown function (ip: 0x7fc434106792)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
jl_apply at /julia-release-1-dot-8/src/julia.h:1839 [inlined]
do_call at /julia-release-1-dot-8/src/interpreter.c:126
eval_value at /julia-release-1-dot-8/src/interpreter.c:215
eval_stmt_value at /julia-release-1-dot-8/src/interpreter.c:166 [inlined]
eval_body at /julia-release-1-dot-8/src/interpreter.c:612
jl_interpret_toplevel_thunk at /julia-release-1-dot-8/src/interpreter.c:750
jl_toplevel_eval_flex at /julia-release-1-dot-8/src/toplevel.c:906
jl_toplevel_eval_flex at /julia-release-1-dot-8/src/toplevel.c:850
ijl_toplevel_eval_in at /julia-release-1-dot-8/src/toplevel.c:965
eval at ./boot.jl:368 [inlined]
include_string at ./loading.jl:1428
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
_include at ./loading.jl:1488
include at ./Base.jl:419
jfptr_include_55781.clone_1 at /opt/julia/julia-1.8.2/lib/julia/sys.so (unknown line)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
exec_options at ./client.jl:303
_start at ./client.jl:522
jfptr__start_56736.clone_1 at /opt/julia/julia-1.8.2/lib/julia/sys.so (unknown line)
_jl_invoke at /julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /julia-release-1-dot-8/src/gf.c:2549
jl_apply at /julia-release-1-dot-8/src/julia.h:1839 [inlined]
true_main at /julia-release-1-dot-8/src/jlapi.c:575
jl_repl_entrypoint at /julia-release-1-dot-8/src/jlapi.c:719
main at julia (unknown line)
unknown function (ip: 0x7fc44fe14d8f)
__libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x401098)
Allocations: 95997191 (Pool: 95950971; Big: 46220); GC: 89
/proj/bin/utils.sh: line 82: 639328 Segmentation fault (core dumped) nice ionice julia "$@"
I tried using again a inmutable struct but now with hash/== implemented, but it didn't help
I could generate a MWE. The problem is when you make the FS struct mutable and try to persist a recursive reference (it also happens when there are 2 objects which reference each other):
using JLD2
abstract type AT end
mutable struct FS
fields::Dict{Symbol, Any}
end
(JLD2.writeas(::Type{T})) where {T <: AT} = FS
function Base.convert(::Type{FS}, t::AT)
@info "saving $(typeof(t))"
return FS(Dict(f => getproperty(t, f) for f in fieldnames(typeof(t))))
end
function Base.convert(::Type{T}, fs::FS) where {T <: AT}
t = T(; fs.fields...)
@info "loading" T
#@info "loading $T from $(pointer_from_objref(fs)) to $(pointer_from_objref(t))"
return t
end
Base.@kwdef mutable struct T1 <: AT
t1::Base.RefValue{Any}
end
function save()
t1 = T1(Ref{Any}(nothing))
t1.t1[] = t1
@info "t1" t1
return save_object("kk.jld2", t1)
end
save()
t1 = load_object("kk.jld2")
@info "t1" t1
The last @info
shows that the restore T1.t1[] is not a T, but an FS.
┌ Info: t1 └ t1.t1[] = FS(Dict{Symbol, Any}(:t1 => Base.RefValue{Any}(FS(#= circular reference @-3 =#))))
This only happens when FS is mutable.
thanks
I think the last 2 problems I report has nothing to do with this PR. I can also reproduce them with master branch. I'll create another PR
@JonasIsensee thanks to you for your quick PR! Do you have an estimation on a release date of the next JLD2 version? smiley
By the way, initially the PR didn't avoid the duplications on my real project, and I found out that it happened when the duplicated reference was in a Set and I was serializing through a
struct
instead of aDict
. I finally realized that the problem is solved if I change myFS
struct to bemutable
. Maybe it's worth mentioning it in the documentation to avoid other people to get into the same gotcha.using JLD2 abstract type AT end Base.@kwdef mutable struct T1 <: AT f end Base.@kwdef mutable struct T2 <: AT t1s::Set{T1} t1::T1 end struct FS # <= we get duplicated instances unless it's a mutable struct fields::Dict{Symbol, Any} end (JLD2.writeas(::Type{T})) where {T <: AT} = FS function Base.convert(::Type{FS}, t::AT) @info "saving $(typeof(t))" return FS(Dict(f => getproperty(t, f) for f in fieldnames(typeof(t)))) end function Base.convert(::Type{T}, fs::FS) where {T <: AT} t = T(; fs.fields...) return t end t1 = T1(1) t2 = T2(Set([t1]), t1) save_object("kk.jld2", t2) t2 = load_object("kk.jld2") @info "t1s" t2.t1 === only(t2.t1s)
Ok, here's a simpler example:
Base.@kwdef mutable struct AStruct6
x::Int
end
as = AStruct6(1)
struct ASerialized
x::Int
end
JLD2.writeas(::Type{AStruct6}) = ASerialized
JLD2.wconvert(::Type{ASerialized}, t::AStruct6) = ASerialized(t.x)
JLD2.rconvert(::Type{AStruct6}, dsa::ASerialized) = (println("converted"); AStruct6(dsa.x))
save_object("kk.jld2", (as, as))
asl = load_object("kk.jld2")
asl[1] === asl[2]
and this is not fixable. If you want to preserve object identity relations then you have to convert to mutable objects.
Also, a more general note: converting to Dict
is probably not particularly performant.
Dict
s themselves use custom serialization to convert to a Vector{Pair{K,V}}
.
and this is not fixable. If you want to preserve object identity relations then you have to convert to mutable objects.
I understand. thanks!
Also, a more general note: converting to
Dict
is probably not particularly performant.Dict
s themselves use custom serialization to convert to aVector{Pair{K,V}}
.
good to know. thanks!
Image that I have a data structure where I have multiple references to the same object
and I need customm serialization. Let's do something generic to simplify
It looks like T1 instance, referenced from T2 & T3, is correctly saved only once
The T1 Dict is loaded only once (ptr 0x00007f5f94133800) but, then my load function created 2 different instance
I tried these 2 toy implementations of hash & ==, but they didn't help
Ideally imho the library should be able to reuse the Cached T1 and only call convert(T1) once in this case. Otherwise, we would need JLD2.convert to receive a new Context argument where the developer could cache the previously created instances. This Context could be also useful for other use cases, eg gather data produced by inner persistence structs. What do you think? thanks