Skip to content

Commit

Permalink
add many missing backedges getting lost during incremental deserializ…
Browse files Browse the repository at this point in the history 
…ation

we need to restore not only backedges directly to the new methods,
but also to any methods whos backedges have not been inferred
but which might propagate invalidation changes to the new code

fix #21141
  • Loading branch information
@vtjnash
vtjnash committed Apr 10, 2017
1 parent 6495312 commit 61cdfc4
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 35 deletions.
151 changes: 118 additions & 33 deletions src/dump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ static arraylist_t reinit_list;
// (only used by the incremental serializer in MODE_MODULE)
static jl_array_t *serializer_worklist;

// inverse of backedges tree
// (only used by the incremental serializer in MODE_MODULE)
htable_t edges_map;

// list of modules being deserialized with __init__ methods
// (not used in MODE_AST)
jl_array_t *jl_module_init_order;
Expand Down Expand Up @@ -1152,30 +1156,42 @@ static void jl_serialize_missing_backedges_to_mod(jl_serializer_state *s, jl_met
size_t i, l = jl_array_len(backedges);
for (i = 1; i < l; i += 2) {
jl_method_instance_t *caller = (jl_method_instance_t*)jl_array_ptr_ref(backedges, i);
if (caller->max_world == ~(size_t)0 && module_in_worklist(caller->def->module)) {
jl_serialize_value(s, caller);
jl_serialize_value(s, jl_array_ptr_ref(backedges, i - 1));
if (caller->max_world == ~(size_t)0) {
jl_value_t *missing_callee = jl_array_ptr_ref(backedges, i - 1);
jl_array_t **edges = (jl_array_t**)ptrhash_bp(&edges_map, (void*)caller);
if (*edges == HT_NOTFOUND)
*edges = jl_alloc_vec_any(0);
jl_array_ptr_1d_push(*edges, missing_callee);
}
}
}
}

static int jl_serialize_backedges_to_mod(jl_typemap_entry_t *ml, void *closure)
// the intent of this function is to invert the backedges tree
static void serialize_backedges(jl_method_instance_t *callee)
{
jl_serializer_state *s = (jl_serializer_state*)closure;
jl_method_instance_t *callee = ml->func.linfo;
jl_array_t *backedges = callee->backedges;
if (backedges) {
assert(callee->max_world == ~(size_t)0);
size_t i, l = jl_array_len(backedges);
for (i = 0; i < l; i++) {
jl_method_instance_t *caller = (jl_method_instance_t*)jl_array_ptr_ref(backedges, i);
if (caller->max_world == ~(size_t)0 && module_in_worklist(caller->def->module)) {
jl_serialize_value(s, caller);
jl_serialize_value(s, callee);
if (caller->max_world == ~(size_t)0) {
jl_array_t **edges = (jl_array_t**)ptrhash_bp(&edges_map, caller);
if (*edges == HT_NOTFOUND)
*edges = jl_alloc_vec_any(0);
jl_array_ptr_1d_push(*edges, (jl_value_t*)callee);
}
}
}
}


static int jl_serialize_backedges_to_mod(jl_typemap_entry_t *ml, void *closure)
{
(void)(jl_serializer_state*)closure;
jl_method_instance_t *callee = ml->func.linfo;
serialize_backedges(callee);
return 1;
}

Expand Down Expand Up @@ -1233,6 +1249,36 @@ static void jl_serialize_lambdas_from_mod(jl_serializer_state *s, jl_module_t *m
}
}

static void jl_serialize_backedges(jl_serializer_state *s)
{
arraylist_t worklist;
arraylist_new(&worklist, 0);
size_t i;
void **table = edges_map.table;
for (i = 0; i < edges_map.size; i += 2) {
jl_method_instance_t *caller = (jl_method_instance_t*)table[i];
jl_array_t *callee = (jl_array_t*)table[i + 1];
if (callee != HT_NOTFOUND && module_in_worklist(caller->def->module)) {
arraylist_push(&worklist, (void*)caller);
}
}
while (worklist.len) {
jl_method_instance_t *caller = (jl_method_instance_t*)arraylist_pop(&worklist);
jl_array_t **pcallee = (jl_array_t**)ptrhash_bp(&edges_map, (void*)caller),
*callee = *pcallee;
if (callee != HT_NOTFOUND) {
jl_serialize_value(s, caller);
jl_serialize_value(s, callee);
*pcallee = HT_NOTFOUND;
for (i = 0; i < jl_array_len(callee); i++) {
jl_value_t *c = jl_array_ptr_ref(callee, i);
if (jl_is_method_instance(c))
arraylist_push(&worklist, (void*)c);
}
}
}
}

// serialize information about all of the modules accessible directly from Main
static void write_mod_list(ios_t *s)
{
Expand Down Expand Up @@ -2084,7 +2130,7 @@ typedef struct _linkedlist_t {
};
struct {
jl_method_instance_t *caller;
jl_value_t *callee;
jl_array_t *callee;
};
} def[100];
size_t count;
Expand Down Expand Up @@ -2118,12 +2164,11 @@ static void jl_insert_methods(linkedlist_t *list)
while (list) {
size_t i;
for (i = 0; i < list->count; i++) {
if (jl_is_method(list->def[i].meth)) {
jl_method_t *meth = list->def[i].meth;
jl_datatype_t *gf = jl_first_argument_datatype((jl_value_t*)meth->sig);
assert(jl_is_datatype(gf) && gf->name->mt);
jl_method_table_insert(gf->name->mt, meth, list->def[i].simpletype);
}
assert(jl_is_method(list->def[i].meth));
jl_method_t *meth = list->def[i].meth;
jl_datatype_t *gf = jl_first_argument_datatype((jl_value_t*)meth->sig);
assert(jl_is_datatype(gf) && gf->name->mt);
jl_method_table_insert(gf->name->mt, meth, list->def[i].simpletype);
}
list = list->next;
}
Expand All @@ -2133,23 +2178,56 @@ void jl_method_instance_delete(jl_method_instance_t *mi);
static void jl_insert_backedges(linkedlist_t *list)
{
while (list) {
size_t i;
size_t i, j;
for (i = 0; i < list->count; i++) {
if (!jl_is_method(list->def[i].meth)) {
jl_method_instance_t *caller = list->def[i].caller;
assert(jl_is_method_instance(caller));
jl_value_t *callee = list->def[i].callee;
if (jl_is_method_instance(callee)) {
jl_method_instance_t *callee_mi = (jl_method_instance_t*)callee;
if (callee_mi->max_world == ~(size_t)0)
jl_method_instance_add_backedge(callee_mi, caller);
else
jl_method_instance_delete(caller);
}
else {
jl_datatype_t *gf = jl_first_argument_datatype(callee);
assert(jl_is_datatype(gf) && gf->name->mt);
jl_method_table_add_backedge(gf->name->mt, callee, (jl_value_t*)caller);
jl_method_instance_t *caller = list->def[i].caller;
assert(jl_is_method_instance(caller));
jl_array_t *callees = list->def[i].callee;
assert(jl_is_array(callees));
if (!caller->inferred || module_in_worklist(caller->def->module)) {
for (j = 0; j < jl_array_len(callees); j++) {
jl_value_t *callee = jl_array_ptr_ref(callees, j);
if (jl_is_method_instance(callee)) {
jl_method_instance_t *callee_mi = (jl_method_instance_t*)callee;
if (!module_in_worklist(callee_mi->def->module)) {
// verify that this MethodInstance is still the correct lookup for this callee sig
jl_value_t *sig = callee_mi->specTypes;
jl_method_t *m = NULL;
if (jl_is_datatype(sig)) {
jl_datatype_t *ftype = jl_first_argument_datatype(sig);
jl_methtable_t *mt = ftype->name->mt;
jl_typemap_entry_t *entry = jl_typemap_assoc_by_type(
mt->defs, (jl_datatype_t*)sig,
NULL, /*inexact*/0, /*subtype*/1, 0, jl_world_counter);
if (entry != NULL)
m = entry->func.method;
}
if (m != callee_mi->def) {
// probably no good, just invalidate everything now
jl_method_instance_delete(caller);
break;
}
}
if (callee_mi->max_world == ~(size_t)0) {
// if this callee is still valid, just add a backedege
jl_method_instance_add_backedge(callee_mi, caller);
}
else if (caller->min_world == jl_world_counter) {
// if this caller was just added, delete everything associated with it
jl_method_instance_delete(caller);
break;
}
else {
// the caller must have been something pre-existing
// assume that it'll take care of deleting the deserialized code
// whenever we process it here
}
}
else {
jl_datatype_t *gf = jl_first_argument_datatype(callee);
assert(jl_is_datatype(gf) && gf->name->mt);
jl_method_table_add_backedge(gf->name->mt, callee, (jl_value_t*)caller);
}
}
}
}
Expand Down Expand Up @@ -2800,6 +2878,7 @@ JL_DLLEXPORT int jl_save_incremental(const char *fname, jl_array_t *worklist)
// best to keep it early (before any actual initialization)

arraylist_new(&reinit_list, 0);
htable_new(&edges_map, 0);
htable_new(&backref_table, 5000);
ptrhash_put(&backref_table, jl_main_module, (char*)HT_NOTFOUND + 1);
backref_table_numel = 1;
Expand All @@ -2814,10 +2893,13 @@ JL_DLLEXPORT int jl_save_incremental(const char *fname, jl_array_t *worklist)
jl_serialize_value(&s, worklist);
jl_serialize_lambdas_from_mod(&s, jl_main_module);
jl_serialize_value(&s, NULL); // signal end of lambdas
jl_serialize_backedges(&s);
jl_serialize_value(&s, NULL); // signal end of backedges
jl_finalize_serializer(&s); // done with f
serializer_worklist = NULL;

jl_gc_enable(en);
htable_reset(&edges_map, 0);
htable_reset(&backref_table, 0);
arraylist_free(&reinit_list);
ios_close(&f);
Expand Down Expand Up @@ -3167,6 +3249,8 @@ static jl_value_t *_jl_restore_incremental(ios_t *f)
// get list of external generic functions
linkedlist_t external_methods;
jl_deserialize_methods_from_mod(&s, &external_methods);
linkedlist_t external_backedges;
jl_deserialize_methods_from_mod(&s, &external_backedges);

arraylist_t *tracee_list = NULL;
if (jl_newmeth_tracer)
Expand All @@ -3178,8 +3262,9 @@ static jl_value_t *_jl_restore_incremental(ios_t *f)
init_order = jl_finalize_deserializer(&s, tracee_list); // done with f and s (needs to be after recache types)
jl_insert_methods(&external_methods); // hook up methods of external generic functions (needs to be after recache types)
jl_recache_other(&dependent_worlds); // make all of the other objects identities correct (needs to be after insert methods)
jl_insert_backedges(&external_methods); // restore external backedges (needs to be after recache other)
jl_insert_backedges(&external_backedges); // restore external backedges (needs to be after recache other)
free_linkedlist(external_methods.next);
free_linkedlist(external_backedges.next);
serializer_worklist = NULL;

arraylist_free(&flagref_list);
Expand Down
47 changes: 45 additions & 2 deletions test/compile.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Base.Test

Foo_module = :Foo4b3a94a1a081a8cb
Foo2_module = :F2oo4b3a94a1a081a8cb
FooBase_module = :FooBase4b3a94a1a081a8cb
@eval module ConflictingBindings
export $Foo_module, $FooBase_module
Expand All @@ -21,6 +22,7 @@ insert!(LOAD_PATH, 1, dir)
insert!(Base.LOAD_CACHE_PATH, 1, dir)
try
Foo_file = joinpath(dir, "$Foo_module.jl")
Foo2_file = joinpath(dir, "$Foo2_module.jl")
FooBase_file = joinpath(dir, "$FooBase_module.jl")

write(FooBase_file,
Expand All @@ -30,12 +32,23 @@ try
module $FooBase_module
end
""")
write(Foo2_file,
"""
__precompile__(true)
module $Foo2_module
export override
override(x::Integer) = 2
override(x::AbstractFloat) = Float64(override(1))
end
""")
write(Foo_file,
"""
__precompile__(true)
module $Foo_module
using $FooBase_module
import $Foo2_module: $Foo2_module, override
# test that docs get reconnected
@doc "foo function" foo(x) = x + 1
Expand Down Expand Up @@ -109,16 +122,39 @@ try
ccall(:jl_specializations_get_linfo, Ref{Core.MethodInstance}, (Any, Any, Any, UInt),
some_method, Tuple{typeof(Base.include), String}, Core.svec(), typemax(UInt))
end
g() = override(1.0)
Base.Test.@test g() === 2.0 # compile this
end
""")
@test_throws ErrorException Core.kwfunc(Base.nothing) # make sure `nothing` didn't have a kwfunc (which would invalidate the attempted test)

# Issue #12623
@test __precompile__(true) === nothing

# Issue #21307
Base.require(Foo2_module)
@eval let Foo2_module = $(QuoteNode(Foo2_module)) # use @eval to see the results of loading the compile
Foo = getfield(Main, Foo2_module)
Foo.override(::Int) = 'a'
Foo.override(::Float32) = 'b'
end

Base.require(Foo_module)
cachefile = joinpath(dir, "$Foo_module.ji")

@eval let Foo_module = $(QuoteNode(Foo_module)), # use @eval to see the results of loading the compile
Foo = getfield(Main, Foo_module)
@test Foo.foo(17) == 18
@test Foo.Bar.bar(17) == 19

# Issue #21307
@test Foo.g() === 97.0
@test Foo.override(1.0e0) == Float64('a')
@test Foo.override(1.0f0) == 'b'
@test Foo.override(UInt(1)) == 2
end

cachefile = joinpath(dir, "$Foo_module.ji")
# use _require_from_serialized to ensure that the test fails if
# the module doesn't reload from the image:
@test_warn "WARNING: replacing module Foo4b3a94a1a081a8cb.\nWARNING: Method definition " begin
Expand All @@ -129,6 +165,7 @@ try
@test_throws MethodError Foo.foo(17) # world shouldn't be visible yet
end
@eval let Foo_module = $(QuoteNode(Foo_module)), # use @eval to see the results of loading the compile
Foo2_module = $(QuoteNode(Foo2_module)),
FooBase_module = $(QuoteNode(FooBase_module)),
Foo = getfield(Main, Foo_module),
dir = $(QuoteNode(dir)),
Expand All @@ -137,6 +174,12 @@ try
@test Foo.foo(17) == 18
@test Foo.Bar.bar(17) == 19

# Issue #21307
@test Foo.g() === 97.0
@test Foo.override(1.0e0) == Float64('a')
@test Foo.override(1.0f0) == 'b'
@test Foo.override(UInt(1)) == 2

# issue #12284:
@test stringmime("text/plain", Base.Docs.doc(Foo.foo)) == "foo function\n"
@test stringmime("text/plain", Base.Docs.doc(Foo.Bar.bar)) == "bar function\n"
Expand All @@ -147,7 +190,7 @@ try

modules, deps1 = Base.cache_dependencies(cachefile)
@test sort(modules) == Any[(s, Base.module_uuid(getfield(Foo, s))) for s in
[:Base, :Core, FooBase_module, :Main]]
[:Base, :Core, Foo2_module, FooBase_module, :Main]]
@test deps == deps1

@test current_task()(0x01, 0x4000, 0x30031234) == 2
Expand Down

0 comments on commit 61cdfc4

Please sign in to comment.