From abf4dcfce4b0c7a1f965e6ef1263f00a72bc20bd Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 3 Mar 2025 08:22:44 +0000 Subject: [PATCH 1/4] Be more selective when invalidating code instances On master, when we invalidate a CodeInstance, we also invalidate the entire associated MethodInstance. However, this is highly problematic, because we have a lot of CodeInstances that are associated with `getproperty(::Module, ::Symbol)` through constant propagation. If one of these CodeInstances gets invalidated (e.g. because the resolution of const-propagated M.s binding changed), it would invalidate essentially the entire world. Prevent this by re-checking the forward edges list to make sure that the code instance we're invalidating is actually in there. --- src/gf.c | 49 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/gf.c b/src/gf.c index 1d93d03cc59d2..c07b90a5c12a2 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1839,7 +1839,7 @@ JL_DLLEXPORT jl_value_t *jl_debug_method_invalidation(int state) return jl_nothing; } -static void _invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, int depth); +static void _invalidate_backedges(jl_method_instance_t *replaced_mi, jl_code_instance_t *replaced_ci, size_t max_world, int depth); // recursively invalidate cached methods that had an edge to a replaced method static void invalidate_code_instance(jl_code_instance_t *replaced, size_t max_world, int depth) @@ -1858,13 +1858,15 @@ static void invalidate_code_instance(jl_code_instance_t *replaced, size_t max_wo if (!jl_is_method(replaced_mi->def.method)) return; // shouldn't happen, but better to be safe JL_LOCK(&replaced_mi->def.method->writelock); - if (jl_atomic_load_relaxed(&replaced->max_world) == ~(size_t)0) { + size_t replacedmaxworld = jl_atomic_load_relaxed(&replaced->max_world); + if (replacedmaxworld == ~(size_t)0) { assert(jl_atomic_load_relaxed(&replaced->min_world) - 1 <= max_world && "attempting to set illogical world constraints (probable race condition)"); jl_atomic_store_release(&replaced->max_world, max_world); + // recurse to all backedges to update their valid range also + _invalidate_backedges(replaced_mi, replaced, max_world, depth + 1); + } else { + assert(jl_atomic_load_relaxed(&replaced->max_world) <= max_world); } - assert(jl_atomic_load_relaxed(&replaced->max_world) <= max_world); - // recurse to all backedges to update their valid range also - _invalidate_backedges(replaced_mi, max_world, depth + 1); JL_UNLOCK(&replaced_mi->def.method->writelock); } @@ -1873,19 +1875,42 @@ JL_DLLEXPORT void jl_invalidate_code_instance(jl_code_instance_t *replaced, size invalidate_code_instance(replaced, max_world, 1); } -static void _invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, int depth) { +static void _invalidate_backedges(jl_method_instance_t *replaced_mi, jl_code_instance_t *replaced_ci, size_t max_world, int depth) { jl_array_t *backedges = replaced_mi->backedges; if (backedges) { // invalidate callers (if any) replaced_mi->backedges = NULL; JL_GC_PUSH1(&backedges); size_t i = 0, l = jl_array_nrows(backedges); + size_t ins = 0; jl_code_instance_t *replaced; while (i < l) { - i = get_next_edge(backedges, i, NULL, &replaced); + jl_value_t *invokesig = NULL; + i = get_next_edge(backedges, i, &invokesig, &replaced); JL_GC_PROMISE_ROOTED(replaced); // propagated by get_next_edge from backedges + if (replaced_ci) { + // If we're invalidating a particular codeinstance, only invalidate + // this backedge it actually has an edge for our codeinstance. + jl_svec_t *edges = jl_atomic_load_relaxed(&replaced->edges); + for (size_t j = 0; j < jl_svec_len(edges); ++j) { + jl_value_t *edge = jl_svecref(edges, j); + if (edge == (jl_value_t*)replaced_mi || edge == (jl_value_t*)replaced_ci) + goto found; + } + // Keep this entry in the backedge list, but compact it + ins = set_next_edge(backedges, ins, invokesig, replaced); + continue; + found:; + } invalidate_code_instance(replaced, max_world, depth); } + if (replaced_ci && ins != 0) { + jl_array_del_end(backedges, l - ins); + // If we're only invalidating one ci, we don't know which ci any particular + // backedge was for, so we can't delete them. Put them back. + replaced_mi->backedges = backedges; + jl_gc_wb(replaced_mi, backedges); + } JL_GC_POP(); } } @@ -1894,7 +1919,7 @@ static void _invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_ static void invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, const char *why) { JL_LOCK(&replaced_mi->def.method->writelock); - _invalidate_backedges(replaced_mi, max_world, 1); + _invalidate_backedges(replaced_mi, NULL, max_world, 1); JL_UNLOCK(&replaced_mi->def.method->writelock); if (why && _jl_debug_method_invalidation) { jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)replaced_mi); @@ -1928,8 +1953,8 @@ JL_DLLEXPORT void jl_method_instance_add_backedge(jl_method_instance_t *callee, size_t i = 0, l = jl_array_nrows(callee->backedges); for (i = 0; i < l; i++) { // optimized version of while (i < l) i = get_next_edge(callee->backedges, i, &invokeTypes, &mi); - jl_value_t *mi = jl_array_ptr_ref(callee->backedges, i); - if (mi != (jl_value_t*)caller) + jl_value_t *ciedge = jl_array_ptr_ref(callee->backedges, i); + if (ciedge != (jl_value_t*)caller) continue; jl_value_t *invokeTypes = i > 0 ? jl_array_ptr_ref(callee->backedges, i - 1) : NULL; if (invokeTypes && jl_is_method_instance(invokeTypes)) @@ -2372,7 +2397,7 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry) continue; loctag = jl_atomic_load_relaxed(&m->specializations); // use loctag for a gcroot _Atomic(jl_method_instance_t*) *data; - size_t i, l; + size_t l; if (jl_is_svec(loctag)) { data = (_Atomic(jl_method_instance_t*)*)jl_svec_data(loctag); l = jl_svec_len(loctag); @@ -2382,7 +2407,7 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry) l = 1; } enum morespec_options ambig = morespec_unknown; - for (i = 0; i < l; i++) { + for (size_t i = 0; i < l; i++) { jl_method_instance_t *mi = jl_atomic_load_relaxed(&data[i]); if ((jl_value_t*)mi == jl_nothing) continue; From cdf8820cd0142a1cff6690981409dfdc28eef1a9 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 3 Mar 2025 04:46:19 +0000 Subject: [PATCH 2/4] bpart: Turn on invalidation for guard->defined transitions This addresses one of the last remaining TODOs of the binding partition work by performing invalidations when bindings transition from being undefined to being defined. This in particular finally addresses the performance issue that #54733 was intended to address (the issue was closed when we merged the mechanism, but it had so far been turned off). Turning on the invalidations themselves were always easy (a one line deletion). What is harder is making sure that the additional invalidations don't take extra time. To this end, we add two additional flags, one on Bindings, and one on methods. The flag on bindings tells us whether any method scan has so far found an implicit (not tracked in ->backedges) reference to this binding in any method body. The insight here is that most undefined bindings will not have been referenced previously (because they did not exist), so with a simple one bit saturating counter of the number of edges that would exist (if we did store them), we can fast-path the invalidation. However, this is not quite sufficient, as people often do things like: ``` foo() = bar() bar() = ... ... ``` which, without further improvements would incur an invalidation upon the definition of `bar`. The second insight (and what the flag on `Method` is for) is that we don't actually need to scan the method body until there is something to invalidate (i.e. until some `CodeInstance` has been created for the method). By defering the scanning until the first time that inference accesses the lowered code (with a flag to only do it once), we can easily avoid invalidation in the above scenario (while still invalidating if `foo()` was called before the definition of `bar`). As a further bonus, this also speeds up bootstrap by about 20% (putting us about back to where we used to be before the full bpart change) by skipping unnecessary invalidations even for non-guard transitions. Finally, this does not yet turn on inference's ability to infer guard partitions as `Union{}`. The reason for this is that such partitions can be replaced by backdated constants without invalidation. However, as soon as we remove the backdated const mechanism, this PR will allow us to turn on that change, further speeding up inference (by cutting off inference on branches known to error due to missing bindings). --- Compiler/src/utilities.jl | 19 +++++++++++++++++++ base/expr.jl | 4 ++++ base/invalidation.jl | 37 ++++++++++++++++++++----------------- base/runtime_internals.jl | 2 ++ src/jltypes.c | 8 +++++--- src/julia.h | 7 ++++++- src/method.c | 28 ++++++++++++++++++++++++---- src/module.c | 7 ++++--- test/rebinding.jl | 20 ++++++++++++++++++++ 9 files changed, 104 insertions(+), 28 deletions(-) diff --git a/Compiler/src/utilities.jl b/Compiler/src/utilities.jl index c322d1062cea1..1c9bdf2229fe8 100644 --- a/Compiler/src/utilities.jl +++ b/Compiler/src/utilities.jl @@ -129,6 +129,25 @@ function retrieve_code_info(mi::MethodInstance, world::UInt) else c = copy(src::CodeInfo) end + if !def.did_scan_source + # This scan must happen: + # 1. After method definition + # 2. Before any code instances that may have relied on information + # from implicit GlobalRefs for this method are added to the cache + # 3. Preferably while the IR is already uncompressed + # 4. As late as possible, as early adding of the backedges may cause + # spurious invalidations. + # + # At the moment we do so here, because + # 1. It's reasonably late + # 2. It has easy access to the uncompressed IR + # 3. We necessarily pass through here before relying on any + # information obtained from implicit GlobalRefs. + # + # However, the exact placement of this scan is not as important as + # long as the above conditions are met. + ccall(:jl_scan_method_source_now, Cvoid, (Any, Any), def, c) + end end if c isa CodeInfo c.parent = mi diff --git a/base/expr.jl b/base/expr.jl index b24e6d67d48d9..634e2ddb7b4de 100644 --- a/base/expr.jl +++ b/base/expr.jl @@ -1352,6 +1352,10 @@ function make_atomic(order, ex) op = :+ elseif ex.head === :(-=) op = :- + elseif ex.head === :(|=) + op = :| + elseif ex.head === :(&=) + op = :& elseif @isdefined string shead = string(ex.head) if endswith(shead, '=') diff --git a/base/invalidation.jl b/base/invalidation.jl index 539999c15dd1d..c0ef6ee75374d 100644 --- a/base/invalidation.jl +++ b/base/invalidation.jl @@ -115,28 +115,27 @@ end function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core.BindingPartition, new_bpart::Union{Core.BindingPartition, Nothing}, new_max_world::UInt) gr = b.globalref - if !is_some_guard(binding_kind(invalidated_bpart)) - # TODO: We may want to invalidate for these anyway, since they have performance implications + if (b.flags & BINDING_FLAG_ANY_IMPLICIT_EDGES) != 0 foreach_module_mtable(gr.mod, new_max_world) do mt::Core.MethodTable for method in MethodList(mt) invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world) end return true end - if isdefined(b, :backedges) - for edge in b.backedges - if isa(edge, CodeInstance) - ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), edge, new_max_world) - elseif isa(edge, Core.Binding) - isdefined(edge, :partitions) || continue - latest_bpart = edge.partitions - latest_bpart.max_world == typemax(UInt) || continue - is_some_imported(binding_kind(latest_bpart)) || continue - partition_restriction(latest_bpart) === b || continue - invalidate_code_for_globalref!(edge, latest_bpart, nothing, new_max_world) - else - invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world) - end + end + if isdefined(b, :backedges) + for edge in b.backedges + if isa(edge, CodeInstance) + ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), edge, new_max_world) + elseif isa(edge, Core.Binding) + isdefined(edge, :partitions) || continue + latest_bpart = edge.partitions + latest_bpart.max_world == typemax(UInt) || continue + is_some_imported(binding_kind(latest_bpart)) || continue + partition_restriction(latest_bpart) === b || continue + invalidate_code_for_globalref!(edge, latest_bpart, nothing, new_max_world) + else + invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world) end end end @@ -166,7 +165,11 @@ gr_needs_backedge_in_module(gr::GlobalRef, mod::Module) = gr.mod !== mod # N.B.: This needs to match jl_maybe_add_binding_backedge function maybe_add_binding_backedge!(b::Core.Binding, edge::Union{Method, CodeInstance}) method = isa(edge, Method) ? edge : edge.def.def::Method - gr_needs_backedge_in_module(b.globalref, method.module) || return + methmod = method.module + if !gr_needs_backedge_in_module(b.globalref, methmod) + @atomic :acquire_release b.flags |= BINDING_FLAG_ANY_IMPLICIT_EDGES + return + end if !isdefined(b, :backedges) b.backedges = Any[] end diff --git a/base/runtime_internals.jl b/base/runtime_internals.jl index 9fd621e0e0da8..e448edfb589f2 100644 --- a/base/runtime_internals.jl +++ b/base/runtime_internals.jl @@ -216,6 +216,8 @@ const PARTITION_FLAG_DEPWARN = 0x40 const PARTITION_MASK_KIND = 0x0f const PARTITION_MASK_FLAG = 0xf0 +const BINDING_FLAG_ANY_IMPLICIT_EDGES = 0x8 + is_defined_const_binding(kind::UInt8) = (kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_BACKDATED_CONST) is_some_const_binding(kind::UInt8) = (is_defined_const_binding(kind) || kind == PARTITION_KIND_UNDEF_CONST) is_some_imported(kind::UInt8) = (kind == PARTITION_KIND_IMPLICIT || kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED) diff --git a/src/jltypes.c b/src/jltypes.c index 4ab1f02758c69..3b93684880801 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -3275,7 +3275,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_svec(5, jl_any_type/*jl_globalref_type*/, jl_any_type, jl_binding_partition_type, jl_any_type, jl_uint8_type), jl_emptysvec, 0, 1, 0); - const static uint32_t binding_atomicfields[] = { 0x0005 }; // Set fields 2, 3 as atomic + const static uint32_t binding_atomicfields[] = { 0x0016 }; // Set fields 2, 3, 5 as atomic jl_binding_type->name->atomicfields = binding_atomicfields; const static uint32_t binding_constfields[] = { 0x0001 }; // Set fields 1 as constant jl_binding_type->name->constfields = binding_constfields; @@ -3539,7 +3539,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_method_type = jl_new_datatype(jl_symbol("Method"), core, jl_any_type, jl_emptysvec, - jl_perm_symsvec(31, + jl_perm_symsvec(32, "name", "module", "file", @@ -3568,10 +3568,11 @@ void jl_init_types(void) JL_GC_DISABLED "isva", "is_for_opaque_closure", "nospecializeinfer", + "did_scan_source", "constprop", "max_varargs", "purity"), - jl_svec(31, + jl_svec(32, jl_symbol_type, jl_module_type, jl_symbol_type, @@ -3600,6 +3601,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_bool_type, jl_bool_type, jl_bool_type, + jl_bool_type, jl_uint8_type, jl_uint8_type, jl_uint16_type), diff --git a/src/julia.h b/src/julia.h index 0c1d4137f313c..521074df523ce 100644 --- a/src/julia.h +++ b/src/julia.h @@ -375,6 +375,8 @@ typedef struct _jl_method_t { uint8_t isva; uint8_t is_for_opaque_closure; uint8_t nospecializeinfer; + _Atomic(uint8_t) did_scan_source; + // uint8 settings uint8_t constprop; // 0x00 = use heuristic; 0x01 = aggressive; 0x02 = none uint8_t max_varargs; // 0xFF = use heuristic; otherwise, max # of args to expand @@ -751,7 +753,10 @@ enum jl_binding_flags { BINDING_FLAG_DID_PRINT_BACKDATE_ADMONITION = 0x1, BINDING_FLAG_DID_PRINT_IMPLICIT_IMPORT_ADMONITION = 0x2, // `export` is tracked in partitions, but sets this as well - BINDING_FLAG_PUBLICP = 0x4 + BINDING_FLAG_PUBLICP = 0x4, + // Set if any methods defined in this module implicitly reference + // this binding. If not, invalidation is optimized. + BINDING_FLAG_ANY_IMPLICIT_EDGES = 0x8 }; typedef struct _jl_binding_t { diff --git a/src/method.c b/src/method.c index ffd33645310f3..1eb361143de6a 100644 --- a/src/method.c +++ b/src/method.c @@ -39,6 +39,28 @@ static void check_c_types(const char *where, jl_value_t *rt, jl_value_t *at) } } +JL_DLLEXPORT void jl_scan_method_source_now(jl_method_t *m, jl_value_t *src) +{ + if (!jl_atomic_load_relaxed(&m->did_scan_source)) { + jl_code_info_t *code = NULL; + JL_GC_PUSH1(&code); + if (!jl_is_code_info(src)) + code = jl_uncompress_ir(m, NULL, src); + else + code = (jl_code_info_t*)src; + jl_array_t *stmts = code->code; + size_t i, l = jl_array_nrows(stmts); + for (i = 0; i < l; i++) { + jl_value_t *stmt = jl_array_ptr_ref(stmts, i); + if (jl_is_globalref(stmt)) { + jl_maybe_add_binding_backedge((jl_globalref_t*)stmt, m->module, (jl_value_t*)m); + } + } + jl_atomic_store_relaxed(&m->did_scan_source, 1); + JL_GC_POP(); + } +} + // Resolve references to non-locally-defined variables to become references to global // variables in `module` (unless the rvalue is one of the type parameters in `sparam_vals`). static jl_value_t *resolve_definition_effects(jl_value_t *expr, jl_module_t *module, jl_svec_t *sparam_vals, jl_value_t *binding_edge, @@ -47,10 +69,7 @@ static jl_value_t *resolve_definition_effects(jl_value_t *expr, jl_module_t *mod if (jl_is_symbol(expr)) { jl_error("Found raw symbol in code returned from lowering. Expected all symbols to have been resolved to GlobalRef or slots."); } - if (jl_is_globalref(expr)) { - jl_maybe_add_binding_backedge((jl_globalref_t*)expr, module, binding_edge); - return expr; - } + if (!jl_is_expr(expr)) { return expr; } @@ -973,6 +992,7 @@ JL_DLLEXPORT jl_method_t *jl_new_method_uninit(jl_module_t *module) jl_atomic_store_relaxed(&m->deleted_world, 1); m->is_for_opaque_closure = 0; m->nospecializeinfer = 0; + jl_atomic_store_relaxed(&m->did_scan_source, 0); m->constprop = 0; m->purity.bits = 0; m->max_varargs = UINT8_MAX; diff --git a/src/module.c b/src/module.c index 6b31c3f851a79..ed4713045a40d 100644 --- a/src/module.c +++ b/src/module.c @@ -1345,15 +1345,16 @@ JL_DLLEXPORT void jl_maybe_add_binding_backedge(jl_globalref_t *gr, jl_module_t { if (!edge) return; + jl_binding_t *b = gr->binding; + if (!b) + b = jl_get_module_binding(gr->mod, gr->name, 1); // N.B.: The logic for evaluating whether a backedge is required must // match the invalidation logic. if (gr->mod == defining_module) { // No backedge required - invalidation will forward scan + jl_atomic_fetch_or(&b->flags, BINDING_FLAG_ANY_IMPLICIT_EDGES); return; } - jl_binding_t *b = gr->binding; - if (!b) - b = jl_get_module_binding(gr->mod, gr->name, 1); jl_add_binding_backedge(b, edge); } diff --git a/test/rebinding.jl b/test/rebinding.jl index 8f54aa99b15bd..ab9696c7f0222 100644 --- a/test/rebinding.jl +++ b/test/rebinding.jl @@ -298,3 +298,23 @@ module RangeMerge @test !contains(get_llvm(f, Tuple{}), "jl_get_binding_value") end + +# Test that we invalidate for undefined -> defined transitions (#54733) +module UndefinedTransitions + using Test + function foo54733() + for i = 1:1_000_000_000 + bar54733(i) + end + return 1 + end + @test_throws UndefVarError foo54733() + let ci = first(methods(foo54733)).specializations.cache + @test !Base.Compiler.is_nothrow(Base.Compiler.decode_effects(ci.ipo_purity_bits)) + end + bar54733(x) = 3x + @test foo54733() === 1 + let ci = first(methods(foo54733)).specializations.cache + @test Base.Compiler.is_nothrow(Base.Compiler.decode_effects(ci.ipo_purity_bits)) + end +end From e7efe42ece419c04d170887d856609286e63836b Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 3 Mar 2025 06:41:36 +0000 Subject: [PATCH 3/4] bpart: Don't invalidate for changes that inference can't see This implements the optimizations I promised in #57602 by checking in invalidation whether or not the information that inference sees changes. The primary situation in which this is useful is avoiding an invalidation for `export` flag changes or changes of which module a binding is being imported from that do not change what the actual binding being imported is. Base itself uses these patterns sparingly, so the bootstrap impact over #57615 is minimal (though non-zero). --- Compiler/src/abstractinterpretation.jl | 18 ++++--- base/invalidation.jl | 72 ++++++++++++++++++-------- src/module.c | 44 +++++++++++++--- 3 files changed, 97 insertions(+), 37 deletions(-) diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl index a9be5e9216458..e8d459fc43288 100644 --- a/Compiler/src/abstractinterpretation.jl +++ b/Compiler/src/abstractinterpretation.jl @@ -3633,18 +3633,20 @@ scan_partitions(query::Function, interp, g::GlobalRef, wwr::WorldWithRange) = abstract_load_all_consistent_leaf_partitions(interp, g::GlobalRef, wwr::WorldWithRange) = scan_leaf_partitions(abstract_eval_partition_load, interp, g, wwr) +function abstract_eval_globalref_partition(interp, binding::Core.Binding, partition::Core.BindingPartition) + # For inference purposes, we don't particularly care which global binding we end up loading, we only + # care about its type. However, we would still like to terminate the world range for the particular + # binding we end up reaching such that codegen can emit a simpler pointer load. + Pair{RTEffects, Union{Nothing, Core.Binding}}( + abstract_eval_partition_load(interp, partition), + binding_kind(partition) in (PARTITION_KIND_GLOBAL, PARTITION_KIND_DECLARED) ? binding : nothing) +end + function abstract_eval_globalref(interp, g::GlobalRef, saw_latestworld::Bool, sv::AbsIntState) if saw_latestworld return RTEffects(Any, Any, generic_getglobal_effects) end - (valid_worlds, (ret, binding_if_global)) = scan_leaf_partitions(interp, g, sv.world) do interp, binding, partition - # For inference purposes, we don't particularly care which global binding we end up loading, we only - # care about its type. However, we would still like to terminate the world range for the particular - # binding we end up reaching such that codegen can emit a simpler pointer load. - Pair{RTEffects, Union{Nothing, Core.Binding}}( - abstract_eval_partition_load(interp, partition), - binding_kind(partition) in (PARTITION_KIND_GLOBAL, PARTITION_KIND_DECLARED) ? binding : nothing) - end + (valid_worlds, (ret, binding_if_global)) = scan_leaf_partitions(abstract_eval_globalref_partition, interp, g, sv.world) update_valid_age!(sv, valid_worlds) if ret.rt !== Union{} && ret.exct === UndefVarError && binding_if_global !== nothing && InferenceParams(interp).assume_bindings_static if isdefined(binding_if_global, :value) diff --git a/base/invalidation.jl b/base/invalidation.jl index c0ef6ee75374d..9c4f27d3ad844 100644 --- a/base/invalidation.jl +++ b/base/invalidation.jl @@ -113,33 +113,55 @@ function invalidate_method_for_globalref!(gr::GlobalRef, method::Method, invalid end end -function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core.BindingPartition, new_bpart::Union{Core.BindingPartition, Nothing}, new_max_world::UInt) +export_affecting_partition_flags(bpart::Core.BindingPartition) = + ((bpart.kind & PARTITION_MASK_KIND) == PARTITION_KIND_GUARD, + (bpart.kind & PARTITION_FLAG_EXPORTED) != 0, + (bpart.kind & PARTITION_FLAG_DEPRECATED) != 0) + +function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core.BindingPartition, new_bpart::Core.BindingPartition, new_max_world::UInt) gr = b.globalref - if (b.flags & BINDING_FLAG_ANY_IMPLICIT_EDGES) != 0 - foreach_module_mtable(gr.mod, new_max_world) do mt::Core.MethodTable - for method in MethodList(mt) - invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world) + + (_, (ib, ibpart)) = Compiler.walk_binding_partition(b, invalidated_bpart, new_max_world) + (_, (nb, nbpart)) = Compiler.walk_binding_partition(b, new_bpart, new_max_world+1) + + # abstract_eval_globalref_partition is the maximum amount of information that inference + # reads from a binding partition. If this information does not change - we do not need to + # invalidate any code that inference created, because we know that the result will not change. + need_to_invalidate_code = + Compiler.abstract_eval_globalref_partition(nothing, ib, ibpart) !== + Compiler.abstract_eval_globalref_partition(nothing, nb, nbpart) + + need_to_invalidate_export = export_affecting_partition_flags(invalidated_bpart) !== + export_affecting_partition_flags(new_bpart) + + if need_to_invalidate_code + if (b.flags & BINDING_FLAG_ANY_IMPLICIT_EDGES) != 0 + foreach_module_mtable(gr.mod, new_max_world) do mt::Core.MethodTable + for method in MethodList(mt) + invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world) + end + return true end - return true end - end - if isdefined(b, :backedges) - for edge in b.backedges - if isa(edge, CodeInstance) - ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), edge, new_max_world) - elseif isa(edge, Core.Binding) - isdefined(edge, :partitions) || continue - latest_bpart = edge.partitions - latest_bpart.max_world == typemax(UInt) || continue - is_some_imported(binding_kind(latest_bpart)) || continue - partition_restriction(latest_bpart) === b || continue - invalidate_code_for_globalref!(edge, latest_bpart, nothing, new_max_world) - else - invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world) + if isdefined(b, :backedges) + for edge in b.backedges + if isa(edge, CodeInstance) + ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), edge, new_max_world) + elseif isa(edge, Core.Binding) + isdefined(edge, :partitions) || continue + latest_bpart = edge.partitions + latest_bpart.max_world == typemax(UInt) || continue + is_some_imported(binding_kind(latest_bpart)) || continue + partition_restriction(latest_bpart) === b || continue + invalidate_code_for_globalref!(edge, latest_bpart, latest_bpart, new_max_world) + else + invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world) + end end end end - if (invalidated_bpart.kind & PARTITION_FLAG_EXPORTED != 0) || (new_bpart !== nothing && (new_bpart.kind & PARTITION_FLAG_EXPORTED != 0)) + + if need_to_invalidate_code || need_to_invalidate_export # This binding was exported - we need to check all modules that `using` us to see if they # have a binding that is affected by this change. usings_backedges = ccall(:jl_get_module_usings_backedges, Any, (Any,), gr.mod) @@ -151,8 +173,12 @@ function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core latest_bpart = user_binding.partitions latest_bpart.max_world == typemax(UInt) || continue binding_kind(latest_bpart) in (PARTITION_KIND_IMPLICIT, PARTITION_KIND_FAILED, PARTITION_KIND_GUARD) || continue - @atomic :release latest_bpart.max_world = new_max_world - invalidate_code_for_globalref!(convert(Core.Binding, user_binding), latest_bpart, nothing, new_max_world) + new_bpart = need_to_invalidate_export ? + ccall(:jl_maybe_reresolve_implicit, Any, (Any, Any, Csize_t), user_binding, latest_bpart, new_max_world) : + latest_bpart + if need_to_invalidate_code || new_bpart !== latest_bpart + invalidate_code_for_globalref!(convert(Core.Binding, user_binding), latest_bpart, new_bpart, new_max_world) + end end end end diff --git a/src/module.c b/src/module.c index ed4713045a40d..648d0b047408e 100644 --- a/src/module.c +++ b/src/module.c @@ -106,6 +106,9 @@ void jl_check_new_binding_implicit( if (tempbmax_world < max_world) max_world = tempbmax_world; + // N.B.: Which aspects of the partition are considered here needs to + // be kept in sync with `export_affecting_partition_flags` in the + // invalidation code. if ((tempbpart->kind & PARTITION_FLAG_EXPORTED) == 0) continue; @@ -165,6 +168,29 @@ void jl_check_new_binding_implicit( return; } +JL_DLLEXPORT jl_binding_partition_t *jl_maybe_reresolve_implicit(jl_binding_t *b, size_t new_max_world) +{ + jl_binding_partition_t *new_bpart = new_binding_partition(); + jl_binding_partition_t *bpart = jl_atomic_load_acquire(&b->partitions); + assert(bpart); + while (1) { + jl_atomic_store_relaxed(&new_bpart->next, bpart); + jl_gc_wb(new_bpart, bpart); + jl_check_new_binding_implicit(new_bpart, b, NULL, new_max_world+1); + JL_GC_PROMISE_ROOTED(new_bpart); // TODO: Analyzer doesn't understand MAYBE_UNROOTED properly + if (bpart->kind & PARTITION_FLAG_EXPORTED) + new_bpart->kind |= PARTITION_FLAG_EXPORTED; + if (new_bpart->kind == bpart->kind && new_bpart->restriction == bpart->restriction) + return bpart; + // Resolution changed, insert the new partition + size_t expected_max_world = ~(size_t)0; + if (jl_atomic_cmpswap(&bpart->max_world, &expected_max_world, new_max_world) && + jl_atomic_cmpswap(&b->partitions, &bpart, new_bpart)) + break; + } + return new_bpart; +} + STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_(jl_binding_t *b JL_PROPAGATES_ROOT, jl_value_t *parent, _Atomic(jl_binding_partition_t *)*insert, size_t world, modstack_t *st) JL_GLOBALLY_ROOTED { assert(jl_is_binding(b)); @@ -183,7 +209,7 @@ STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_(jl_binding_t *b if (!new_bpart) new_bpart = new_binding_partition(); jl_atomic_store_relaxed(&new_bpart->next, bpart); - jl_gc_wb_fresh(new_bpart, bpart); + jl_gc_wb(new_bpart, bpart); // Not fresh the second time around the loop new_bpart->min_world = bpart ? jl_atomic_load_relaxed(&bpart->max_world) + 1 : 0; jl_atomic_store_relaxed(&new_bpart->max_world, max_world); JL_GC_PROMISE_ROOTED(new_bpart); // TODO: Analyzer doesn't understand MAYBE_UNROOTED properly @@ -1112,10 +1138,12 @@ JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from) jl_sym_t *var = b->globalref->name; jl_binding_t *tob = jl_get_module_binding(to, var, 0); if (tob) { - jl_binding_partition_t *tobpart = jl_get_binding_partition(tob, new_world); - enum jl_partition_kind kind = jl_binding_kind(tobpart); - if (jl_bkind_is_some_implicit(kind)) { - jl_replace_binding_locked(tob, tobpart, NULL, PARTITION_KIND_IMPLICIT_RECOMPUTE, new_world); + jl_binding_partition_t *tobpart = jl_atomic_load_relaxed(&tob->partitions); + if (tobpart) { + enum jl_partition_kind kind = jl_binding_kind(tobpart); + if (jl_bkind_is_some_implicit(kind)) { + jl_replace_binding_locked(tob, tobpart, NULL, PARTITION_KIND_IMPLICIT_RECOMPUTE, new_world); + } } } } @@ -1380,7 +1408,6 @@ JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b, jl_atomic_store_relaxed(&jl_first_image_replacement_world, new_world); assert(jl_atomic_load_relaxed(&b->partitions) == old_bpart); - jl_atomic_store_release(&old_bpart->max_world, new_world-1); jl_binding_partition_t *new_bpart = new_binding_partition(); JL_GC_PUSH1(&new_bpart); new_bpart->min_world = new_world; @@ -1388,12 +1415,17 @@ JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b, assert(!restriction_val); jl_check_new_binding_implicit(new_bpart /* callee rooted */, b, NULL, new_world); new_bpart->kind |= kind & PARTITION_MASK_FLAG; + if (new_bpart->kind == old_bpart->kind && new_bpart->restriction == old_bpart->restriction) { + JL_GC_POP(); + return old_bpart; + } } else { new_bpart->kind = kind; new_bpart->restriction = restriction_val; jl_gc_wb_fresh(new_bpart, restriction_val); } + jl_atomic_store_release(&old_bpart->max_world, new_world-1); jl_atomic_store_relaxed(&new_bpart->next, old_bpart); jl_gc_wb_fresh(new_bpart, old_bpart); From a8a1c3bcc378d545148f7cb4be2c5869c897e8ea Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 3 Mar 2025 19:30:53 +0000 Subject: [PATCH 4/4] Align module base between invalidation and edge tracking Our implicit edge tracking for bindings does not explicitly store any edges for bindings in the *current* module. The idea behind this is that this is a good time-space tradeoff for validation, because substantially all binding references in a module will be to its defining module, while the total number of methods within a module is limited and substantially smaller than the total number of methods in the entire system. However, we have an issue where the code that stores these edges and the invalidation code disagree on which module is the *current* one. The edge storing code was using the module in which the method was defined, while the invalidation code was using the one in which the MethodTable is defined. With these being misaligned, we can miss necessary invalidations. Both options are in principle possible, but I think the former is better, because the module in which the method is defined is also the module that we are likely to have a lot of references to (since they get referenced implicitly by just writing symbols in the code). However, this presents a problem: We don't actually have a way to iterate all the methods defined in a particular module, without just doing the brute force thing of scanning all methods and filtering. To address this, build on the deferred scanning code added in #57615 to also add any scanned modules to an explicit list in `Module`. This costs some space, but only proportional to the number of defined methods, (and thus proportional to the written source code). Note that we don't actually observe any issues in the test suite on master due to this bug. However, this is because we are grossly over-invalidating, which hides the missing invalidations from this issue (#57617). --- Compiler/src/utilities.jl | 2 +- base/invalidation.jl | 28 ++++++++------------------- src/gc-stock.c | 3 +++ src/jltypes.c | 2 +- src/julia.h | 4 ++++ src/julia_internal.h | 3 ++- src/method.c | 23 +++++++++++++++++++--- src/module.c | 40 ++++++++++++++++++++++++++++++--------- src/staticdata.c | 4 ++++ 9 files changed, 74 insertions(+), 35 deletions(-) diff --git a/Compiler/src/utilities.jl b/Compiler/src/utilities.jl index 1c9bdf2229fe8..6113bdb14e7df 100644 --- a/Compiler/src/utilities.jl +++ b/Compiler/src/utilities.jl @@ -129,7 +129,7 @@ function retrieve_code_info(mi::MethodInstance, world::UInt) else c = copy(src::CodeInfo) end - if !def.did_scan_source + if (def.did_scan_source & 0x1) == 0x0 # This scan must happen: # 1. After method definition # 2. Before any code instances that may have relied on information diff --git a/base/invalidation.jl b/base/invalidation.jl index 9c4f27d3ad844..5b48af0171205 100644 --- a/base/invalidation.jl +++ b/base/invalidation.jl @@ -136,11 +136,10 @@ function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core if need_to_invalidate_code if (b.flags & BINDING_FLAG_ANY_IMPLICIT_EDGES) != 0 - foreach_module_mtable(gr.mod, new_max_world) do mt::Core.MethodTable - for method in MethodList(mt) - invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world) - end - return true + nmethods = ccall(:jl_module_scanned_methods_length, Csize_t, (Any,), gr.mod) + for i = 1:nmethods + method = ccall(:jl_module_scanned_methods_getindex, Any, (Any, Csize_t), gr.mod, i)::Method + invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world) end end if isdefined(b, :backedges) @@ -166,7 +165,7 @@ function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core # have a binding that is affected by this change. usings_backedges = ccall(:jl_get_module_usings_backedges, Any, (Any,), gr.mod) if usings_backedges !== nothing - for user in usings_backedges::Vector{Any} + for user::Module in usings_backedges::Vector{Any} user_binding = ccall(:jl_get_module_binding_or_nothing, Any, (Any, Any), user, gr.name) user_binding === nothing && continue isdefined(user_binding, :partitions) || continue @@ -186,21 +185,10 @@ end invalidate_code_for_globalref!(gr::GlobalRef, invalidated_bpart::Core.BindingPartition, new_bpart::Core.BindingPartition, new_max_world::UInt) = invalidate_code_for_globalref!(convert(Core.Binding, gr), invalidated_bpart, new_bpart, new_max_world) -gr_needs_backedge_in_module(gr::GlobalRef, mod::Module) = gr.mod !== mod - -# N.B.: This needs to match jl_maybe_add_binding_backedge function maybe_add_binding_backedge!(b::Core.Binding, edge::Union{Method, CodeInstance}) - method = isa(edge, Method) ? edge : edge.def.def::Method - methmod = method.module - if !gr_needs_backedge_in_module(b.globalref, methmod) - @atomic :acquire_release b.flags |= BINDING_FLAG_ANY_IMPLICIT_EDGES - return - end - if !isdefined(b, :backedges) - b.backedges = Any[] - end - !isempty(b.backedges) && b.backedges[end] === edge && return - push!(b.backedges, edge) + meth = isa(edge, Method) ? edge : Compiler.get_ci_mi(edge).def + ccall(:jl_maybe_add_binding_backedge, Cint, (Any, Any, Any), b, edge, meth) + return nothing end function binding_was_invalidated(b::Core.Binding) diff --git a/src/gc-stock.c b/src/gc-stock.c index 3d3bc9f485e51..0bc4ceca52257 100644 --- a/src/gc-stock.c +++ b/src/gc-stock.c @@ -2147,6 +2147,9 @@ STATIC_INLINE void gc_mark_module_binding(jl_ptls_t ptls, jl_module_t *mb_parent gc_assert_parent_validity((jl_value_t *)mb_parent, (jl_value_t *)mb_parent->usings_backedges); gc_try_claim_and_push(mq, (jl_value_t *)mb_parent->usings_backedges, &nptr); gc_heap_snapshot_record_binding_partition_edge((jl_value_t*)mb_parent, mb_parent->usings_backedges); + gc_assert_parent_validity((jl_value_t *)mb_parent, (jl_value_t *)mb_parent->scanned_methods); + gc_try_claim_and_push(mq, (jl_value_t *)mb_parent->scanned_methods, &nptr); + gc_heap_snapshot_record_binding_partition_edge((jl_value_t*)mb_parent, mb_parent->scanned_methods); size_t nusings = module_usings_length(mb_parent); if (nusings > 0) { // this is only necessary because bindings for "using" modules diff --git a/src/jltypes.c b/src/jltypes.c index 3b93684880801..0cbf5c83fe1d0 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -3601,7 +3601,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_bool_type, jl_bool_type, jl_bool_type, - jl_bool_type, + jl_uint8_type, jl_uint8_type, jl_uint8_type, jl_uint16_type), diff --git a/src/julia.h b/src/julia.h index 521074df523ce..c67e19db2a6fc 100644 --- a/src/julia.h +++ b/src/julia.h @@ -375,6 +375,8 @@ typedef struct _jl_method_t { uint8_t isva; uint8_t is_for_opaque_closure; uint8_t nospecializeinfer; + // bit flags, 0x01 = scanned + // 0x02 = added to module scanned list (either from scanning or inference edge) _Atomic(uint8_t) did_scan_source; // uint8 settings @@ -782,6 +784,7 @@ typedef struct _jl_module_t { jl_sym_t *file; int32_t line; jl_value_t *usings_backedges; + jl_value_t *scanned_methods; // hidden fields: arraylist_t usings; /* arraylist of struct jl_module_using */ // modules with all bindings potentially imported jl_uuid_t build_id; @@ -2059,6 +2062,7 @@ JL_DLLEXPORT int jl_get_module_infer(jl_module_t *m); JL_DLLEXPORT void jl_set_module_max_methods(jl_module_t *self, int value); JL_DLLEXPORT int jl_get_module_max_methods(jl_module_t *m); JL_DLLEXPORT jl_value_t *jl_get_module_usings_backedges(jl_module_t *m); +JL_DLLEXPORT jl_value_t *jl_get_module_scanned_methods(jl_module_t *m); JL_DLLEXPORT jl_value_t *jl_get_module_binding_or_nothing(jl_module_t *m, jl_sym_t *s); // get binding for reading diff --git a/src/julia_internal.h b/src/julia_internal.h index 93c5fb4f0eb0b..204790d7a7703 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -722,7 +722,7 @@ jl_code_info_t *jl_new_code_info_from_ir(jl_expr_t *ast); JL_DLLEXPORT jl_code_info_t *jl_new_code_info_uninit(void); JL_DLLEXPORT void jl_resolve_definition_effects_in_ir(jl_array_t *stmts, jl_module_t *m, jl_svec_t *sparam_vals, jl_value_t *binding_edge, int binding_effects); -JL_DLLEXPORT void jl_maybe_add_binding_backedge(jl_globalref_t *gr, jl_module_t *defining_module, jl_value_t *edge); +JL_DLLEXPORT int jl_maybe_add_binding_backedge(jl_binding_t *b, jl_value_t *edge, jl_method_t *in_method); JL_DLLEXPORT void jl_add_binding_backedge(jl_binding_t *b, jl_value_t *edge); int get_next_edge(jl_array_t *list, int i, jl_value_t** invokesig, jl_code_instance_t **caller) JL_NOTSAFEPOINT; @@ -878,6 +878,7 @@ STATIC_INLINE size_t module_usings_max(jl_module_t *m) JL_NOTSAFEPOINT { } JL_DLLEXPORT jl_sym_t *jl_module_name(jl_module_t *m) JL_NOTSAFEPOINT; +void jl_add_scanned_method(jl_module_t *m, jl_method_t *meth); jl_value_t *jl_eval_global_var(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *e); jl_value_t *jl_interpret_opaque_closure(jl_opaque_closure_t *clos, jl_value_t **args, size_t nargs); jl_value_t *jl_interpret_toplevel_thunk(jl_module_t *m, jl_code_info_t *src); diff --git a/src/method.c b/src/method.c index 1eb361143de6a..11609f9d3b61d 100644 --- a/src/method.c +++ b/src/method.c @@ -39,9 +39,20 @@ static void check_c_types(const char *where, jl_value_t *rt, jl_value_t *at) } } +void jl_add_scanned_method(jl_module_t *m, jl_method_t *meth) +{ + JL_LOCK(&m->lock); + if (m->scanned_methods == jl_nothing) { + m->scanned_methods = (jl_value_t*)jl_alloc_vec_any(0); + jl_gc_wb(m, m->scanned_methods); + } + jl_array_ptr_1d_push((jl_array_t*)m->scanned_methods, (jl_value_t*)meth); + JL_UNLOCK(&m->lock); +} + JL_DLLEXPORT void jl_scan_method_source_now(jl_method_t *m, jl_value_t *src) { - if (!jl_atomic_load_relaxed(&m->did_scan_source)) { + if (!jl_atomic_fetch_or(&m->did_scan_source, 1)) { jl_code_info_t *code = NULL; JL_GC_PUSH1(&code); if (!jl_is_code_info(src)) @@ -50,13 +61,19 @@ JL_DLLEXPORT void jl_scan_method_source_now(jl_method_t *m, jl_value_t *src) code = (jl_code_info_t*)src; jl_array_t *stmts = code->code; size_t i, l = jl_array_nrows(stmts); + int any_implicit = 0; for (i = 0; i < l; i++) { jl_value_t *stmt = jl_array_ptr_ref(stmts, i); if (jl_is_globalref(stmt)) { - jl_maybe_add_binding_backedge((jl_globalref_t*)stmt, m->module, (jl_value_t*)m); + jl_globalref_t *gr = (jl_globalref_t*)stmt; + jl_binding_t *b = gr->binding; + if (!b) + b = jl_get_module_binding(gr->mod, gr->name, 1); + any_implicit |= jl_maybe_add_binding_backedge(b, (jl_value_t*)m, m); } } - jl_atomic_store_relaxed(&m->did_scan_source, 1); + if (any_implicit && !(jl_atomic_fetch_or(&m->did_scan_source, 0x2) & 0x2)) + jl_add_scanned_method(m->module, m); JL_GC_POP(); } } diff --git a/src/module.c b/src/module.c index 648d0b047408e..3b557251bc577 100644 --- a/src/module.c +++ b/src/module.c @@ -209,7 +209,8 @@ STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_(jl_binding_t *b if (!new_bpart) new_bpart = new_binding_partition(); jl_atomic_store_relaxed(&new_bpart->next, bpart); - jl_gc_wb(new_bpart, bpart); // Not fresh the second time around the loop + if (bpart) + jl_gc_wb(new_bpart, bpart); // Not fresh the second time around the loop new_bpart->min_world = bpart ? jl_atomic_load_relaxed(&bpart->max_world) + 1 : 0; jl_atomic_store_relaxed(&new_bpart->max_world, max_world); JL_GC_PROMISE_ROOTED(new_bpart); // TODO: Analyzer doesn't understand MAYBE_UNROOTED properly @@ -319,6 +320,7 @@ JL_DLLEXPORT jl_module_t *jl_new_module__(jl_sym_t *name, jl_module_t *parent) m->build_id.hi = ~(uint64_t)0; jl_atomic_store_relaxed(&m->counter, 1); m->usings_backedges = jl_nothing; + m->scanned_methods = jl_nothing; m->nospecialize = 0; m->optlevel = -1; m->compile = -1; @@ -1163,6 +1165,25 @@ JL_DLLEXPORT jl_value_t *jl_get_module_usings_backedges(jl_module_t *m) return m->usings_backedges; } +JL_DLLEXPORT size_t jl_module_scanned_methods_length(jl_module_t *m) +{ + JL_LOCK(&m->lock); + size_t len = 0; + if (m->scanned_methods != jl_nothing) + len = jl_array_len(m->scanned_methods); + JL_UNLOCK(&m->lock); + return len; +} + +JL_DLLEXPORT jl_value_t *jl_module_scanned_methods_getindex(jl_module_t *m, size_t i) +{ + JL_LOCK(&m->lock); + assert(m->scanned_methods != jl_nothing); + jl_value_t *ret = jl_array_ptr_ref(m->scanned_methods, i-1); + JL_UNLOCK(&m->lock); + return ret; +} + JL_DLLEXPORT jl_value_t *jl_get_module_binding_or_nothing(jl_module_t *m, jl_sym_t *s) { jl_binding_t *b = jl_get_module_binding(m, s, 0); @@ -1369,21 +1390,22 @@ JL_DLLEXPORT void jl_add_binding_backedge(jl_binding_t *b, jl_value_t *edge) // Called for all GlobalRefs found in lowered code. Adds backedges for cross-module // GlobalRefs. -JL_DLLEXPORT void jl_maybe_add_binding_backedge(jl_globalref_t *gr, jl_module_t *defining_module, jl_value_t *edge) +JL_DLLEXPORT int jl_maybe_add_binding_backedge(jl_binding_t *b, jl_value_t *edge, jl_method_t *for_method) { if (!edge) - return; - jl_binding_t *b = gr->binding; - if (!b) - b = jl_get_module_binding(gr->mod, gr->name, 1); + return 0; + jl_module_t *defining_module = for_method->module; // N.B.: The logic for evaluating whether a backedge is required must // match the invalidation logic. - if (gr->mod == defining_module) { + if (b->globalref->mod == defining_module) { // No backedge required - invalidation will forward scan jl_atomic_fetch_or(&b->flags, BINDING_FLAG_ANY_IMPLICIT_EDGES); - return; + if (!(jl_atomic_fetch_or(&for_method->did_scan_source, 0x2) & 0x2)) + jl_add_scanned_method(for_method->module, for_method); + return 1; } - jl_add_binding_backedge(b, edge); + jl_add_binding_backedge(b, (jl_value_t*)edge); + return 0; } JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked(jl_binding_t *b, diff --git a/src/staticdata.c b/src/staticdata.c index 9b9c6a81c8099..0868fb9b3a58e 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -812,6 +812,7 @@ static void jl_queue_module_for_serialization(jl_serializer_state *s, jl_module_ } jl_queue_for_serialization(s, m->usings_backedges); + jl_queue_for_serialization(s, m->scanned_methods); } // Anything that requires uniquing or fixing during deserialization needs to be "toplevel" @@ -1324,6 +1325,9 @@ static void jl_write_module(jl_serializer_state *s, uintptr_t item, jl_module_t newm->usings_backedges = NULL; arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, usings_backedges))); arraylist_push(&s->relocs_list, (void*)backref_id(s, m->usings_backedges, s->link_ids_relocs)); + newm->scanned_methods = NULL; + arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, scanned_methods))); + arraylist_push(&s->relocs_list, (void*)backref_id(s, m->scanned_methods, s->link_ids_relocs)); // After reload, everything that has happened in this process happened semantically at // (for .incremental) or before jl_require_world, so reset this flag.