Skip to content

Commit 0bfd815

Browse files
committed
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).
1 parent 854ceb5 commit 0bfd815

File tree

3 files changed

+97
-37
lines changed

3 files changed

+97
-37
lines changed

Compiler/src/abstractinterpretation.jl

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3633,18 +3633,20 @@ scan_partitions(query::Function, interp, g::GlobalRef, wwr::WorldWithRange) =
36333633
abstract_load_all_consistent_leaf_partitions(interp, g::GlobalRef, wwr::WorldWithRange) =
36343634
scan_leaf_partitions(abstract_eval_partition_load, interp, g, wwr)
36353635

3636+
function abstract_eval_globalref_partition(interp, binding::Core.Binding, partition::Core.BindingPartition)
3637+
# For inference purposes, we don't particularly care which global binding we end up loading, we only
3638+
# care about its type. However, we would still like to terminate the world range for the particular
3639+
# binding we end up reaching such that codegen can emit a simpler pointer load.
3640+
Pair{RTEffects, Union{Nothing, Core.Binding}}(
3641+
abstract_eval_partition_load(interp, partition),
3642+
binding_kind(partition) in (PARTITION_KIND_GLOBAL, PARTITION_KIND_DECLARED) ? binding : nothing)
3643+
end
3644+
36363645
function abstract_eval_globalref(interp, g::GlobalRef, saw_latestworld::Bool, sv::AbsIntState)
36373646
if saw_latestworld
36383647
return RTEffects(Any, Any, generic_getglobal_effects)
36393648
end
3640-
(valid_worlds, (ret, binding_if_global)) = scan_leaf_partitions(interp, g, sv.world) do interp, binding, partition
3641-
# For inference purposes, we don't particularly care which global binding we end up loading, we only
3642-
# care about its type. However, we would still like to terminate the world range for the particular
3643-
# binding we end up reaching such that codegen can emit a simpler pointer load.
3644-
Pair{RTEffects, Union{Nothing, Core.Binding}}(
3645-
abstract_eval_partition_load(interp, partition),
3646-
binding_kind(partition) in (PARTITION_KIND_GLOBAL, PARTITION_KIND_DECLARED) ? binding : nothing)
3647-
end
3649+
(valid_worlds, (ret, binding_if_global)) = scan_leaf_partitions(abstract_eval_globalref_partition, interp, g, sv.world)
36483650
update_valid_age!(sv, valid_worlds)
36493651
if ret.rt !== Union{} && ret.exct === UndefVarError && binding_if_global !== nothing && InferenceParams(interp).assume_bindings_static
36503652
if isdefined(binding_if_global, :value)

base/invalidation.jl

Lines changed: 49 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -113,33 +113,55 @@ function invalidate_method_for_globalref!(gr::GlobalRef, method::Method, invalid
113113
end
114114
end
115115

116-
function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core.BindingPartition, new_bpart::Union{Core.BindingPartition, Nothing}, new_max_world::UInt)
116+
export_affecting_partition_flags(bpart::Core.BindingPartition) =
117+
((bpart.kind & PARTITION_MASK_KIND) == PARTITION_KIND_GUARD,
118+
(bpart.kind & PARTITION_FLAG_EXPORTED) != 0,
119+
(bpart.kind & PARTITION_FLAG_DEPRECATED) != 0)
120+
121+
function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core.BindingPartition, new_bpart::Core.BindingPartition, new_max_world::UInt)
117122
gr = b.globalref
118-
if (b.flags & BINDING_FLAG_ANY_IMPLICIT_EDGES) != 0
119-
foreach_module_mtable(gr.mod, new_max_world) do mt::Core.MethodTable
120-
for method in MethodList(mt)
121-
invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world)
123+
124+
(_, (ib, ibpart)) = Compiler.walk_binding_partition(b, invalidated_bpart, new_max_world)
125+
(_, (nb, nbpart)) = Compiler.walk_binding_partition(b, new_bpart, new_max_world+1)
126+
127+
# abstract_eval_globalref_partition is the maximum amount of information that inference
128+
# reads from a binding partition. If this information does not change - we do not need to
129+
# invalidate any code that inference created, because we know that the result will not change.
130+
need_to_invalidate_code =
131+
Compiler.abstract_eval_globalref_partition(nothing, ib, ibpart) !==
132+
Compiler.abstract_eval_globalref_partition(nothing, nb, nbpart)
133+
134+
need_to_invalidate_export = export_affecting_partition_flags(invalidated_bpart) !==
135+
export_affecting_partition_flags(new_bpart)
136+
137+
if need_to_invalidate_code
138+
if (b.flags & BINDING_FLAG_ANY_IMPLICIT_EDGES) != 0
139+
foreach_module_mtable(gr.mod, new_max_world) do mt::Core.MethodTable
140+
for method in MethodList(mt)
141+
invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world)
142+
end
143+
return true
122144
end
123-
return true
124145
end
125-
end
126-
if isdefined(b, :backedges)
127-
for edge in b.backedges
128-
if isa(edge, CodeInstance)
129-
ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), edge, new_max_world)
130-
elseif isa(edge, Core.Binding)
131-
isdefined(edge, :partitions) || continue
132-
latest_bpart = edge.partitions
133-
latest_bpart.max_world == typemax(UInt) || continue
134-
is_some_imported(binding_kind(latest_bpart)) || continue
135-
partition_restriction(latest_bpart) === b || continue
136-
invalidate_code_for_globalref!(edge, latest_bpart, nothing, new_max_world)
137-
else
138-
invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world)
146+
if isdefined(b, :backedges)
147+
for edge in b.backedges
148+
if isa(edge, CodeInstance)
149+
ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), edge, new_max_world)
150+
elseif isa(edge, Core.Binding)
151+
isdefined(edge, :partitions) || continue
152+
latest_bpart = edge.partitions
153+
latest_bpart.max_world == typemax(UInt) || continue
154+
is_some_imported(binding_kind(latest_bpart)) || continue
155+
partition_restriction(latest_bpart) === b || continue
156+
invalidate_code_for_globalref!(edge, latest_bpart, latest_bpart, new_max_world)
157+
else
158+
invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world)
159+
end
139160
end
140161
end
141162
end
142-
if (invalidated_bpart.kind & PARTITION_FLAG_EXPORTED != 0) || (new_bpart !== nothing && (new_bpart.kind & PARTITION_FLAG_EXPORTED != 0))
163+
164+
if need_to_invalidate_code || need_to_invalidate_export
143165
# This binding was exported - we need to check all modules that `using` us to see if they
144166
# have a binding that is affected by this change.
145167
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
151173
latest_bpart = user_binding.partitions
152174
latest_bpart.max_world == typemax(UInt) || continue
153175
binding_kind(latest_bpart) in (PARTITION_KIND_IMPLICIT, PARTITION_KIND_FAILED, PARTITION_KIND_GUARD) || continue
154-
@atomic :release latest_bpart.max_world = new_max_world
155-
invalidate_code_for_globalref!(convert(Core.Binding, user_binding), latest_bpart, nothing, new_max_world)
176+
new_bpart = need_to_invalidate_export ?
177+
ccall(:jl_maybe_reresolve_implicit, Any, (Any, Any, Csize_t), user_binding, latest_bpart, new_max_world) :
178+
latest_bpart
179+
if need_to_invalidate_code || new_bpart !== latest_bpart
180+
invalidate_code_for_globalref!(convert(Core.Binding, user_binding), latest_bpart, new_bpart, new_max_world)
181+
end
156182
end
157183
end
158184
end

src/module.c

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ void jl_check_new_binding_implicit(
106106
if (tempbmax_world < max_world)
107107
max_world = tempbmax_world;
108108

109+
// N.B.: Which aspects of the partition are considered here needs to
110+
// be kept in sync with `export_affecting_partition_flags` in the
111+
// invalidation code.
109112
if ((tempbpart->kind & PARTITION_FLAG_EXPORTED) == 0)
110113
continue;
111114

@@ -165,6 +168,29 @@ void jl_check_new_binding_implicit(
165168
return;
166169
}
167170

171+
JL_DLLEXPORT jl_binding_partition_t *jl_maybe_reresolve_implicit(jl_binding_t *b, size_t new_max_world)
172+
{
173+
jl_binding_partition_t *new_bpart = new_binding_partition();
174+
jl_binding_partition_t *bpart = jl_atomic_load_acquire(&b->partitions);
175+
assert(bpart);
176+
while (1) {
177+
jl_atomic_store_relaxed(&new_bpart->next, bpart);
178+
jl_gc_wb(new_bpart, bpart);
179+
jl_check_new_binding_implicit(new_bpart, b, NULL, new_max_world+1);
180+
JL_GC_PROMISE_ROOTED(new_bpart); // TODO: Analyzer doesn't understand MAYBE_UNROOTED properly
181+
if (bpart->kind & PARTITION_FLAG_EXPORTED)
182+
new_bpart->kind |= PARTITION_FLAG_EXPORTED;
183+
if (new_bpart->kind == bpart->kind && new_bpart->restriction == bpart->restriction)
184+
return bpart;
185+
// Resolution changed, insert the new partition
186+
size_t expected_max_world = ~(size_t)0;
187+
if (jl_atomic_cmpswap(&bpart->max_world, &expected_max_world, new_max_world) &&
188+
jl_atomic_cmpswap(&b->partitions, &bpart, new_bpart))
189+
break;
190+
}
191+
return new_bpart;
192+
}
193+
168194
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
169195
{
170196
assert(jl_is_binding(b));
@@ -183,7 +209,7 @@ STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_(jl_binding_t *b
183209
if (!new_bpart)
184210
new_bpart = new_binding_partition();
185211
jl_atomic_store_relaxed(&new_bpart->next, bpart);
186-
jl_gc_wb_fresh(new_bpart, bpart);
212+
jl_gc_wb(new_bpart, bpart); // Not fresh the second time around the loop
187213
new_bpart->min_world = bpart ? jl_atomic_load_relaxed(&bpart->max_world) + 1 : 0;
188214
jl_atomic_store_relaxed(&new_bpart->max_world, max_world);
189215
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)
11121138
jl_sym_t *var = b->globalref->name;
11131139
jl_binding_t *tob = jl_get_module_binding(to, var, 0);
11141140
if (tob) {
1115-
jl_binding_partition_t *tobpart = jl_get_binding_partition(tob, new_world);
1116-
enum jl_partition_kind kind = jl_binding_kind(tobpart);
1117-
if (jl_bkind_is_some_implicit(kind)) {
1118-
jl_replace_binding_locked(tob, tobpart, NULL, PARTITION_KIND_IMPLICIT_RECOMPUTE, new_world);
1141+
jl_binding_partition_t *tobpart = jl_atomic_load_relaxed(&tob->partitions);
1142+
if (tobpart) {
1143+
enum jl_partition_kind kind = jl_binding_kind(tobpart);
1144+
if (jl_bkind_is_some_implicit(kind)) {
1145+
jl_replace_binding_locked(tob, tobpart, NULL, PARTITION_KIND_IMPLICIT_RECOMPUTE, new_world);
1146+
}
11191147
}
11201148
}
11211149
}
@@ -1380,20 +1408,24 @@ JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b,
13801408
jl_atomic_store_relaxed(&jl_first_image_replacement_world, new_world);
13811409

13821410
assert(jl_atomic_load_relaxed(&b->partitions) == old_bpart);
1383-
jl_atomic_store_release(&old_bpart->max_world, new_world-1);
13841411
jl_binding_partition_t *new_bpart = new_binding_partition();
13851412
JL_GC_PUSH1(&new_bpart);
13861413
new_bpart->min_world = new_world;
13871414
if ((kind & PARTITION_MASK_KIND) == PARTITION_KIND_IMPLICIT_RECOMPUTE) {
13881415
assert(!restriction_val);
13891416
jl_check_new_binding_implicit(new_bpart /* callee rooted */, b, NULL, new_world);
13901417
new_bpart->kind |= kind & PARTITION_MASK_FLAG;
1418+
if (new_bpart->kind == old_bpart->kind && new_bpart->restriction == old_bpart->restriction) {
1419+
JL_GC_POP();
1420+
return old_bpart;
1421+
}
13911422
}
13921423
else {
13931424
new_bpart->kind = kind;
13941425
new_bpart->restriction = restriction_val;
13951426
jl_gc_wb_fresh(new_bpart, restriction_val);
13961427
}
1428+
jl_atomic_store_release(&old_bpart->max_world, new_world-1);
13971429
jl_atomic_store_relaxed(&new_bpart->next, old_bpart);
13981430
jl_gc_wb_fresh(new_bpart, old_bpart);
13991431

0 commit comments

Comments
 (0)