Skip to content

Commit 5eb5155

Browse files
authored
replace incorrect Method.deleted_world with more useful Method.dispatch_status enum (#58291)
The original purpose of this field was to manage quickly detecting if a method was replaced, but that stopped being correct after #53415. It was a fairly heavy-weight description of that single bit of information. This bit of information allows quickly bypassing some method lookups from pkgimages, since it can quickly detect that the result is trivially correct (such as single-argument functions). Also fixes #58215
1 parent 6adb4cf commit 5eb5155

File tree

15 files changed

+113
-67
lines changed

15 files changed

+113
-67
lines changed

Compiler/src/typeinfer.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1540,7 +1540,7 @@ function compile!(codeinfos::Vector{Any}, workqueue::CompilationQueue;
15401540
# if this method is generally visible to the current compilation world,
15411541
# and this is either the primary world, or not applicable in the primary world
15421542
# then we want to compile and emit this
1543-
if item.def.primary_world <= world <= item.def.deleted_world
1543+
if item.def.primary_world <= world
15441544
ci = typeinf_ext(interp, item, SOURCE_MODE_GET_SOURCE)
15451545
ci isa CodeInstance && push!(workqueue, ci)
15461546
end

base/errorshow.jl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -592,8 +592,6 @@ function show_method_candidates(io::IO, ex::MethodError, kwargs=[])
592592
end
593593
if ex.world < reinterpret(UInt, method.primary_world)
594594
print(iob, " (method too new to be called from this world context.)")
595-
elseif ex.world > reinterpret(UInt, method.deleted_world)
596-
print(iob, " (method deleted before this world age.)")
597595
end
598596
println(iob)
599597

base/staticdata.jl

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,30 @@ end
288288

289289
function verify_call(@nospecialize(sig), expecteds::Core.SimpleVector, i::Int, n::Int, world::UInt)
290290
# verify that these edges intersect with the same methods as before
291+
if n == 1
292+
# first, fast-path a check if the expected method simply dominates its sig anyways
293+
# so the result of ml_matches is already simply known
294+
let t = expecteds[i], meth, minworld, maxworld, result
295+
if t isa Method
296+
meth = t
297+
else
298+
if t isa CodeInstance
299+
t = get_ci_mi(t)
300+
else
301+
t = t::MethodInstance
302+
end
303+
meth = t.def::Method
304+
end
305+
if !iszero(meth.dispatch_status & METHOD_SIG_LATEST_ONLY)
306+
minworld = meth.primary_world
307+
@assert minworld world
308+
maxworld = typemax(UInt)
309+
result = Any[] # result is unused
310+
return minworld, maxworld, result
311+
end
312+
end
313+
end
314+
# next, compare the current result of ml_matches to the old result
291315
lim = _jl_debug_method_invalidation[] !== nothing ? Int(typemax(Int32)) : n
292316
minworld = Ref{UInt}(1)
293317
maxworld = Ref{UInt}(typemax(UInt))
@@ -340,24 +364,25 @@ function verify_call(@nospecialize(sig), expecteds::Core.SimpleVector, i::Int, n
340364
return minworld[], maxworld[], result
341365
end
342366

367+
# fast-path dispatch_status bit definitions (false indicates unknown)
368+
# true indicates this method would be returned as the result from `which` when invoking `method.sig` in the current latest world
369+
const METHOD_SIG_LATEST_WHICH = 0x1
370+
# true indicates this method would be returned as the only result from `methods` when calling `method.sig` in the current latest world
371+
const METHOD_SIG_LATEST_ONLY = 0x2
372+
343373
function verify_invokesig(@nospecialize(invokesig), expected::Method, world::UInt)
344374
@assert invokesig isa Type
345375
local minworld::UInt, maxworld::UInt
346376
matched = nothing
347-
if invokesig === expected.sig
348-
# the invoke match is `expected` for `expected->sig`, unless `expected` is invalid
349-
# TODO: this is broken since PR #53415
377+
if invokesig === expected.sig && !iszero(expected.dispatch_status & METHOD_SIG_LATEST_WHICH)
378+
# the invoke match is `expected` for `expected->sig`, unless `expected` is replaced
350379
minworld = expected.primary_world
351-
maxworld = expected.deleted_world
352380
@assert minworld world
353-
if maxworld < world
354-
maxworld = 0
355-
end
356-
else
357-
minworld = 1
358381
maxworld = typemax(UInt)
382+
else
359383
mt = get_methodtable(expected)
360384
if mt === nothing
385+
minworld = 1
361386
maxworld = 0
362387
else
363388
matched, valid_worlds = Compiler._findsup(invokesig, mt, world)

src/gf.c

Lines changed: 52 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ void jl_mk_builtin_func(jl_datatype_t *dt, jl_sym_t *sname, jl_fptr_args_t fptr)
296296
m->isva = 1;
297297
m->nargs = 2;
298298
jl_atomic_store_relaxed(&m->primary_world, 1);
299-
jl_atomic_store_relaxed(&m->deleted_world, ~(size_t)0);
299+
jl_atomic_store_relaxed(&m->dispatch_status, METHOD_SIG_LATEST_ONLY | METHOD_SIG_LATEST_ONLY);
300300
m->sig = (jl_value_t*)jl_anytuple_type;
301301
m->slot_syms = jl_an_empty_string;
302302
m->nospecialize = 0;
@@ -307,7 +307,7 @@ void jl_mk_builtin_func(jl_datatype_t *dt, jl_sym_t *sname, jl_fptr_args_t fptr)
307307
JL_GC_PUSH2(&m, &newentry);
308308

309309
newentry = jl_typemap_alloc(jl_anytuple_type, NULL, jl_emptysvec,
310-
(jl_value_t*)m, jl_atomic_load_relaxed(&m->primary_world), jl_atomic_load_relaxed(&m->deleted_world));
310+
(jl_value_t*)m, 1, ~(size_t)0);
311311
jl_typemap_insert(&mt->defs, (jl_value_t*)mt, newentry, jl_cachearg_offset(mt));
312312

313313
jl_method_instance_t *mi = jl_get_specialized(m, (jl_value_t*)jl_anytuple_type, jl_emptysvec);
@@ -1908,7 +1908,7 @@ static int is_replacing(char ambig, jl_value_t *type, jl_method_t *m, jl_method_
19081908
// since m2 was also a previous match over isect,
19091909
// see if m was previously dominant over all m2
19101910
// or if this was already ambiguous before
1911-
if (ambig != morespec_is && !jl_type_morespecific(m->sig, m2->sig)) {
1911+
if (ambig == morespec_is && !jl_type_morespecific(m->sig, m2->sig)) {
19121912
// m and m2 were previously ambiguous over the full intersection of mi with type, and will still be ambiguous with addition of type
19131913
return 0;
19141914
}
@@ -2242,17 +2242,22 @@ JL_DLLEXPORT void jl_method_table_disable(jl_methtable_t *mt, jl_method_t *metho
22422242
JL_LOCK(&world_counter_lock);
22432243
if (!jl_atomic_load_relaxed(&allow_new_worlds))
22442244
jl_error("Method changes have been disabled via a call to disable_new_worlds.");
2245-
JL_LOCK(&mt->writelock);
2246-
// Narrow the world age on the method to make it uncallable
2247-
size_t world = jl_atomic_load_relaxed(&jl_world_counter);
2248-
assert(method == methodentry->func.method);
2249-
assert(jl_atomic_load_relaxed(&method->deleted_world) == ~(size_t)0);
2250-
jl_atomic_store_relaxed(&method->deleted_world, world);
2251-
jl_atomic_store_relaxed(&methodentry->max_world, world);
2252-
jl_method_table_invalidate(mt, method, world);
2253-
jl_atomic_store_release(&jl_world_counter, world + 1);
2254-
JL_UNLOCK(&mt->writelock);
2245+
int enabled = jl_atomic_load_relaxed(&methodentry->max_world) == ~(size_t)0;
2246+
if (enabled) {
2247+
JL_LOCK(&mt->writelock);
2248+
// Narrow the world age on the method to make it uncallable
2249+
size_t world = jl_atomic_load_relaxed(&jl_world_counter);
2250+
assert(method == methodentry->func.method);
2251+
jl_atomic_store_relaxed(&method->dispatch_status, 0);
2252+
assert(jl_atomic_load_relaxed(&methodentry->max_world) == ~(size_t)0);
2253+
jl_atomic_store_relaxed(&methodentry->max_world, world);
2254+
jl_method_table_invalidate(mt, method, world);
2255+
jl_atomic_store_release(&jl_world_counter, world + 1);
2256+
JL_UNLOCK(&mt->writelock);
2257+
}
22552258
JL_UNLOCK(&world_counter_lock);
2259+
if (!enabled)
2260+
jl_errorf("Method of %s already disabled", jl_symbol_name(method->name));
22562261
}
22572262

22582263
static int jl_type_intersection2(jl_value_t *t1, jl_value_t *t2, jl_value_t **isect JL_REQUIRE_ROOTED_SLOT, jl_value_t **isect2 JL_REQUIRE_ROOTED_SLOT)
@@ -2292,9 +2297,9 @@ jl_typemap_entry_t *jl_method_table_add(jl_methtable_t *mt, jl_method_t *method,
22922297
JL_LOCK(&mt->writelock);
22932298
// add our new entry
22942299
assert(jl_atomic_load_relaxed(&method->primary_world) == ~(size_t)0); // min-world
2295-
assert(jl_atomic_load_relaxed(&method->deleted_world) == 1); // max-world
2296-
newentry = jl_typemap_alloc((jl_tupletype_t*)method->sig, simpletype, jl_emptysvec, (jl_value_t*)method,
2297-
jl_atomic_load_relaxed(&method->primary_world), jl_atomic_load_relaxed(&method->deleted_world));
2300+
assert((jl_atomic_load_relaxed(&method->dispatch_status) & METHOD_SIG_LATEST_WHICH) == 0);
2301+
assert((jl_atomic_load_relaxed(&method->dispatch_status) & METHOD_SIG_LATEST_ONLY) == 0);
2302+
newentry = jl_typemap_alloc((jl_tupletype_t*)method->sig, simpletype, jl_emptysvec, (jl_value_t*)method, ~(size_t)0, 1);
22982303
jl_typemap_insert(&mt->defs, (jl_value_t*)mt, newentry, jl_cachearg_offset(mt));
22992304
update_max_args(mt, method->sig);
23002305
JL_UNLOCK(&mt->writelock);
@@ -2315,7 +2320,8 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
23152320
JL_LOCK(&mt->writelock);
23162321
size_t world = jl_atomic_load_relaxed(&method->primary_world);
23172322
assert(world == jl_atomic_load_relaxed(&jl_world_counter) + 1); // min-world
2318-
assert(jl_atomic_load_relaxed(&method->deleted_world) == ~(size_t)0); // max-world
2323+
assert((jl_atomic_load_relaxed(&method->dispatch_status) & METHOD_SIG_LATEST_WHICH) == 0);
2324+
assert((jl_atomic_load_relaxed(&method->dispatch_status) & METHOD_SIG_LATEST_ONLY) == 0);
23192325
assert(jl_atomic_load_relaxed(&newentry->min_world) == ~(size_t)0);
23202326
assert(jl_atomic_load_relaxed(&newentry->max_world) == 1);
23212327
jl_atomic_store_relaxed(&newentry->min_world, world);
@@ -2330,12 +2336,17 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
23302336
// then check what entries we replaced
23312337
oldvalue = get_intersect_matches(jl_atomic_load_relaxed(&mt->defs), newentry, &replaced, jl_cachearg_offset(mt), max_world);
23322338
int invalidated = 0;
2339+
int only = !(jl_atomic_load_relaxed(&method->dispatch_status) & METHOD_SIG_PRECOMPILE_MANY); // will compute if this will be currently the only result that would returned from `ml_matches` given `sig`
23332340
if (replaced) {
23342341
oldvalue = (jl_value_t*)replaced;
2342+
jl_method_t *m = replaced->func.method;
23352343
invalidated = 1;
2336-
method_overwrite(newentry, replaced->func.method);
2344+
method_overwrite(newentry, m);
23372345
// this is an optimized version of below, given we know the type-intersection is exact
2338-
jl_method_table_invalidate(mt, replaced->func.method, max_world);
2346+
jl_method_table_invalidate(mt, m, max_world);
2347+
int m_dispatch = jl_atomic_load_relaxed(&m->dispatch_status);
2348+
jl_atomic_store_relaxed(&m->dispatch_status, 0);
2349+
only = m_dispatch & METHOD_SIG_LATEST_ONLY;
23392350
}
23402351
else {
23412352
jl_method_t *const *d;
@@ -2407,8 +2418,10 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
24072418
memset(morespec, morespec_unknown, n);
24082419
for (j = 0; j < n; j++) {
24092420
jl_method_t *m = d[j];
2410-
if (morespec[j] == (char)morespec_is)
2421+
if (morespec[j] == (char)morespec_is) {
2422+
only = 0;
24112423
continue;
2424+
}
24122425
loctag = jl_atomic_load_relaxed(&m->specializations); // use loctag for a gcroot
24132426
_Atomic(jl_method_instance_t*) *data;
24142427
size_t l;
@@ -2438,7 +2451,7 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
24382451
// not actually shadowing--the existing method is still better
24392452
break;
24402453
if (ambig == morespec_unknown)
2441-
ambig = jl_type_morespecific(type, m->sig) ? morespec_is : morespec_isnot;
2454+
ambig = jl_type_morespecific(type, m->sig) ? morespec_isnot : morespec_is;
24422455
// replacing a method--see if this really was the selected method previously
24432456
// over the intersection (not ambiguous) and the new method will be selected now (morespec_is)
24442457
int replaced_dispatch = is_replacing(ambig, type, m, d, n, isect, isect2, morespec);
@@ -2455,6 +2468,20 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
24552468
invalidated |= invalidatedmi;
24562469
}
24572470
}
2471+
// now compute and store updates to METHOD_SIG_LATEST_ONLY
2472+
int m_dispatch = jl_atomic_load_relaxed(&m->dispatch_status);
2473+
if (m_dispatch & METHOD_SIG_LATEST_ONLY) {
2474+
if (morespec[j] == (char)morespec_unknown)
2475+
morespec[j] = (char)(jl_type_morespecific(m->sig, type) ? morespec_is : morespec_isnot);
2476+
if (morespec[j] == (char)morespec_isnot)
2477+
jl_atomic_store_relaxed(&m->dispatch_status, ~METHOD_SIG_LATEST_ONLY & m_dispatch);
2478+
}
2479+
if (only) {
2480+
if (morespec[j] == (char)morespec_is || ambig == morespec_is ||
2481+
(ambig == morespec_unknown && !jl_type_morespecific(type, m->sig))) {
2482+
only = 0;
2483+
}
2484+
}
24582485
}
24592486
if (jl_array_nrows(oldmi)) {
24602487
// search mt->cache and leafcache and drop anything that might overlap with the new method
@@ -2485,7 +2512,8 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
24852512
loctag = jl_cstr_to_string("jl_method_table_insert");
24862513
jl_array_ptr_1d_push(_jl_debug_method_invalidation, loctag);
24872514
}
2488-
jl_atomic_store_relaxed(&newentry->max_world, jl_atomic_load_relaxed(&method->deleted_world));
2515+
jl_atomic_store_relaxed(&newentry->max_world, ~(size_t)0);
2516+
jl_atomic_store_relaxed(&method->dispatch_status, METHOD_SIG_LATEST_WHICH | (only ? METHOD_SIG_LATEST_ONLY : 0)); // TODO: this should be sequenced fully after the world counter store
24892517
JL_UNLOCK(&mt->writelock);
24902518
JL_GC_POP();
24912519
}
@@ -2499,7 +2527,6 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method
24992527
jl_error("Method changes have been disabled via a call to disable_new_worlds.");
25002528
size_t world = jl_atomic_load_relaxed(&jl_world_counter) + 1;
25012529
jl_atomic_store_relaxed(&method->primary_world, world);
2502-
jl_atomic_store_relaxed(&method->deleted_world, ~(size_t)0);
25032530
jl_method_table_activate(mt, newentry);
25042531
jl_atomic_store_release(&jl_world_counter, world);
25052532
JL_UNLOCK(&world_counter_lock);
@@ -3882,6 +3909,8 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio
38823909
closure->match.min_valid = max_world + 1;
38833910
return 1;
38843911
}
3912+
if (closure->match.max_valid > max_world)
3913+
closure->match.max_valid = max_world;
38853914
jl_method_t *meth = ml->func.method;
38863915
if (closure->lim >= 0 && jl_is_dispatch_tupletype(meth->sig)) {
38873916
int replaced = 0;
@@ -4554,12 +4583,9 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
45544583
jl_method_t *m = matc->method;
45554584
// method applicability is the same as typemapentry applicability
45564585
size_t min_world = jl_atomic_load_relaxed(&m->primary_world);
4557-
size_t max_world = jl_atomic_load_relaxed(&m->deleted_world);
45584586
// intersect the env valid range with method lookup's inclusive valid range
45594587
if (env.match.min_valid < min_world)
45604588
env.match.min_valid = min_world;
4561-
if (env.match.max_valid > max_world)
4562-
env.match.max_valid = max_world;
45634589
}
45644590
if (mt && cache_result && ((jl_datatype_t*)unw)->isdispatchtuple) { // cache_result parameter keeps this from being recursive
45654591
if (len == 1 && !has_ambiguity) {

src/jltypes.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3545,8 +3545,8 @@ void jl_init_types(void) JL_GC_DISABLED
35453545
"module",
35463546
"file",
35473547
"line",
3548+
"dispatch_status", // atomic
35483549
"primary_world", // atomic
3549-
"deleted_world", // atomic
35503550
"sig",
35513551
"specializations", // !const
35523552
"speckeyset", // !const
@@ -3578,7 +3578,7 @@ void jl_init_types(void) JL_GC_DISABLED
35783578
jl_module_type,
35793579
jl_symbol_type,
35803580
jl_int32_type,
3581-
jl_ulong_type,
3581+
jl_int32_type,
35823582
jl_ulong_type,
35833583
jl_type_type,
35843584
jl_any_type, // union(jl_simplevector_type, jl_method_instance_type),

src/julia.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,8 @@ typedef struct _jl_method_t {
331331
struct _jl_module_t *module;
332332
jl_sym_t *file;
333333
int32_t line;
334+
_Atomic(int32_t) dispatch_status; // bits defined in staticdata.jl
334335
_Atomic(size_t) primary_world;
335-
_Atomic(size_t) deleted_world;
336336

337337
// method's type signature. redundant with TypeMapEntry->specTypes
338338
jl_value_t *sig;

src/julia_internal.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,10 @@ typedef union {
677677
#define SOURCE_MODE_NOT_REQUIRED 0x0
678678
#define SOURCE_MODE_ABI 0x1
679679

680+
#define METHOD_SIG_LATEST_WHICH 0b0001
681+
#define METHOD_SIG_LATEST_ONLY 0b0010
682+
#define METHOD_SIG_PRECOMPILE_MANY 0b0100
683+
680684
JL_DLLEXPORT jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner);
681685
JL_DLLEXPORT void jl_engine_fulfill(jl_code_instance_t *ci, jl_code_info_t *src);
682686
void jl_engine_sweep(jl_ptls_t *gc_all_tls_states) JL_NOTSAFEPOINT;

src/method.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *mi, size_t
727727
JL_TRY {
728728
ct->ptls->in_pure_callback = 1;
729729
ct->world_age = jl_atomic_load_relaxed(&def->primary_world);
730-
if (ct->world_age > jl_atomic_load_acquire(&jl_world_counter) || jl_atomic_load_relaxed(&def->deleted_world) < ct->world_age)
730+
if (ct->world_age > jl_atomic_load_acquire(&jl_world_counter))
731731
jl_error("The generator method cannot run until it is added to a method table.");
732732

733733
// invoke code generator
@@ -1006,7 +1006,7 @@ JL_DLLEXPORT jl_method_t *jl_new_method_uninit(jl_module_t *module)
10061006
m->isva = 0;
10071007
m->nargs = 0;
10081008
jl_atomic_store_relaxed(&m->primary_world, ~(size_t)0);
1009-
jl_atomic_store_relaxed(&m->deleted_world, 1);
1009+
jl_atomic_store_relaxed(&m->dispatch_status, 0);
10101010
m->is_for_opaque_closure = 0;
10111011
m->nospecializeinfer = 0;
10121012
jl_atomic_store_relaxed(&m->did_scan_source, 0);

src/opaque_closure.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ JL_DLLEXPORT jl_opaque_closure_t *jl_new_opaque_closure_from_code_info(jl_tuplet
159159
size_t world = jl_current_task->world_age;
160160
// these are only legal in the current world since they are not in any tables
161161
jl_atomic_store_release(&meth->primary_world, world);
162-
jl_atomic_store_release(&meth->deleted_world, world);
163162

164163
if (isinferred) {
165164
jl_value_t *argslotty = jl_array_ptr_ref(ci->slottypes, 0);

0 commit comments

Comments
 (0)