Skip to content

Commit 80728fb

Browse files
vtjnashvtjnash
committed
fixup! avoid invalidations when it doesn't resolve an MethodError
code/design cleanup
1 parent 6f10b8f commit 80728fb

File tree

1 file changed

+79
-106
lines changed

1 file changed

+79
-106
lines changed

‎src/gf.c

Lines changed: 79 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1432,7 +1432,7 @@ JL_DLLEXPORT void jl_method_table_add_backedge(jl_methtable_t *mt, jl_value_t *t
14321432

14331433
struct invalidate_mt_env {
14341434
jl_typemap_entry_t *newentry;
1435-
jl_value_t *shadowed;
1435+
jl_array_t *shadowed;
14361436
size_t max_world;
14371437
int invalidated;
14381438
};
@@ -1442,25 +1442,15 @@ static int invalidate_mt_cache(jl_typemap_entry_t *oldentry, void *closure0)
14421442
JL_GC_PROMISE_ROOTED(env->newentry);
14431443
if (oldentry->max_world == ~(size_t)0) {
14441444
jl_method_instance_t *mi = oldentry->func.linfo;
1445-
jl_method_t *m = mi->def.method;
14461445
int intersects = 0;
1447-
if (jl_is_method(env->shadowed)) {
1448-
if ((jl_value_t*)m == env->shadowed) {
1446+
jl_method_instance_t **d = (jl_method_instance_t**)jl_array_ptr_data(env->shadowed);
1447+
size_t i, n = jl_array_len(env->shadowed);
1448+
for (i = 0; i < n; i++) {
1449+
if (mi == d[i]) {
14491450
intersects = 1;
1451+
break;
14501452
}
14511453
}
1452-
else {
1453-
assert(jl_is_array(env->shadowed));
1454-
jl_method_t **d = (jl_method_t**)jl_array_ptr_data(env->shadowed);
1455-
size_t i, n = jl_array_len(env->shadowed);
1456-
for (i = 0; i < n; i++) {
1457-
if (m == d[i]) {
1458-
intersects = 1;
1459-
break;
1460-
}
1461-
}
1462-
}
1463-
intersects = intersects && !jl_has_empty_intersection((jl_value_t*)oldentry->sig, (jl_value_t*)env->newentry->sig);
14641454
if (intersects) {
14651455
if (_jl_debug_method_invalidation) {
14661456
jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)oldentry);
@@ -1506,20 +1496,12 @@ static jl_typemap_entry_t *do_typemap_search(jl_methtable_t *mt JL_PROPAGATES_RO
15061496
}
15071497
#endif
15081498

1509-
// TODO: decrease repeated work?
1510-
// This implementation is stupidly inefficient, but probably correct
1511-
JL_DLLEXPORT void jl_method_table_disable(jl_methtable_t *mt, jl_method_t *method)
1499+
static void jl_method_table_invalidate(jl_methtable_t *mt, jl_typemap_entry_t *methodentry, jl_method_t *method, size_t max_world)
15121500
{
1513-
if (jl_options.incremental && jl_generating_output())
1514-
jl_printf(JL_STDERR, "WARNING: method deletion during Module precompile may lead to undefined behavior"
1515-
"\n ** incremental compilation may be fatally broken for this module **\n\n");
1516-
jl_typemap_entry_t *methodentry = do_typemap_search(mt, method);
1517-
JL_LOCK(&mt->writelock);
1518-
// Narrow the world age on the method to make it uncallable
1519-
method->deleted_world = methodentry->max_world = jl_world_counter++;
1501+
method->deleted_world = methodentry->max_world = max_world;
15201502
// drop this method from mt->cache
15211503
struct invalidate_mt_env mt_cache_env;
1522-
mt_cache_env.max_world = methodentry->max_world - 1;
1504+
mt_cache_env.max_world = max_world;
15231505
mt_cache_env.newentry = methodentry;
15241506
mt_cache_env.shadowed = NULL;
15251507
mt_cache_env.invalidated = 0;
@@ -1554,6 +1536,17 @@ JL_DLLEXPORT void jl_method_table_disable(jl_methtable_t *mt, jl_method_t *metho
15541536
jl_array_ptr_1d_push(_jl_debug_method_invalidation, loctag);
15551537
JL_GC_POP();
15561538
}
1539+
}
1540+
1541+
JL_DLLEXPORT void jl_method_table_disable(jl_methtable_t *mt, jl_method_t *method)
1542+
{
1543+
if (jl_options.incremental && jl_generating_output())
1544+
jl_printf(JL_STDERR, "WARNING: method deletion during Module precompile may lead to undefined behavior"
1545+
"\n ** incremental compilation may be fatally broken for this module **\n\n");
1546+
jl_typemap_entry_t *methodentry = do_typemap_search(mt, method);
1547+
JL_LOCK(&mt->writelock);
1548+
// Narrow the world age on the method to make it uncallable
1549+
jl_method_table_invalidate(mt, methodentry, method, ++jl_world_counter);
15571550
JL_UNLOCK(&mt->writelock);
15581551
}
15591552

@@ -1564,46 +1557,31 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method
15641557
assert(jl_is_mtable(mt));
15651558
jl_value_t *type = method->sig;
15661559
jl_value_t *oldvalue = NULL;
1560+
jl_array_t *oldmi = NULL;
15671561
if (method->primary_world == 1)
15681562
method->primary_world = ++jl_world_counter;
15691563
size_t max_world = method->primary_world - 1;
1570-
int invalidated = 0;
15711564
jl_value_t *loctag = NULL; // debug info for invalidation
15721565
jl_value_t *isect = NULL;
15731566
jl_typemap_entry_t *newentry = NULL;
1574-
JL_GC_PUSH4(&oldvalue, &newentry, &loctag, &isect);
1567+
JL_GC_PUSH5(&oldvalue, &oldmi, &newentry, &loctag, &isect);
15751568
JL_LOCK(&mt->writelock);
1576-
// first delete the existing entry (we'll disable it later)
1569+
// first find if we have an existing entry to delete
15771570
struct jl_typemap_assoc search = {(jl_value_t*)type, method->primary_world, NULL, 0, ~(size_t)0};
15781571
jl_typemap_entry_t *oldentry = jl_typemap_assoc_by_type(mt->defs, &search, /*offs*/0, /*subtype*/0);
1579-
if (oldentry) {
1580-
oldentry->max_world = method->primary_world - 1;
1581-
// TODO: just append our new entry right here
1582-
}
15831572
// then add our new entry
15841573
newentry = jl_typemap_alloc((jl_tupletype_t*)type, simpletype, jl_emptysvec,
15851574
(jl_value_t*)method, method->primary_world, method->deleted_world);
15861575
jl_typemap_insert(&mt->defs, (jl_value_t*)mt, newentry, 0, &method_defs);
1587-
int any_to_drop = 0;
15881576
if (oldentry) {
15891577
jl_method_t *m = oldentry->func.method;
15901578
method_overwrite(newentry, m);
1591-
jl_svec_t *specializations = jl_atomic_load_acquire(&m->specializations);
1592-
jl_method_instance_t **data = (jl_method_instance_t**)jl_svec_data(specializations);
1593-
size_t i, l = jl_svec_len(specializations);
1594-
for (i = 0; i < l; i++) {
1595-
jl_method_instance_t *mi = jl_atomic_load_relaxed(&data[i]);
1596-
if (mi == NULL)
1597-
continue;
1598-
if (mi->backedges)
1599-
invalidate_backedges(mi, max_world, "jl_method_table_insert");
1600-
}
1601-
any_to_drop = l > 0;
1602-
oldvalue = oldentry->func.value;
1579+
jl_method_table_invalidate(mt, oldentry, m, max_world);
16031580
}
16041581
else {
16051582
oldvalue = get_intersect_matches(mt->defs, newentry);
16061583

1584+
int invalidated = 0;
16071585
jl_method_t **d;
16081586
size_t j, n;
16091587
if (oldvalue == NULL) {
@@ -1652,100 +1630,95 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method
16521630
jl_array_del_end(mt->backedges, na - ins);
16531631
}
16541632
if (oldvalue) {
1633+
oldmi = jl_alloc_vec_any(0);
1634+
enum morespec_options {
1635+
morespec_unknown,
1636+
morespec_isnot,
1637+
morespec_is
1638+
};
16551639
char *morespec = (char*)alloca(n);
1656-
memset(morespec, 0, n);
1640+
memset(morespec, morespec_unknown, n);
16571641
for (j = 0; j < n; j++) {
16581642
jl_method_t *m = d[j];
1659-
if (morespec[j])
1643+
if (morespec[j] == (char)morespec_is)
16601644
continue;
16611645
jl_svec_t *specializations = jl_atomic_load_acquire(&m->specializations);
16621646
jl_method_instance_t **data = (jl_method_instance_t**)jl_svec_data(specializations);
16631647
size_t i, l = jl_svec_len(specializations);
1664-
int invalid = 0;
1665-
int shadowing = 0;
1666-
int ambig = 0;
1648+
enum morespec_options ambig = morespec_unknown;
16671649
for (i = 0; i < l; i++) {
16681650
jl_method_instance_t *mi = jl_atomic_load_relaxed(&data[i]);
16691651
if (mi == NULL)
16701652
continue;
1671-
isect = jl_type_intersection(type, (jl_value_t*)mi->specTypes);
1653+
isect = jl_type_intersection(m->sig, (jl_value_t*)mi->specTypes);
1654+
isect = jl_type_intersection(type, isect);
16721655
if (isect != jl_bottom_type) {
1673-
if (shadowing == 0) {
1674-
if (jl_type_morespecific(m->sig, type))
1675-
// not actually shadowing--the existing method is still better
1676-
break;
1677-
shadowing = 1;
1678-
ambig = !jl_type_morespecific(type, m->sig);
1679-
}
1656+
if (morespec[j] == (char)morespec_unknown)
1657+
morespec[j] = (char)jl_type_morespecific(m->sig, type) ? morespec_is : morespec_isnot;
1658+
if (morespec[j] == (char)morespec_is)
1659+
// not actually shadowing--the existing method is still better
1660+
break;
1661+
if (ambig == morespec_unknown)
1662+
ambig = jl_type_morespecific(type, m->sig) ? morespec_is : morespec_isnot;
16801663
// replacing a method--see if this really was the selected method previously
16811664
// over the intersection
1682-
if (ambig) {
1665+
if (ambig == morespec_isnot) {
16831666
size_t k;
16841667
for (k = 0; k < n; k++) {
16851668
jl_method_t *m2 = d[k];
16861669
if (m == m2 || !jl_subtype(isect, m2->sig))
16871670
continue;
1688-
if (morespec[k])
1671+
if (morespec[k] == (char)morespec_unknown)
1672+
morespec[k] = (char)jl_type_morespecific(m2->sig, type) ? morespec_is : morespec_isnot;
1673+
if (morespec[k] == (char)morespec_is)
1674+
// not actually shadowing this--m2 will still be better
16891675
break;
1690-
if (k > j) { // possibly haven't actually computed morespec yet
1691-
if (jl_type_morespecific(m2->sig, type)) {
1692-
// not actually shadowing this--m2 will still be better
1693-
morespec[k] = 1;
1694-
break;
1695-
}
1696-
}
16971676
// since m2 was also a previous match over isect,
1698-
// see if m was also previously dominant over m2
1677+
// see if m was also previously dominant over all m2
16991678
if (!jl_type_morespecific(m->sig, m2->sig))
17001679
break;
17011680
}
17021681
if (k != n)
17031682
continue;
17041683
}
1705-
invalid = 1;
1706-
if (mi->backedges)
1684+
jl_array_ptr_1d_push(oldmi, (jl_value_t*)mi);
1685+
if (mi->backedges) {
1686+
invalidated = 1;
17071687
invalidate_backedges(mi, max_world, "jl_method_table_insert");
1688+
}
17081689
}
17091690
}
1710-
if (invalid == 0)
1711-
morespec[j] = 1; // the method won't need to be dropped from any cache
1712-
}
1713-
for (j = 0; j < n; j++) {
1714-
if (morespec[j])
1715-
d[j] = NULL;
1716-
else
1717-
any_to_drop = 1;
17181691
}
1719-
}
1720-
}
1721-
if (any_to_drop) {
1722-
// drop anything in mt->cache that might overlap with the new method
1723-
struct invalidate_mt_env mt_cache_env;
1724-
mt_cache_env.max_world = max_world;
1725-
mt_cache_env.shadowed = oldvalue;
1726-
mt_cache_env.newentry = newentry;
1727-
mt_cache_env.invalidated = 0;
1728-
1729-
jl_typemap_visitor(mt->cache, invalidate_mt_cache, (void*)&mt_cache_env);
1730-
jl_array_t *leafcache = mt->leafcache;
1731-
size_t i, l = jl_array_len(leafcache);
1732-
for (i = 1; i < l; i += 2) {
1733-
jl_value_t *entry = jl_array_ptr_ref(leafcache, i);
1734-
if (entry) {
1735-
while (entry != jl_nothing) {
1736-
invalidate_mt_cache((jl_typemap_entry_t*)entry, (void*)&mt_cache_env);
1737-
entry = (jl_value_t*)((jl_typemap_entry_t*)entry)->next;
1692+
if (jl_array_len(oldmi)) {
1693+
// search mt->cache and leafcache and drop anything that might overlap with the new method
1694+
// TODO: keep track of just the `mi` for which shadowing was true (to avoid recomputing that here)
1695+
struct invalidate_mt_env mt_cache_env;
1696+
mt_cache_env.max_world = max_world;
1697+
mt_cache_env.shadowed = oldmi;
1698+
mt_cache_env.newentry = newentry;
1699+
mt_cache_env.invalidated = 0;
1700+
1701+
jl_typemap_visitor(mt->cache, invalidate_mt_cache, (void*)&mt_cache_env);
1702+
jl_array_t *leafcache = mt->leafcache;
1703+
size_t i, l = jl_array_len(leafcache);
1704+
for (i = 1; i < l; i += 2) {
1705+
jl_value_t *entry = jl_array_ptr_ref(leafcache, i);
1706+
if (entry) {
1707+
while (entry != jl_nothing) {
1708+
invalidate_mt_cache((jl_typemap_entry_t*)entry, (void*)&mt_cache_env);
1709+
entry = (jl_value_t*)((jl_typemap_entry_t*)entry)->next;
1710+
}
1711+
}
17381712
}
17391713
}
17401714
}
1741-
invalidated = 1;
1742-
}
1743-
if (invalidated && _jl_debug_method_invalidation) {
1744-
jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)method);
1745-
loctag = jl_cstr_to_string("jl_method_table_insert");
1746-
jl_array_ptr_1d_push(_jl_debug_method_invalidation, loctag);
1715+
if (invalidated && _jl_debug_method_invalidation) {
1716+
jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)method);
1717+
loctag = jl_cstr_to_string("jl_method_table_insert");
1718+
jl_array_ptr_1d_push(_jl_debug_method_invalidation, loctag);
1719+
}
1720+
update_max_args(mt, type);
17471721
}
1748-
update_max_args(mt, type);
17491722
JL_UNLOCK(&mt->writelock);
17501723
JL_GC_POP();
17511724
}

0 commit comments

Comments
 (0)