From ead1fd9c42b1d970827c9e6720c5072332414905 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 17 Feb 2022 10:23:52 -0600 Subject: [PATCH] Fix a failure to invalidate --- src/dump.c | 39 ++++++++++++++++++++++++++++++++++++--- src/gf.c | 17 ++++++++++------- src/julia_internal.h | 1 + 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/dump.c b/src/dump.c index cf971b768608f..0c279b43d1c19 100644 --- a/src/dump.c +++ b/src/dump.c @@ -2160,12 +2160,45 @@ static void jl_insert_methods(jl_array_t *list) } } +void remove_code_instance_from_validation(jl_code_instance_t *codeinst) +{ + ptrhash_remove(&new_code_instance_validate, codeinst); +} + static void jl_insert_method_instances(jl_array_t *list) { - size_t i, l = jl_array_len(list); - for (i = 0; i < l; i++) { + size_t i, l = jl_array_len(list), ll = l; + // Validate the MethodInstances + size_t world = jl_atomic_load_acquire(&jl_world_counter); + for (i = 0; i < ll; i++) { jl_method_instance_t *mi = (jl_method_instance_t*)jl_array_ptr_ref(list, i); assert(jl_is_method_instance(mi)); + if (jl_is_method(mi->def.method)) { + // Is this still the method we'd be calling? + jl_methtable_t *mt = jl_method_table_for(mi->specTypes); + jl_value_t *mworld = jl_methtable_lookup(mt, mi->specTypes, world); + if (jl_is_method(mworld) && mi->def.method != (jl_method_t*)mworld) { + invalidate_backedges(&remove_code_instance_from_validation, mi, world, "jl_insert_method_instance"); + // The codeinst of this mi haven't yet been removed + jl_code_instance_t *codeinst = mi->cache; + while (codeinst) { + remove_code_instance_from_validation(codeinst); + codeinst = codeinst->next; + } + if (_jl_debug_method_invalidation) { + jl_array_ptr_1d_push(_jl_debug_method_invalidation, mworld); + jl_array_ptr_1d_push(_jl_debug_method_invalidation, jl_cstr_to_string("jl_method_table_insert")); // GC disabled + } + jl_array_ptr_set(list, i, jl_array_ptr_ref(list, ll-1)); + jl_array_ptr_set(list, ll-1, mi); + ll--; + } + } + } + // While it's tempting to just remove the invalidated MIs altogether (loop only up to `ll`), + // this hurts the ability of SnoopCompile to diagnose problems. + for (i = 0; i < l; i++) { + jl_method_instance_t *mi = (jl_method_instance_t*)jl_array_ptr_ref(list, i); jl_method_instance_t *milive = jl_specializations_get_or_insert(mi); ptrhash_put(&uniquing_table, mi, milive); // store the association for the 2nd pass } @@ -2315,7 +2348,7 @@ static void jl_insert_backedges(jl_array_t *list, jl_array_t *targets) // then enable it jl_code_instance_t *codeinst = caller->cache; while (codeinst) { - if (codeinst->min_world > 0) + if (ptrhash_get(&new_code_instance_validate, codeinst) != HT_NOTFOUND && codeinst->min_world > 0) codeinst->max_world = ~(size_t)0; ptrhash_remove(&new_code_instance_validate, codeinst); // mark it as handled codeinst = jl_atomic_load_relaxed(&codeinst->next); diff --git a/src/gf.c b/src/gf.c index a6b1f693d1f78..101c2bd952708 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1403,8 +1403,10 @@ static void invalidate_external(jl_method_instance_t *mi, size_t max_world) { } } +static void do_nothing_with_codeinst(jl_code_instance_t *ci) {} + // recursively invalidate cached methods that had an edge to a replaced method -static void invalidate_method_instance(jl_method_instance_t *replaced, size_t max_world, int depth) +static void invalidate_method_instance(void (*f)(jl_code_instance_t*), jl_method_instance_t *replaced, size_t max_world, int depth) { if (_jl_debug_method_invalidation) { jl_value_t *boxeddepth = NULL; @@ -1424,6 +1426,7 @@ static void invalidate_method_instance(jl_method_instance_t *replaced, size_t ma codeinst->max_world = max_world; } assert(codeinst->max_world <= max_world); + (*f)(codeinst); codeinst = jl_atomic_load_relaxed(&codeinst->next); } // recurse to all backedges to update their valid range also @@ -1433,14 +1436,14 @@ static void invalidate_method_instance(jl_method_instance_t *replaced, size_t ma size_t i, l = jl_array_len(backedges); for (i = 0; i < l; i++) { jl_method_instance_t *replaced = (jl_method_instance_t*)jl_array_ptr_ref(backedges, i); - invalidate_method_instance(replaced, max_world, depth + 1); + invalidate_method_instance(f, replaced, max_world, depth + 1); } } JL_UNLOCK(&replaced->def.method->writelock); } // invalidate cached methods that overlap this definition -static void invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, const char *why) +void invalidate_backedges(void (*f)(jl_code_instance_t*), jl_method_instance_t *replaced_mi, size_t max_world, const char *why) { JL_LOCK(&replaced_mi->def.method->writelock); jl_array_t *backedges = replaced_mi->backedges; @@ -1450,7 +1453,7 @@ static void invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_w size_t i, l = jl_array_len(backedges); jl_method_instance_t **replaced = (jl_method_instance_t**)jl_array_ptr_data(backedges); for (i = 0; i < l; i++) { - invalidate_method_instance(replaced[i], max_world, 1); + invalidate_method_instance(f, replaced[i], max_world, 1); } } JL_UNLOCK(&replaced_mi->def.method->writelock); @@ -1613,7 +1616,7 @@ static void jl_method_table_invalidate(jl_methtable_t *mt, jl_typemap_entry_t *m if ((jl_value_t*)mi != jl_nothing) { invalidated = 1; invalidate_external(mi, methodentry->max_world); - invalidate_backedges(mi, methodentry->max_world, "jl_method_table_disable"); + invalidate_backedges(&do_nothing_with_codeinst, mi, methodentry->max_world, "jl_method_table_disable"); } } if (invalidated && _jl_debug_method_invalidation) { @@ -1744,7 +1747,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method if (missing) { jl_method_instance_t *backedge = (jl_method_instance_t*)backedges[i]; invalidate_external(backedge, max_world); - invalidate_method_instance(backedge, max_world, 0); + invalidate_method_instance(&do_nothing_with_codeinst, backedge, max_world, 0); invalidated = 1; if (_jl_debug_method_invalidation) jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)backedgetyp); @@ -1814,7 +1817,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method invalidate_external(mi, max_world); if (mi->backedges) { invalidated = 1; - invalidate_backedges(mi, max_world, "jl_method_table_insert"); + invalidate_backedges(&do_nothing_with_codeinst, mi, max_world, "jl_method_table_insert"); } } } diff --git a/src/julia_internal.h b/src/julia_internal.h index bbd81cf3b1fec..21c46129b7149 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -223,6 +223,7 @@ extern tracer_cb jl_newmeth_tracer; void jl_call_tracer(tracer_cb callback, jl_value_t *tracee); void print_func_loc(JL_STREAM *s, jl_method_t *m); extern jl_array_t *_jl_debug_method_invalidation JL_GLOBALLY_ROOTED; +void invalidate_backedges(void (*f)(jl_code_instance_t*), jl_method_instance_t *replaced_mi, size_t max_world, const char *why); extern JL_DLLEXPORT size_t jl_page_size; extern jl_function_t *jl_typeinf_func;