Merge adjacent implicit binding partitions

src/jltypes.c

@@ -3266,8 +3266,10 @@ void jl_init_types(void) JL_GC_DISABLED
                         jl_svec(5, jl_any_type,
                         jl_ulong_type, jl_ulong_type, jl_any_type/*jl_binding_partition_type*/, jl_ulong_type),
                         jl_emptysvec, 0, 1, 0);
-    const static uint32_t binding_partition_atomicfields[] = { 0b01101 }; // Set fields 1, 3, 4 as atomic
+    const static uint32_t binding_partition_atomicfields[] = { 0b01111 }; // Set fields 1, 2, 3, 4 as atomic
     jl_binding_partition_type->name->atomicfields = binding_partition_atomicfields;
+    const static uint32_t binding_partition_constfields[] = { 0x100001 }; // Set fields 1, 5 as constant
+    jl_binding_partition_type->name->constfields = binding_partition_constfields;
 
     jl_binding_type =
         jl_new_datatype(jl_symbol("Binding"), core, jl_any_type, jl_emptysvec,

src/julia.h

@@ -760,7 +760,7 @@ typedef struct JL_ALIGNED_ATTR(8) _jl_binding_partition_t {
      * } restriction;
      */
     jl_value_t *restriction;
-    size_t min_world;
+    _Atomic(size_t) min_world;
     _Atomic(size_t) max_world;
     _Atomic(struct _jl_binding_partition_t *) next;
     size_t kind;
@@ -1942,8 +1942,8 @@ JL_DLLEXPORT jl_sym_t *jl_get_root_symbol(void);
 JL_DLLEXPORT jl_value_t *jl_get_binding_value(jl_binding_t *b JL_PROPAGATES_ROOT);
 JL_DLLEXPORT jl_value_t *jl_get_binding_value_in_world(jl_binding_t *b JL_PROPAGATES_ROOT, size_t world);
 JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_const(jl_binding_t *b JL_PROPAGATES_ROOT);
-JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT;
-JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_and_const(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT;
+JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_debug_only(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT;
+JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_latest_resolved_and_const_debug_only(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT;
 JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_module_t *mod, jl_sym_t *name);
 JL_DLLEXPORT jl_method_t *jl_method_def(jl_svec_t *argdata, jl_methtable_t *mt, jl_code_info_t *f, jl_module_t *module);
 JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo, size_t world, jl_code_instance_t **cache);

src/rtutils.c

@@ -579,7 +579,7 @@ JL_DLLEXPORT jl_value_t *jl_stderr_obj(void) JL_NOTSAFEPOINT
     if (jl_base_module == NULL)
         return NULL;
     jl_binding_t *stderr_obj = jl_get_module_binding(jl_base_module, jl_symbol("stderr"), 0);
-    return stderr_obj ? jl_get_binding_value_if_resolved(stderr_obj) : NULL;
+    return stderr_obj ? jl_get_binding_value_if_resolved_debug_only(stderr_obj) : NULL;
 }
 
 // toys for debugging ---------------------------------------------------------
@@ -674,7 +674,7 @@ static int is_globname_binding(jl_value_t *v, jl_datatype_t *dv) JL_NOTSAFEPOINT
     jl_sym_t *globname = dv->name->mt != NULL ? dv->name->mt->name : NULL;
     if (globname && dv->name->module) {
         jl_binding_t *b = jl_get_module_binding(dv->name->module, globname, 0);
-        jl_value_t *bv = jl_get_binding_value_if_resolved_and_const(b);
+        jl_value_t *bv = jl_get_binding_value_if_latest_resolved_and_const_debug_only(b);
         // The `||` makes this function work for both function instances and function types.
         if (bv && (bv == v || jl_typeof(bv) == v))
             return 1;

src/staticdata.c

@@ -1749,7 +1749,7 @@ static void jl_write_values(jl_serializer_state *s) JL_GC_DISABLED
                     write_uint(f, 0);
                 }
             } else {
-                write_uint(f, bpart->min_world);
+                write_uint(f, jl_atomic_load_relaxed(&bpart->min_world));
                 write_uint(f, max_world);
             }
             write_pointerfield(s, (jl_value_t*)jl_atomic_load_relaxed(&bpart->next));
@@ -3618,7 +3618,7 @@ static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t
         // and allocate a fresh bpart?
         jl_update_loaded_bpart(b, bpart);
         bpart->kind |= (raw_kind & PARTITION_MASK_FLAG);
-        if (bpart->min_world > jl_require_world)
+        if (jl_atomic_load_relaxed(&bpart->min_world) > jl_require_world)
             goto invalidated;
     }
     if (!jl_bkind_is_some_explicit_import(kind) && kind != PARTITION_KIND_IMPLICIT_GLOBAL)
@@ -3629,7 +3629,8 @@ static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t
     jl_binding_partition_t *latest_imported_bpart = jl_atomic_load_relaxed(&imported_binding->partitions);
     if (!latest_imported_bpart)
         return 1;
-    if (latest_imported_bpart->min_world <= bpart->min_world) {
+    if (jl_atomic_load_relaxed(&latest_imported_bpart->min_world) <=
+        jl_atomic_load_relaxed(&bpart->min_world)) {
 add_backedge:
         // Imported binding is still valid
         if ((kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED) &&
@@ -3640,8 +3641,9 @@ static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t
     }
     else {
         // Binding partition was invalidated
-        assert(bpart->min_world == jl_require_world);
-        bpart->min_world = latest_imported_bpart->min_world;
+        assert(jl_atomic_load_relaxed(&bpart->min_world) == jl_require_world);
+        jl_atomic_store_relaxed(&bpart->min_world,
+            jl_atomic_load_relaxed(&latest_imported_bpart->min_world));
     }
 invalidated:
     // We need to go through and re-validate any bindings in the same image that

src/toplevel.c

@@ -309,7 +309,7 @@ void jl_declare_global(jl_module_t *m, jl_value_t *arg, jl_value_t *set_type, in
                     goto check_type;
                 }
                 check_safe_newbinding(gm, gs);
-                if (bpart->min_world == new_world) {
+                if (jl_atomic_load_relaxed(&bpart->min_world) == new_world) {
                     bpart->kind = new_kind | (bpart->kind & PARTITION_MASK_FLAG);
                     bpart->restriction = global_type;
                     if (global_type)
@@ -730,7 +730,7 @@ JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val2(
     JL_LOCK(&world_counter_lock);
     size_t new_world = jl_atomic_load_relaxed(&jl_world_counter) + 1;
     jl_binding_partition_t *bpart = jl_declare_constant_val3(b, mod, var, val, constant_kind, new_world);
-    if (bpart->min_world == new_world)
+    if (jl_atomic_load_relaxed(&bpart->min_world) == new_world)
         jl_atomic_store_release(&jl_world_counter, new_world);
     JL_UNLOCK(&world_counter_lock);
     return bpart;

test/rebinding.jl

@@ -318,3 +318,68 @@ module UndefinedTransitions
         @test Base.Compiler.is_nothrow(Base.Compiler.decode_effects(ci.ipo_purity_bits))
     end
 end
+
+# Identical implicit partitions should be merge (#57923)
+for binding in (convert(Core.Binding, GlobalRef(Base, :Math)),
+                convert(Core.Binding, GlobalRef(Base, :Intrinsics)))
+    # Test that these both only have two partitions
+    @test isdefined(binding, :partitions)
+    @test isdefined(binding.partitions, :next)
+    @test !isdefined(binding.partitions.next, :next)
+end
+
+# Test various scenarios for implicit partition merging
+module MergeStress
+    for i = 1:5
+        @eval module $(Symbol("M$i"))
+            export x, y
+            const x = 1
+            const y = 2
+        end
+    end
+    const before = Base.get_world_counter()
+    using .M1
+    const afterM1 = Base.get_world_counter()
+    using .M2
+    const afterM2 = Base.get_world_counter()
+    using .M3
+    const afterM3 = Base.get_world_counter()
+    using .M4
+    const afterM4 = Base.get_world_counter()
+    using .M5
+    const afterM5 = Base.get_world_counter()
+end
+
+function count_partitions(b::Core.Binding)
+    n = 0
+    isdefined(b, :partitions) || return n
+    bpart = b.partitions
+    while true
+        n += 1
+        isdefined(bpart, :next) || break
+        bpart = bpart.next
+    end
+    return n
+end
+using Base: invoke_in_world
+
+const xbinding = convert(Core.Binding, GlobalRef(MergeStress, :x))
+function access_and_count(point)
+    invoke_in_world(getglobal(MergeStress, point), getglobal, MergeStress, :x)
+    count_partitions(xbinding)
+end
+
+@test count_partitions(xbinding) == 0
+@test access_and_count(:afterM1) == 1
+# M2 is the first change to the `usings` table after M1. The partitions
+# can and should be merged
+@test access_and_count(:afterM2) == 1
+
+# There is a gap between M2 and M5 - the partitions should not be merged
+@test access_and_count(:afterM5) == 2
+
+# M4 and M5 are adjacent, these partitions should also be merged (in the opposite direction)
+@test access_and_count(:afterM4) == 2
+
+# M3 connects all, so we should have a single partition
+@test access_and_count(:afterM3) == 1