Skip to content

Commit 240b0ea

Browse files
vtjnashvtjnash
committed
function cache: fix memory leak
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 e4fa75d commit 240b0ea

File tree

1 file changed

+38
-72
lines changed

1 file changed

+38
-72
lines changed

‎src/gf.c

+38-72
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,14 @@ 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+
return (jl_method_instance_t*)entry->func.value;
1092+
1093+
jl_method_instance_t *nf = NULL;
10851094
jl_svec_t *env = jl_emptysvec;
1086-
jl_method_t *func = NULL;
10871095
jl_tupletype_t *sig = NULL;
1088-
jl_method_instance_t *nf = NULL;
1089-
JL_GC_PUSH4(&env, &entry, &func, &sig);
1090-
1096+
JL_GC_PUSH2(&env, &sig);
10911097
entry = jl_typemap_assoc_by_type(mt->defs, (jl_value_t*)tt, &env, /*subtype*/1, /*offs*/0, world, /*max_world_mask*/0);
10921098
if (entry != NULL) {
10931099
jl_method_t *m = entry->func.method;
@@ -1101,12 +1107,10 @@ static jl_method_instance_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_datatype
11011107
#endif
11021108
int sharp_match;
11031109
sig = join_tsig(tt, m->sig, &sharp_match);
1104-
if (!mt_cache) {
1110+
if (!mt_cache)
11051111
nf = jl_specializations_get_linfo(m, (jl_value_t*)sig, env, world);
1106-
}
1107-
else {
1112+
else
11081113
nf = cache_method(mt, &mt->cache, (jl_value_t*)mt, sig, tt, m, world, env, allow_exec);
1109-
}
11101114
}
11111115
}
11121116
JL_GC_POP();
@@ -1652,8 +1656,9 @@ jl_tupletype_t *arg_type_tuple(jl_value_t **args, size_t nargs)
16521656
return tt;
16531657
}
16541658

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)
1659+
static jl_method_instance_t *jl_method_lookup_by_type(
1660+
jl_methtable_t *mt, jl_tupletype_t *types,
1661+
int cache, int allow_exec, size_t world)
16571662
{
16581663
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);
16591664
if (entry) {
@@ -1663,25 +1668,12 @@ jl_method_instance_t *jl_method_lookup_by_type(jl_methtable_t *mt, jl_tupletype_
16631668
return linfo;
16641669
}
16651670
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-
}
16741671
if (jl_is_datatype((jl_value_t*)types) && types->isdispatchtuple)
1675-
cache = 1;
1672+
cache = 1; // TODO: this might be detrimental to performance, should be tested
16761673
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-
}
1674+
assert(sf->min_world <= entry->min_world && sf->max_world >= entry->max_world &&
1675+
"typemap consistency error: MethodInstance doesn't apply to full range of its entry");
1676+
JL_UNLOCK(&mt->writelock);
16851677
return sf;
16861678
}
16871679

@@ -1696,24 +1688,11 @@ jl_method_instance_t *jl_method_lookup(jl_methtable_t *mt, jl_value_t **args, si
16961688
if (entry)
16971689
return entry->func.linfo;
16981690
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-
}
17041691
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-
}
1692+
JL_GC_PUSH1(&tt);
1693+
jl_method_instance_t *sf = jl_mt_assoc_by_type(mt, tt, cache, 1, world);
17161694
JL_GC_POP();
1695+
JL_UNLOCK(&mt->writelock);
17171696
return sf;
17181697
}
17191698

@@ -1906,6 +1885,7 @@ jl_method_instance_t *jl_get_specialization1(jl_tupletype_t *types, size_t world
19061885
JL_LOCK(&mt->writelock);
19071886
nf = cache_method(mt, &mt->cache, (jl_value_t*)mt, sig, ti, m, world, env, /*allow_exec*/1);
19081887
JL_UNLOCK(&mt->writelock);
1888+
assert(nf->min_world <= world && nf->max_world >= world);
19091889
}
19101890
// // get the specialization without caching it
19111891
// int need_guard_entries = 0;
@@ -1917,7 +1897,6 @@ jl_method_instance_t *jl_get_specialization1(jl_tupletype_t *types, size_t world
19171897
// sig = jl_apply_tuple_type(newparams);
19181898
// nf = jl_specializations_get_linfo(m, (jl_value_t*)sig, env, world);
19191899
}
1920-
assert(nf == NULL || (nf->min_world <= world && nf->max_world >= world));
19211900
JL_GC_POP();
19221901
return nf;
19231902
}
@@ -2123,18 +2102,12 @@ STATIC_INLINE jl_method_instance_t *jl_lookup_generic_(jl_value_t **args, uint32
21232102
}
21242103
else {
21252104
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-
}
2105+
// cache miss case
2106+
JL_TIMING(METHOD_LOOKUP_SLOW);
2107+
jl_tupletype_t *tt = arg_type_tuple(args, nargs);
2108+
JL_GC_PUSH1(&tt);
2109+
mfunc = jl_mt_assoc_by_type(mt, tt, /*cache*/1, /*allow_exec*/1, world);
2110+
JL_GC_POP();
21382111
JL_UNLOCK(&mt->writelock);
21392112
if (mfunc == NULL) {
21402113
#ifdef JL_TRACE
@@ -2223,25 +2196,18 @@ jl_value_t *jl_gf_invoke(jl_value_t *types0, jl_value_t **args, size_t nargs)
22232196
}
22242197
else {
22252198
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;
2199+
tt = arg_type_tuple(args, nargs);
2200+
if (jl_is_unionall(method->sig)) {
2201+
int sub = jl_subtype_matching((jl_value_t*)tt, (jl_value_t*)method->sig, &tpenv);
2202+
assert(sub); (void)sub;
22302203
}
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-
}
22372204

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

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-
}
2208+
int sharp_match;
2209+
sig = join_tsig(tt, method->sig, &sharp_match);
2210+
mfunc = cache_method(mt, &method->invokes, entry->func.value, sig, tt, method, world, tpenv, 1);
22452211
JL_UNLOCK(&method->writelock);
22462212
}
22472213
JL_GC_POP();

0 commit comments

Comments
 (0)