Skip to content

Commit

Permalink
reland "Fix serialization of code instances" (#46939)
Browse files Browse the repository at this point in the history
If they are encountered in a Julia object directly, as opposed to
e.g. during serialization of a method instance, we'll have first
populated the backref table as part of jl_serialize_value, so we
need to skip that check during code instance serialization.
  • Loading branch information
maleadt authored Sep 28, 2022
1 parent b2c2abf commit e7a5c36
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
14 changes: 8 additions & 6 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -698,13 +698,15 @@ static int jl_serialize_generic(jl_serializer_state *s, jl_value_t *v) JL_GC_DIS
return 0;
}

static void jl_serialize_code_instance(jl_serializer_state *s, jl_code_instance_t *codeinst, int skip_partial_opaque, int internal) JL_GC_DISABLED
static void jl_serialize_code_instance(jl_serializer_state *s, jl_code_instance_t *codeinst,
int skip_partial_opaque, int internal,
int force) JL_GC_DISABLED
{
if (internal > 2) {
while (codeinst && !codeinst->relocatability)
codeinst = codeinst->next;
}
if (jl_serialize_generic(s, (jl_value_t*)codeinst)) {
if (!force && jl_serialize_generic(s, (jl_value_t*)codeinst)) {
return;
}
assert(codeinst != NULL); // handle by jl_serialize_generic, but this makes clang-sa happy
Expand All @@ -725,7 +727,7 @@ static void jl_serialize_code_instance(jl_serializer_state *s, jl_code_instance_
if (write_ret_type && codeinst->rettype_const &&
jl_typeis(codeinst->rettype_const, jl_partial_opaque_type)) {
if (skip_partial_opaque) {
jl_serialize_code_instance(s, codeinst->next, skip_partial_opaque, internal);
jl_serialize_code_instance(s, codeinst->next, skip_partial_opaque, internal, 0);
return;
}
else {
Expand All @@ -752,7 +754,7 @@ static void jl_serialize_code_instance(jl_serializer_state *s, jl_code_instance_
jl_serialize_value(s, jl_nothing);
}
write_uint8(s->s, codeinst->relocatability);
jl_serialize_code_instance(s, codeinst->next, skip_partial_opaque, internal);
jl_serialize_code_instance(s, codeinst->next, skip_partial_opaque, internal, 0);
}

enum METHOD_SERIALIZATION_MODE {
Expand Down Expand Up @@ -1013,10 +1015,10 @@ static void jl_serialize_value_(jl_serializer_state *s, jl_value_t *v, int as_li
}
jl_serialize_value(s, (jl_value_t*)backedges);
jl_serialize_value(s, (jl_value_t*)NULL); //callbacks
jl_serialize_code_instance(s, mi->cache, 1, internal);
jl_serialize_code_instance(s, mi->cache, 1, internal, 0);
}
else if (jl_is_code_instance(v)) {
jl_serialize_code_instance(s, (jl_code_instance_t*)v, 0, 2);
jl_serialize_code_instance(s, (jl_code_instance_t*)v, 0, 2, 1);
}
else if (jl_typeis(v, jl_module_type)) {
jl_serialize_module(s, (jl_module_t*)v);
Expand Down
17 changes: 17 additions & 0 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1546,5 +1546,22 @@ precompile_test_harness("Issue #46558") do load_path
@test (@eval $Foo.foo(1)) == 2
end

precompile_test_harness("issue #46296") do load_path
write(joinpath(load_path, "CodeInstancePrecompile.jl"),
"""
module CodeInstancePrecompile
mi = first(methods(identity)).specializations[1]
ci = Core.CodeInstance(mi, Any, nothing, nothing, zero(Int32), typemin(UInt),
typemax(UInt), zero(UInt32), zero(UInt32), nothing, 0x00)
__init__() = @assert ci isa Core.CodeInstance
end
""")
Base.compilecache(Base.PkgId("CodeInstancePrecompile"))
(@eval (using CodeInstancePrecompile))
end

empty!(Base.DEPOT_PATH)
append!(Base.DEPOT_PATH, original_depot_path)

5 comments on commit e7a5c36

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some improvements to spmat, presumably from #46790. All else looks like expected levels of noise

Please sign in to comment.