Skip to content

Commit fb1c21c

Browse files
vtjnashvtjnash
authored
function cache: fix memory leak (#26982)
some lookup functions (most notably, jl_get_specialization1) were bypassing the initial cache lookup, resulting in repeatedly trying to re-cache them, and wasting a small amount memory also remove some fast-path code for threading race conditions: the benefit is too negligible to be worth the extra code maintenance
1 parent 365d389 commit fb1c21c

File tree

1 file changed

+39
-72
lines changed

1 file changed

+39
-72
lines changed

‎src/gf.c

Lines changed: 39 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,10 @@ static jl_method_instance_t *cache_method(
880880
int allow_exec)
881881
{
882882
// caller must hold the mt->writelock
883+
jl_typemap_entry_t *entry = jl_typemap_assoc_by_type(*cache, (jl_value_t*)tt, NULL, /*subtype*/1, jl_cachearg_offset(mt), world, /*max_world_mask*/0);
884+
if (entry && entry->func.value)
885+
return (jl_method_instance_t*)entry->func.value;
886+
883887
jl_value_t *decl = (jl_value_t*)definition->sig;
884888
jl_value_t *temp = NULL;
885889
jl_value_t *temp2 = NULL;
@@ -1082,12 +1086,17 @@ static jl_method_instance_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_datatype
10821086
{
10831087
// caller must hold the mt->writelock
10841088
jl_typemap_entry_t *entry = NULL;
1089+
entry = jl_typemap_assoc_by_type(mt->cache, (jl_value_t*)tt, NULL, /*subtype*/1, jl_cachearg_offset(mt), world, /*max_world_mask*/0);
1090+
if (entry && entry->func.value) {
1091+
assert(entry->func.linfo->min_world <= entry->min_world && entry->func.linfo->max_world >= entry->max_world &&
1092+
"typemap consistency error: MethodInstance doesn't apply to full range of its entry");
1093+
return entry->func.linfo;
1094+
}
1095+
1096+
jl_method_instance_t *nf = NULL;
10851097
jl_svec_t *env = jl_emptysvec;
1086-
jl_method_t *func = NULL;
10871098
jl_tupletype_t *sig = NULL;
1088-
jl_method_instance_t *nf = NULL;
1089-
JL_GC_PUSH4(&env, &entry, &func, &sig);
1090-
1099+
JL_GC_PUSH2(&env, &sig);
10911100
entry = jl_typemap_assoc_by_type(mt->defs, (jl_value_t*)tt, &env, /*subtype*/1, /*offs*/0, world, /*max_world_mask*/0);
10921101
if (entry != NULL) {
10931102
jl_method_t *m = entry->func.method;
@@ -1101,12 +1110,10 @@ static jl_method_instance_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_datatype
11011110
#endif
11021111
int sharp_match;
11031112
sig = join_tsig(tt, m->sig, &sharp_match);
1104-
if (!mt_cache) {
1113+
if (!mt_cache)
11051114
nf = jl_specializations_get_linfo(m, (jl_value_t*)sig, env, world);
1106-
}
1107-
else {
1115+
else
11081116
nf = cache_method(mt, &mt->cache, (jl_value_t*)mt, sig, tt, m, world, env, allow_exec);
1109-
}
11101117
}
11111118
}
11121119
JL_GC_POP();
@@ -1652,8 +1659,9 @@ jl_tupletype_t *arg_type_tuple(jl_value_t **args, size_t nargs)
16521659
return tt;
16531660
}
16541661

1655-
jl_method_instance_t *jl_method_lookup_by_type(jl_methtable_t *mt, jl_tupletype_t *types,
1656-
int cache, int allow_exec, size_t world)
1662+
static jl_method_instance_t *jl_method_lookup_by_type(
1663+
jl_methtable_t *mt, jl_tupletype_t *types,
1664+
int cache, int allow_exec, size_t world)
16571665
{
16581666
jl_typemap_entry_t *entry = jl_typemap_assoc_by_type(mt->cache, (jl_value_t*)types, NULL, /*subtype*/1, jl_cachearg_offset(mt), world, /*max_world_mask*/0);
16591667
if (entry) {
@@ -1663,25 +1671,10 @@ jl_method_instance_t *jl_method_lookup_by_type(jl_methtable_t *mt, jl_tupletype_
16631671
return linfo;
16641672
}
16651673
JL_LOCK(&mt->writelock);
1666-
entry = jl_typemap_assoc_by_type(mt->cache, (jl_value_t*)types, NULL, /*subtype*/1, jl_cachearg_offset(mt), world, /*max_world_mask*/0);
1667-
if (entry) {
1668-
jl_method_instance_t *linfo = (jl_method_instance_t*)entry->func.value;
1669-
assert(linfo->min_world <= entry->min_world && linfo->max_world >= entry->max_world &&
1670-
"typemap consistency error: MethodInstance doesn't apply to full range of its entry");
1671-
JL_UNLOCK(&mt->writelock);
1672-
return linfo;
1673-
}
16741674
if (jl_is_datatype((jl_value_t*)types) && types->isdispatchtuple)
1675-
cache = 1;
1675+
cache = 1; // TODO: this might be detrimental to performance, should be tested
16761676
jl_method_instance_t *sf = jl_mt_assoc_by_type(mt, types, cache, allow_exec, world);
1677-
if (cache) {
1678-
JL_UNLOCK(&mt->writelock);
1679-
}
1680-
else {
1681-
JL_GC_PUSH1(&sf);
1682-
JL_UNLOCK(&mt->writelock);
1683-
JL_GC_POP();
1684-
}
1677+
JL_UNLOCK(&mt->writelock);
16851678
return sf;
16861679
}
16871680

@@ -1696,24 +1689,11 @@ jl_method_instance_t *jl_method_lookup(jl_methtable_t *mt, jl_value_t **args, si
16961689
if (entry)
16971690
return entry->func.linfo;
16981691
JL_LOCK(&mt->writelock);
1699-
entry = jl_typemap_assoc_exact(mt->cache, args, nargs, jl_cachearg_offset(mt), world);
1700-
if (entry) {
1701-
JL_UNLOCK(&mt->writelock);
1702-
return entry->func.linfo;
1703-
}
17041692
jl_tupletype_t *tt = arg_type_tuple(args, nargs);
1705-
jl_method_instance_t *sf = NULL;
1706-
JL_GC_PUSH2(&tt, &sf);
1707-
sf = jl_mt_assoc_by_type(mt, tt, cache, 1, world);
1708-
if (cache) {
1709-
JL_UNLOCK(&mt->writelock);
1710-
}
1711-
else {
1712-
JL_GC_PUSH1(&sf);
1713-
JL_UNLOCK(&mt->writelock);
1714-
JL_GC_POP();
1715-
}
1693+
JL_GC_PUSH1(&tt);
1694+
jl_method_instance_t *sf = jl_mt_assoc_by_type(mt, tt, cache, 1, world);
17161695
JL_GC_POP();
1696+
JL_UNLOCK(&mt->writelock);
17171697
return sf;
17181698
}
17191699

@@ -1906,6 +1886,7 @@ jl_method_instance_t *jl_get_specialization1(jl_tupletype_t *types, size_t world
19061886
JL_LOCK(&mt->writelock);
19071887
nf = cache_method(mt, &mt->cache, (jl_value_t*)mt, sig, ti, m, world, env, /*allow_exec*/1);
19081888
JL_UNLOCK(&mt->writelock);
1889+
assert(nf->min_world <= world && nf->max_world >= world);
19091890
}
19101891
// // get the specialization without caching it
19111892
// int need_guard_entries = 0;
@@ -1917,7 +1898,6 @@ jl_method_instance_t *jl_get_specialization1(jl_tupletype_t *types, size_t world
19171898
// sig = jl_apply_tuple_type(newparams);
19181899
// nf = jl_specializations_get_linfo(m, (jl_value_t*)sig, env, world);
19191900
}
1920-
assert(nf == NULL || (nf->min_world <= world && nf->max_world >= world));
19211901
JL_GC_POP();
19221902
return nf;
19231903
}
@@ -2123,18 +2103,12 @@ STATIC_INLINE jl_method_instance_t *jl_lookup_generic_(jl_value_t **args, uint32
21232103
}
21242104
else {
21252105
JL_LOCK(&mt->writelock);
2126-
entry = jl_typemap_assoc_exact(mt->cache, args, nargs, jl_cachearg_offset(mt), world);
2127-
if (entry) {
2128-
mfunc = entry->func.linfo;
2129-
}
2130-
else {
2131-
// cache miss case
2132-
JL_TIMING(METHOD_LOOKUP_SLOW);
2133-
jl_tupletype_t *tt = arg_type_tuple(args, nargs);
2134-
JL_GC_PUSH1(&tt);
2135-
mfunc = jl_mt_assoc_by_type(mt, tt, /*cache*/1, /*allow_exec*/1, world);
2136-
JL_GC_POP();
2137-
}
2106+
// cache miss case
2107+
JL_TIMING(METHOD_LOOKUP_SLOW);
2108+
jl_tupletype_t *tt = arg_type_tuple(args, nargs);
2109+
JL_GC_PUSH1(&tt);
2110+
mfunc = jl_mt_assoc_by_type(mt, tt, /*cache*/1, /*allow_exec*/1, world);
2111+
JL_GC_POP();
21382112
JL_UNLOCK(&mt->writelock);
21392113
if (mfunc == NULL) {
21402114
#ifdef JL_TRACE
@@ -2223,25 +2197,18 @@ jl_value_t *jl_gf_invoke(jl_value_t *types0, jl_value_t **args, size_t nargs)
22232197
}
22242198
else {
22252199
JL_LOCK(&method->writelock);
2226-
if (method->invokes.unknown != NULL)
2227-
tm = jl_typemap_assoc_exact(method->invokes, args, nargs, jl_cachearg_offset(mt), world);
2228-
if (tm) {
2229-
mfunc = tm->func.linfo;
2200+
tt = arg_type_tuple(args, nargs);
2201+
if (jl_is_unionall(method->sig)) {
2202+
int sub = jl_subtype_matching((jl_value_t*)tt, (jl_value_t*)method->sig, &tpenv);
2203+
assert(sub); (void)sub;
22302204
}
2231-
else {
2232-
tt = arg_type_tuple(args, nargs);
2233-
if (jl_is_unionall(method->sig)) {
2234-
int sub = jl_subtype_matching((jl_value_t*)tt, (jl_value_t*)method->sig, &tpenv);
2235-
assert(sub); (void)sub;
2236-
}
22372205

2238-
if (method->invokes.unknown == NULL)
2239-
method->invokes.unknown = jl_nothing;
2206+
if (method->invokes.unknown == NULL)
2207+
method->invokes.unknown = jl_nothing;
22402208

2241-
int sharp_match;
2242-
sig = join_tsig(tt, method->sig, &sharp_match);
2243-
mfunc = cache_method(mt, &method->invokes, entry->func.value, sig, tt, method, world, tpenv, 1);
2244-
}
2209+
int sharp_match;
2210+
sig = join_tsig(tt, method->sig, &sharp_match);
2211+
mfunc = cache_method(mt, &method->invokes, entry->func.value, sig, tt, method, world, tpenv, 1);
22452212
JL_UNLOCK(&method->writelock);
22462213
}
22472214
JL_GC_POP();

0 commit comments

Comments
 (0)