Skip to content

Commit d762e8c

Browse files
authored
gf: improve ordering of operations based on performance estimates (#36436)
In the last commit, I didn't expect NamedTuple to hammer our performance, but was starting to notice performance problems with trying to call constructors. It's somewhat violating our best-practices in two common cases (both the constructor and structdiff functions are dispatching on non-leaf types instead of values). That was putting a lot of strain on the cache, and so it forms a good test case. Keeping those cases out of the cache, and searching the cache in a more suitable order (significant mainly for constructors because there are always so many of them), offsets that--and seems to possibly make us slightly faster overall as a bonus because of that effect!
1 parent ab520c7 commit d762e8c

File tree

3 files changed

+33
-8
lines changed

3 files changed

+33
-8
lines changed

src/gf.c

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,8 +1139,11 @@ static jl_method_instance_t *cache_method(
11391139

11401140
jl_typemap_entry_t *newentry = jl_typemap_alloc(cachett, simplett, guardsigs, (jl_value_t*)newmeth, min_valid, max_valid);
11411141
temp = (jl_value_t*)newentry;
1142-
if (mt && cachett == tt && jl_svec_len(guardsigs) == 0) {
1143-
if (!jl_has_free_typevars((jl_value_t*)tt) && jl_lookup_cache_type_(tt) == NULL) {
1142+
if (mt && cachett == tt && jl_svec_len(guardsigs) == 0 && tt->hash && !tt->hasfreetypevars) {
1143+
// we check `tt->hash` exists, since otherwise the NamedTuple
1144+
// constructor and `structdiff` method pollutes this lookup with a lot
1145+
// of garbage in the linear table search
1146+
if (jl_lookup_cache_type_(tt) == NULL) {
11441147
// if this type isn't normally in the cache, force it in there now
11451148
// anyways so that we can depend on it as a token (especially since
11461149
// we just cached it in memory as this method signature anyways)
@@ -1948,6 +1951,11 @@ jl_tupletype_t *arg_type_tuple(jl_value_t *arg1, jl_value_t **args, size_t nargs
19481951
return jl_inst_arg_tuple_type(arg1, args, nargs, 1);
19491952
}
19501953

1954+
jl_tupletype_t *lookup_arg_type_tuple(jl_value_t *arg1 JL_PROPAGATES_ROOT, jl_value_t **args, size_t nargs)
1955+
{
1956+
return jl_lookup_arg_tuple_type(arg1, args, nargs, 1);
1957+
}
1958+
19511959
jl_method_instance_t *jl_method_lookup(jl_value_t **args, size_t nargs, size_t world)
19521960
{
19531961
assert(nargs > 0 && "expected caller to handle this case");
@@ -2449,14 +2457,25 @@ STATIC_INLINE jl_method_instance_t *jl_lookup_generic_(jl_value_t *F, jl_value_t
24492457
// if no method was found in the associative cache, check the full cache
24502458
JL_TIMING(METHOD_LOOKUP_FAST);
24512459
mt = jl_gf_mtable(F);
2452-
entry = jl_typemap_assoc_exact(mt->cache, F, args, nargs, jl_cachearg_offset(mt), world);
2460+
jl_array_t *leafcache = jl_atomic_load_relaxed(&mt->leafcache);
2461+
entry = NULL;
2462+
if (leafcache != (jl_array_t*)jl_an_empty_vec_any && jl_typeis(mt->cache, jl_typemap_level_type)) {
2463+
// hashing args is expensive, but looking at mt->cache is probably even more expensive
2464+
tt = lookup_arg_type_tuple(F, args, nargs);
2465+
if (tt != NULL)
2466+
entry = lookup_leafcache(leafcache, (jl_value_t*)tt, world);
2467+
}
24532468
if (entry == NULL) {
2454-
last_alloc = jl_options.malloc_log ? jl_gc_diff_total_bytes() : 0;
2455-
tt = arg_type_tuple(F, args, nargs);
2456-
jl_array_t *leafcache = jl_atomic_load_relaxed(&mt->leafcache);
2457-
entry = lookup_leafcache(leafcache, (jl_value_t*)tt, world);
2469+
entry = jl_typemap_assoc_exact(mt->cache, F, args, nargs, jl_cachearg_offset(mt), world);
2470+
if (entry == NULL) {
2471+
last_alloc = jl_options.malloc_log ? jl_gc_diff_total_bytes() : 0;
2472+
if (tt == NULL) {
2473+
tt = arg_type_tuple(F, args, nargs);
2474+
entry = lookup_leafcache(leafcache, (jl_value_t*)tt, world);
2475+
}
2476+
}
24582477
}
2459-
if (entry && entry->isleafsig && entry->simplesig == (void*)jl_nothing && entry->guardsigs == jl_emptysvec) {
2478+
if (entry != NULL && entry->isleafsig && entry->simplesig == (void*)jl_nothing && entry->guardsigs == jl_emptysvec) {
24602479
// put the entry into the cache if it's valid for a leafsig lookup,
24612480
// using pick_which to slightly randomize where it ends up
24622481
call_cache[cache_idx[++pick_which[cache_idx[0]] & 3]] = entry;

src/jltypes.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,6 +1513,11 @@ jl_datatype_t *jl_inst_concrete_tupletype_v(jl_value_t **p, size_t np)
15131513
return (jl_datatype_t*)inst_datatype_inner(jl_anytuple_type, NULL, p, np, 1, NULL, NULL);
15141514
}
15151515

1516+
jl_tupletype_t *jl_lookup_arg_tuple_type(jl_value_t *arg1, jl_value_t **args, size_t nargs, int leaf)
1517+
{
1518+
return (jl_datatype_t*)lookup_typevalue(jl_tuple_typename, arg1, args, nargs, leaf);
1519+
}
1520+
15161521
jl_tupletype_t *jl_inst_arg_tuple_type(jl_value_t *arg1, jl_value_t **args, size_t nargs, int leaf)
15171522
{
15181523
jl_tupletype_t *tt = (jl_datatype_t*)lookup_typevalue(jl_tuple_typename, arg1, args, nargs, leaf);

src/julia_internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,7 @@ int jl_has_concrete_subtype(jl_value_t *typ);
444444
jl_datatype_t *jl_inst_concrete_tupletype_v(jl_value_t **p, size_t np) JL_ALWAYS_LEAFTYPE;
445445
jl_datatype_t *jl_inst_concrete_tupletype(jl_svec_t *p) JL_ALWAYS_LEAFTYPE;
446446
jl_tupletype_t *jl_inst_arg_tuple_type(jl_value_t *arg1, jl_value_t **args, size_t nargs, int leaf);
447+
jl_tupletype_t *jl_lookup_arg_tuple_type(jl_value_t *arg1 JL_PROPAGATES_ROOT, jl_value_t **args, size_t nargs, int leaf);
447448
JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method, jl_tupletype_t *simpletype);
448449
jl_datatype_t *jl_mk_builtin_func(jl_datatype_t *dt, const char *name, jl_fptr_args_t fptr) JL_GC_DISABLED;
449450
jl_value_t *jl_type_intersection_env_s(jl_value_t *a, jl_value_t *b, jl_svec_t **penv, int *issubty);

0 commit comments

Comments
 (0)