Skip to content

Commit abf4dcf

Browse files
committed
Be more selective when invalidating code instances
On master, when we invalidate a CodeInstance, we also invalidate the entire associated MethodInstance. However, this is highly problematic, because we have a lot of CodeInstances that are associated with `getproperty(::Module, ::Symbol)` through constant propagation. If one of these CodeInstances gets invalidated (e.g. because the resolution of const-propagated M.s binding changed), it would invalidate essentially the entire world. Prevent this by re-checking the forward edges list to make sure that the code instance we're invalidating is actually in there.
1 parent 0c0419e commit abf4dcf

File tree

1 file changed

+37
-12
lines changed

1 file changed

+37
-12
lines changed

src/gf.c

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1839,7 +1839,7 @@ JL_DLLEXPORT jl_value_t *jl_debug_method_invalidation(int state)
18391839
return jl_nothing;
18401840
}
18411841

1842-
static void _invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, int depth);
1842+
static void _invalidate_backedges(jl_method_instance_t *replaced_mi, jl_code_instance_t *replaced_ci, size_t max_world, int depth);
18431843

18441844
// recursively invalidate cached methods that had an edge to a replaced method
18451845
static void invalidate_code_instance(jl_code_instance_t *replaced, size_t max_world, int depth)
@@ -1858,13 +1858,15 @@ static void invalidate_code_instance(jl_code_instance_t *replaced, size_t max_wo
18581858
if (!jl_is_method(replaced_mi->def.method))
18591859
return; // shouldn't happen, but better to be safe
18601860
JL_LOCK(&replaced_mi->def.method->writelock);
1861-
if (jl_atomic_load_relaxed(&replaced->max_world) == ~(size_t)0) {
1861+
size_t replacedmaxworld = jl_atomic_load_relaxed(&replaced->max_world);
1862+
if (replacedmaxworld == ~(size_t)0) {
18621863
assert(jl_atomic_load_relaxed(&replaced->min_world) - 1 <= max_world && "attempting to set illogical world constraints (probable race condition)");
18631864
jl_atomic_store_release(&replaced->max_world, max_world);
1865+
// recurse to all backedges to update their valid range also
1866+
_invalidate_backedges(replaced_mi, replaced, max_world, depth + 1);
1867+
} else {
1868+
assert(jl_atomic_load_relaxed(&replaced->max_world) <= max_world);
18641869
}
1865-
assert(jl_atomic_load_relaxed(&replaced->max_world) <= max_world);
1866-
// recurse to all backedges to update their valid range also
1867-
_invalidate_backedges(replaced_mi, max_world, depth + 1);
18681870
JL_UNLOCK(&replaced_mi->def.method->writelock);
18691871
}
18701872

@@ -1873,19 +1875,42 @@ JL_DLLEXPORT void jl_invalidate_code_instance(jl_code_instance_t *replaced, size
18731875
invalidate_code_instance(replaced, max_world, 1);
18741876
}
18751877

1876-
static void _invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, int depth) {
1878+
static void _invalidate_backedges(jl_method_instance_t *replaced_mi, jl_code_instance_t *replaced_ci, size_t max_world, int depth) {
18771879
jl_array_t *backedges = replaced_mi->backedges;
18781880
if (backedges) {
18791881
// invalidate callers (if any)
18801882
replaced_mi->backedges = NULL;
18811883
JL_GC_PUSH1(&backedges);
18821884
size_t i = 0, l = jl_array_nrows(backedges);
1885+
size_t ins = 0;
18831886
jl_code_instance_t *replaced;
18841887
while (i < l) {
1885-
i = get_next_edge(backedges, i, NULL, &replaced);
1888+
jl_value_t *invokesig = NULL;
1889+
i = get_next_edge(backedges, i, &invokesig, &replaced);
18861890
JL_GC_PROMISE_ROOTED(replaced); // propagated by get_next_edge from backedges
1891+
if (replaced_ci) {
1892+
// If we're invalidating a particular codeinstance, only invalidate
1893+
// this backedge it actually has an edge for our codeinstance.
1894+
jl_svec_t *edges = jl_atomic_load_relaxed(&replaced->edges);
1895+
for (size_t j = 0; j < jl_svec_len(edges); ++j) {
1896+
jl_value_t *edge = jl_svecref(edges, j);
1897+
if (edge == (jl_value_t*)replaced_mi || edge == (jl_value_t*)replaced_ci)
1898+
goto found;
1899+
}
1900+
// Keep this entry in the backedge list, but compact it
1901+
ins = set_next_edge(backedges, ins, invokesig, replaced);
1902+
continue;
1903+
found:;
1904+
}
18871905
invalidate_code_instance(replaced, max_world, depth);
18881906
}
1907+
if (replaced_ci && ins != 0) {
1908+
jl_array_del_end(backedges, l - ins);
1909+
// If we're only invalidating one ci, we don't know which ci any particular
1910+
// backedge was for, so we can't delete them. Put them back.
1911+
replaced_mi->backedges = backedges;
1912+
jl_gc_wb(replaced_mi, backedges);
1913+
}
18891914
JL_GC_POP();
18901915
}
18911916
}
@@ -1894,7 +1919,7 @@ static void _invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_
18941919
static void invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, const char *why)
18951920
{
18961921
JL_LOCK(&replaced_mi->def.method->writelock);
1897-
_invalidate_backedges(replaced_mi, max_world, 1);
1922+
_invalidate_backedges(replaced_mi, NULL, max_world, 1);
18981923
JL_UNLOCK(&replaced_mi->def.method->writelock);
18991924
if (why && _jl_debug_method_invalidation) {
19001925
jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)replaced_mi);
@@ -1928,8 +1953,8 @@ JL_DLLEXPORT void jl_method_instance_add_backedge(jl_method_instance_t *callee,
19281953
size_t i = 0, l = jl_array_nrows(callee->backedges);
19291954
for (i = 0; i < l; i++) {
19301955
// optimized version of while (i < l) i = get_next_edge(callee->backedges, i, &invokeTypes, &mi);
1931-
jl_value_t *mi = jl_array_ptr_ref(callee->backedges, i);
1932-
if (mi != (jl_value_t*)caller)
1956+
jl_value_t *ciedge = jl_array_ptr_ref(callee->backedges, i);
1957+
if (ciedge != (jl_value_t*)caller)
19331958
continue;
19341959
jl_value_t *invokeTypes = i > 0 ? jl_array_ptr_ref(callee->backedges, i - 1) : NULL;
19351960
if (invokeTypes && jl_is_method_instance(invokeTypes))
@@ -2372,7 +2397,7 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
23722397
continue;
23732398
loctag = jl_atomic_load_relaxed(&m->specializations); // use loctag for a gcroot
23742399
_Atomic(jl_method_instance_t*) *data;
2375-
size_t i, l;
2400+
size_t l;
23762401
if (jl_is_svec(loctag)) {
23772402
data = (_Atomic(jl_method_instance_t*)*)jl_svec_data(loctag);
23782403
l = jl_svec_len(loctag);
@@ -2382,7 +2407,7 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
23822407
l = 1;
23832408
}
23842409
enum morespec_options ambig = morespec_unknown;
2385-
for (i = 0; i < l; i++) {
2410+
for (size_t i = 0; i < l; i++) {
23862411
jl_method_instance_t *mi = jl_atomic_load_relaxed(&data[i]);
23872412
if ((jl_value_t*)mi == jl_nothing)
23882413
continue;

0 commit comments

Comments
 (0)