Skip to content

Commit 02c7014

Browse files
JeffBezansonjohanmon
authored andcommitted
remove code for inserting methods in sorted order (JuliaLang#39797)
We've reached the point where our algorithms no longer need to assume sorting.
1 parent a4bf066 commit 02c7014

File tree

4 files changed

+38
-86
lines changed

4 files changed

+38
-86
lines changed

src/gf.c

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@ void jl_call_tracer(tracer_cb callback, jl_value_t *tracee)
6969

7070
/// ----- Definitions for various internal TypeMaps ----- ///
7171

72-
static struct jl_typemap_info method_defs = {1};
73-
static struct jl_typemap_info lambda_cache = {1};
74-
7572
static int8_t jl_cachearg_offset(jl_methtable_t *mt)
7673
{
7774
return mt->offs;
@@ -243,7 +240,7 @@ jl_datatype_t *jl_mk_builtin_func(jl_datatype_t *dt, const char *name, jl_fptr_a
243240
jl_methtable_t *mt = dt->name->mt;
244241
newentry = jl_typemap_alloc(jl_anytuple_type, NULL, jl_emptysvec,
245242
(jl_value_t*)mi, 1, ~(size_t)0);
246-
jl_typemap_insert(&mt->cache, (jl_value_t*)mt, newentry, 0, &lambda_cache);
243+
jl_typemap_insert(&mt->cache, (jl_value_t*)mt, newentry, 0);
247244

248245
mt->frozen = 1;
249246
JL_GC_POP();
@@ -1090,7 +1087,7 @@ static jl_method_instance_t *cache_method(
10901087
guards++;
10911088
// alternative approach: insert sentinel entry
10921089
//jl_typemap_insert(cache, parent, (jl_tupletype_t*)matc->spec_types,
1093-
// NULL, jl_emptysvec, /*guard*/NULL, jl_cachearg_offset(mt), &lambda_cache, other->min_world, other->max_world);
1090+
// NULL, jl_emptysvec, /*guard*/NULL, jl_cachearg_offset(mt), other->min_world, other->max_world);
10941091
}
10951092
}
10961093
}
@@ -1165,7 +1162,7 @@ static jl_method_instance_t *cache_method(
11651162
jl_gc_wb(mt, mt->leafcache);
11661163
}
11671164
else {
1168-
jl_typemap_insert(cache, parent, newentry, offs, &lambda_cache);
1165+
jl_typemap_insert(cache, parent, newentry, offs);
11691166
}
11701167

11711168
JL_GC_POP();
@@ -1641,7 +1638,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method
16411638
// then add our new entry
16421639
newentry = jl_typemap_alloc((jl_tupletype_t*)type, simpletype, jl_emptysvec,
16431640
(jl_value_t*)method, method->primary_world, method->deleted_world);
1644-
jl_typemap_insert(&mt->defs, (jl_value_t*)mt, newentry, 0, &method_defs);
1641+
jl_typemap_insert(&mt->defs, (jl_value_t*)mt, newentry, 0);
16451642
if (oldentry) {
16461643
jl_method_t *m = oldentry->func.method;
16471644
method_overwrite(newentry, m);

src/julia_internal.h

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,16 +1189,8 @@ void jl_smallintset_insert(jl_array_t **pcache, jl_value_t *parent, smallintset_
11891189

11901190
// -- typemap.c -- //
11911191

1192-
// a descriptor of a jl_typemap_t that gets
1193-
// passed around as self-documentation of the parameters of the type
1194-
struct jl_typemap_info {
1195-
int8_t unsorted; // whether this should be unsorted
1196-
jl_datatype_t **jl_contains; // the type that is being put in this
1197-
};
1198-
11991192
void jl_typemap_insert(jl_typemap_t **cache, jl_value_t *parent,
1200-
jl_typemap_entry_t *newrec, int8_t offs,
1201-
const struct jl_typemap_info *tparams);
1193+
jl_typemap_entry_t *newrec, int8_t offs);
12021194
jl_typemap_entry_t *jl_typemap_alloc(
12031195
jl_tupletype_t *type, jl_tupletype_t *simpletype, jl_svec_t *guardsigs,
12041196
jl_value_t *newvalue, size_t min_world, size_t max_world);

src/typemap.c

Lines changed: 32 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,10 +1087,7 @@ static unsigned jl_typemap_list_count_locked(jl_typemap_entry_t *ml) JL_NOTSAFEP
10871087
return count;
10881088
}
10891089

1090-
static void jl_typemap_level_insert_(jl_typemap_t *map, jl_typemap_level_t *cache, jl_typemap_entry_t *newrec, int8_t offs, const struct jl_typemap_info *tparams);
1091-
static void jl_typemap_list_insert_sorted(
1092-
jl_typemap_t *map, jl_typemap_entry_t **pml, jl_value_t *parent,
1093-
jl_typemap_entry_t *newrec, const struct jl_typemap_info *tparams);
1090+
static void jl_typemap_level_insert_(jl_typemap_t *map, jl_typemap_level_t *cache, jl_typemap_entry_t *newrec, int8_t offs);
10941091

10951092
static jl_typemap_level_t *jl_new_typemap_level(void)
10961093
{
@@ -1108,8 +1105,7 @@ static jl_typemap_level_t *jl_new_typemap_level(void)
11081105
}
11091106

11101107
static jl_typemap_level_t *jl_method_convert_list_to_cache(
1111-
jl_typemap_t *map, jl_typemap_entry_t *ml, int8_t offs,
1112-
const struct jl_typemap_info *tparams)
1108+
jl_typemap_t *map, jl_typemap_entry_t *ml, int8_t offs)
11131109
{
11141110
jl_typemap_level_t *cache = jl_new_typemap_level();
11151111
jl_typemap_entry_t *next = NULL;
@@ -1118,7 +1114,7 @@ static jl_typemap_level_t *jl_method_convert_list_to_cache(
11181114
next = ml->next;
11191115
ml->next = (jl_typemap_entry_t*)jl_nothing;
11201116
// TODO: is it safe to be doing this concurrently with lookups?
1121-
jl_typemap_level_insert_(map, cache, ml, offs, tparams);
1117+
jl_typemap_level_insert_(map, cache, ml, offs);
11221118
ml = next;
11231119
}
11241120
JL_GC_POP();
@@ -1127,58 +1123,59 @@ static jl_typemap_level_t *jl_method_convert_list_to_cache(
11271123

11281124
static void jl_typemap_list_insert_(
11291125
jl_typemap_t *map, jl_typemap_entry_t **pml, jl_value_t *parent,
1130-
jl_typemap_entry_t *newrec, const struct jl_typemap_info *tparams)
1126+
jl_typemap_entry_t *newrec)
11311127
{
1132-
if (*pml == (void*)jl_nothing || newrec->isleafsig || (tparams && tparams->unsorted)) {
1133-
newrec->next = *pml;
1134-
jl_gc_wb(newrec, newrec->next);
1135-
jl_atomic_store_release(pml, newrec);
1136-
jl_gc_wb(parent, newrec);
1137-
}
1138-
else {
1139-
jl_typemap_list_insert_sorted(map, pml, parent, newrec, tparams);
1128+
jl_typemap_entry_t *l = *pml;
1129+
while ((jl_value_t*)l != jl_nothing) {
1130+
if (newrec->isleafsig || !l->isleafsig)
1131+
if (newrec->issimplesig || !l->issimplesig)
1132+
break;
1133+
pml = &l->next;
1134+
parent = (jl_value_t*)l;
1135+
l = l->next;
11401136
}
1137+
newrec->next = l;
1138+
jl_gc_wb(newrec, newrec->next);
1139+
jl_atomic_store_release(pml, newrec);
1140+
jl_gc_wb(parent, newrec);
11411141
}
11421142

11431143
static void jl_typemap_insert_generic(
11441144
jl_typemap_t *map, jl_typemap_t **pml, jl_value_t *parent,
1145-
jl_typemap_entry_t *newrec, int8_t offs,
1146-
const struct jl_typemap_info *tparams)
1145+
jl_typemap_entry_t *newrec, int8_t offs)
11471146
{
11481147
if (jl_typeof(*pml) == (jl_value_t*)jl_typemap_level_type) {
1149-
jl_typemap_level_insert_(map, (jl_typemap_level_t*)*pml, newrec, offs, tparams);
1148+
jl_typemap_level_insert_(map, (jl_typemap_level_t*)*pml, newrec, offs);
11501149
return;
11511150
}
11521151

11531152
unsigned count = jl_typemap_list_count_locked((jl_typemap_entry_t*)*pml);
11541153
if (count > MAX_METHLIST_COUNT) {
11551154
*pml = (jl_typemap_t*)jl_method_convert_list_to_cache(
11561155
map, (jl_typemap_entry_t *)*pml,
1157-
offs, tparams);
1156+
offs);
11581157
jl_gc_wb(parent, *pml);
1159-
jl_typemap_level_insert_(map, (jl_typemap_level_t*)*pml, newrec, offs, tparams);
1158+
jl_typemap_level_insert_(map, (jl_typemap_level_t*)*pml, newrec, offs);
11601159
return;
11611160
}
11621161

11631162
jl_typemap_list_insert_(map, (jl_typemap_entry_t **)pml,
1164-
parent, newrec, tparams);
1163+
parent, newrec);
11651164
}
11661165

11671166
static void jl_typemap_array_insert_(
11681167
jl_typemap_t *map, jl_array_t **cache, jl_value_t *key, jl_typemap_entry_t *newrec,
1169-
jl_value_t *parent, int8_t offs,
1170-
const struct jl_typemap_info *tparams)
1168+
jl_value_t *parent, int8_t offs)
11711169
{
11721170
jl_typemap_t **pml = mtcache_hash_lookup_bp(*cache, key);
11731171
if (pml != NULL)
1174-
jl_typemap_insert_generic(map, pml, (jl_value_t*)*cache, newrec, offs+1, tparams);
1172+
jl_typemap_insert_generic(map, pml, (jl_value_t*)*cache, newrec, offs+1);
11751173
else
11761174
mtcache_hash_insert(cache, parent, key, (jl_typemap_t*)newrec);
11771175
}
11781176

11791177
static void jl_typemap_level_insert_(
1180-
jl_typemap_t *map, jl_typemap_level_t *cache, jl_typemap_entry_t *newrec, int8_t offs,
1181-
const struct jl_typemap_info *tparams)
1178+
jl_typemap_t *map, jl_typemap_level_t *cache, jl_typemap_entry_t *newrec, int8_t offs)
11821179
{
11831180
jl_value_t *ttypes = jl_unwrap_unionall((jl_value_t*)newrec->sig);
11841181
size_t l = jl_nparams(ttypes);
@@ -1205,7 +1202,7 @@ static void jl_typemap_level_insert_(
12051202
t1 = (jl_value_t*)jl_assume(jl_typeofbottom_type)->super;
12061203
// If the type at `offs` is Any, put it in the Any list
12071204
if (t1 && jl_is_any(t1)) {
1208-
jl_typemap_insert_generic(map, &cache->any, (jl_value_t*)cache, newrec, offs+1, tparams);
1205+
jl_typemap_insert_generic(map, &cache->any, (jl_value_t*)cache, newrec, offs+1);
12091206
return;
12101207
}
12111208
// Don't put Varargs in the optimized caches (too hard to handle in lookup and bp)
@@ -1216,12 +1213,12 @@ static void jl_typemap_level_insert_(
12161213
// and we use the table indexed for that purpose.
12171214
jl_value_t *a0 = jl_tparam0(t1);
12181215
if (is_cache_leaf(a0, 1)) {
1219-
jl_typemap_array_insert_(map, &cache->targ, a0, newrec, (jl_value_t*)cache, offs, tparams);
1216+
jl_typemap_array_insert_(map, &cache->targ, a0, newrec, (jl_value_t*)cache, offs);
12201217
return;
12211218
}
12221219
}
12231220
if (is_cache_leaf(t1, 0)) {
1224-
jl_typemap_array_insert_(map, &cache->arg1, t1, newrec, (jl_value_t*)cache, offs, tparams);
1221+
jl_typemap_array_insert_(map, &cache->arg1, t1, newrec, (jl_value_t*)cache, offs);
12251222
return;
12261223
}
12271224

@@ -1231,16 +1228,16 @@ static void jl_typemap_level_insert_(
12311228
if (jl_is_type_type(t1)) {
12321229
a0 = jl_type_extract_name(jl_tparam0(t1));
12331230
jl_datatype_t *super = a0 ? (jl_datatype_t*)jl_unwrap_unionall(((jl_typename_t*)a0)->wrapper) : jl_any_type;
1234-
jl_typemap_array_insert_(map, &cache->tname, (jl_value_t*)super->name, newrec, (jl_value_t*)cache, offs, tparams);
1231+
jl_typemap_array_insert_(map, &cache->tname, (jl_value_t*)super->name, newrec, (jl_value_t*)cache, offs);
12351232
return;
12361233
}
12371234
a0 = jl_type_extract_name(t1);
12381235
if (a0 && a0 != (jl_value_t*)jl_any_type->name) {
1239-
jl_typemap_array_insert_(map, &cache->name1, a0, newrec, (jl_value_t*)cache, offs, tparams);
1236+
jl_typemap_array_insert_(map, &cache->name1, a0, newrec, (jl_value_t*)cache, offs);
12401237
return;
12411238
}
12421239
}
1243-
jl_typemap_list_insert_(map, &cache->linear, (jl_value_t*)cache, newrec, tparams);
1240+
jl_typemap_list_insert_(map, &cache->linear, (jl_value_t*)cache, newrec);
12441241
}
12451242

12461243
jl_typemap_entry_t *jl_typemap_alloc(
@@ -1289,43 +1286,9 @@ jl_typemap_entry_t *jl_typemap_alloc(
12891286
}
12901287

12911288
void jl_typemap_insert(jl_typemap_t **cache, jl_value_t *parent,
1292-
jl_typemap_entry_t *newrec, int8_t offs,
1293-
const struct jl_typemap_info *tparams)
1294-
{
1295-
jl_typemap_insert_generic(*cache, cache, parent, newrec, offs, tparams);
1296-
}
1297-
1298-
static void jl_typemap_list_insert_sorted(
1299-
jl_typemap_t *map, jl_typemap_entry_t **pml, jl_value_t *parent,
1300-
jl_typemap_entry_t *newrec, const struct jl_typemap_info *tparams)
1289+
jl_typemap_entry_t *newrec, int8_t offs)
13011290
{
1302-
jl_typemap_entry_t *l, **pl;
1303-
pl = pml;
1304-
l = *pml;
1305-
jl_value_t *pa = parent;
1306-
while ((jl_value_t*)l != jl_nothing) {
1307-
if (!l->isleafsig) { // quickly ignore all of the leafsig entries (these were handled by caller)
1308-
if (jl_type_morespecific((jl_value_t*)newrec->sig, (jl_value_t*)l->sig)) {
1309-
if (l->simplesig == (void*)jl_nothing ||
1310-
newrec->simplesig != (void*)jl_nothing ||
1311-
!jl_types_equal((jl_value_t*)l->sig, (jl_value_t*)newrec->sig)) {
1312-
// might need to insert multiple entries for a lookup differing only by their simplesig
1313-
// when simplesig contains a kind
1314-
// TODO: make this test more correct or figure out a better way to compute this
1315-
break;
1316-
}
1317-
}
1318-
}
1319-
pl = &l->next;
1320-
pa = (jl_value_t*)l;
1321-
l = l->next;
1322-
}
1323-
1324-
// insert newrec at the first point it is more specific than the following method
1325-
newrec->next = l;
1326-
jl_gc_wb(newrec, l);
1327-
jl_atomic_store_release(pl, newrec);
1328-
jl_gc_wb(pa, newrec);
1291+
jl_typemap_insert_generic(*cache, cache, parent, newrec, offs);
13291292
}
13301293

13311294
#ifdef __cplusplus

test/errorshow.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ method_c2(x::Int32, y::Int32, z::Int32) = true
8686
method_c2(x::T, y::T, z::T) where {T<:Real} = true
8787

8888
Base.show_method_candidates(buf, Base.MethodError(method_c2,(1., 1., 2)))
89-
@test String(take!(buf)) == "\nClosest candidates are:\n method_c2(!Matched::Int32, ::Float64, ::Any...)$cfile$(c2line+2)\n method_c2(!Matched::Int32, ::Any...)$cfile$(c2line+1)\n method_c2(::T, ::T, !Matched::T) where T<:Real$cfile$(c2line+5)\n ..."
89+
@test String(take!(buf)) == "\nClosest candidates are:\n method_c2(!Matched::Int32, ::Float64, ::Any...)$cfile$(c2line+2)\n method_c2(::T, ::T, !Matched::T) where T<:Real$cfile$(c2line+5)\n method_c2(!Matched::Int32, ::Any...)$cfile$(c2line+1)\n ..."
9090

9191
c3line = @__LINE__() + 1
9292
method_c3(x::Float64, y::Float64) = true

0 commit comments

Comments
 (0)