Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: Prepare usings array for world age partition #55357

Merged
merged 1 commit into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/gc-debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,8 @@ int gc_slot_to_arrayidx(void *obj, void *_slot) JL_NOTSAFEPOINT
if (vt == jl_module_type) {
jl_module_t *m = (jl_module_t*)obj;
start = (char*)m->usings.items;
len = m->usings.len;
len = module_usings_length(m);
elsize = sizeof(struct _jl_module_using);
}
else if (vt == jl_simplevector_type) {
start = (char*)jl_svec_data(obj);
Expand Down
12 changes: 7 additions & 5 deletions src/gc-stock.c
Original file line number Diff line number Diff line change
Expand Up @@ -2053,16 +2053,18 @@ STATIC_INLINE void gc_mark_module_binding(jl_ptls_t ptls, jl_module_t *mb_parent
gc_heap_snapshot_record_module_to_binding(mb_parent, bindings, bindingkeyset);
gc_assert_parent_validity((jl_value_t *)mb_parent, (jl_value_t *)mb_parent->parent);
gc_try_claim_and_push(mq, (jl_value_t *)mb_parent->parent, &nptr);
size_t nusings = mb_parent->usings.len;
size_t nusings = module_usings_length(mb_parent);
if (nusings > 0) {
// this is only necessary because bindings for "using" modules
// are added only when accessed. therefore if a module is replaced
// after "using" it but before accessing it, this array might
// contain the only reference.
jl_value_t *obj_parent = (jl_value_t *)mb_parent;
jl_value_t **objary_begin = (jl_value_t **)mb_parent->usings.items;
jl_value_t **objary_end = objary_begin + nusings;
gc_mark_objarray(ptls, obj_parent, objary_begin, objary_end, 1, nptr);
struct _jl_module_using *objary_begin = (struct _jl_module_using *)mb_parent->usings.items;
struct _jl_module_using *objary_end = objary_begin + nusings;
static_assert(sizeof(struct _jl_module_using) == 3*sizeof(void *), "Mismatch in _jl_module_using size");
static_assert(offsetof(struct _jl_module_using, mod) == 0, "Expected `mod` at the beginning of _jl_module_using");
gc_mark_objarray(ptls, obj_parent, (jl_value_t**)objary_begin, (jl_value_t**)objary_end, 3, nptr);
}
else {
gc_mark_push_remset(ptls, (jl_value_t *)mb_parent, nptr);
Expand Down Expand Up @@ -2175,7 +2177,7 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_
if (update_meta)
gc_setmark(ptls, o, bits, sizeof(jl_module_t));
jl_module_t *mb_parent = (jl_module_t *)new_obj;
uintptr_t nptr = ((mb_parent->usings.len + 1) << 2) | (bits & GC_OLD);
uintptr_t nptr = ((module_usings_length(mb_parent) + 1) << 2) | (bits & GC_OLD);
gc_mark_module_binding(ptls, mb_parent, nptr, bits);
}
else if (vtag == jl_task_tag << 4) {
Expand Down
8 changes: 7 additions & 1 deletion src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ typedef struct _jl_module_t {
jl_sym_t *file;
int32_t line;
// hidden fields:
arraylist_t usings; // modules with all bindings potentially imported
arraylist_t usings; /* arraylist of struct jl_module_using */ // modules with all bindings potentially imported
jl_uuid_t build_id;
jl_uuid_t uuid;
_Atomic(uint32_t) counter;
Expand All @@ -728,6 +728,12 @@ typedef struct _jl_module_t {
intptr_t hash;
} jl_module_t;

struct _jl_module_using {
jl_module_t *mod;
size_t min_world;
size_t max_world;
};

struct _jl_globalref_t {
JL_DATA_TYPE
jl_module_t *mod;
Expand Down
22 changes: 22 additions & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,28 @@ void jl_eval_global_expr(jl_module_t *m, jl_expr_t *ex, int set_type);
JL_DLLEXPORT void jl_declare_global(jl_module_t *m, jl_value_t *arg, jl_value_t *set_type);
JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int expanded, const char **toplevel_filename, int *toplevel_lineno);

STATIC_INLINE struct _jl_module_using *module_usings_getidx(jl_module_t *m JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT;
STATIC_INLINE jl_module_t *module_usings_getmod(jl_module_t *m JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT;

#ifndef __clang_gcanalyzer__
// The analyzer doesn't like looking through the arraylist, so just model the
// access for it using this function
STATIC_INLINE struct _jl_module_using *module_usings_getidx(jl_module_t *m JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT {
return (struct _jl_module_using *)&(m->usings.items[3*i]);
}
STATIC_INLINE jl_module_t *module_usings_getmod(jl_module_t *m JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT {
return module_usings_getidx(m, i)->mod;
}
#endif

STATIC_INLINE size_t module_usings_length(jl_module_t *m) JL_NOTSAFEPOINT {
return m->usings.len/3;
}

STATIC_INLINE size_t module_usings_max(jl_module_t *m) JL_NOTSAFEPOINT {
return m->usings.max/3;
}

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);
Expand Down
37 changes: 15 additions & 22 deletions src/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -373,16 +373,6 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_
return b;
}

static inline jl_module_t *module_usings_getidx(jl_module_t *m JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT;

#ifndef __clang_gcanalyzer__
// The analyzer doesn't like looking through the arraylist, so just model the
// access for it using this function
static inline jl_module_t *module_usings_getidx(jl_module_t *m JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT {
return (jl_module_t*)m->usings.items[i];
}
#endif

static int eq_bindings(jl_binding_partition_t *owner, jl_binding_t *alias, size_t world)
{
jl_ptr_kind_union_t owner_pku = jl_atomic_load_relaxed(&owner->restriction);
Expand All @@ -407,11 +397,11 @@ static jl_binding_t *using_resolve_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl
jl_binding_partition_t *bpart = NULL;
jl_module_t *owner = NULL;
JL_LOCK(&m->lock);
int i = (int)m->usings.len - 1;
int i = (int)module_usings_length(m) - 1;
JL_UNLOCK(&m->lock);
for (; i >= 0; --i) {
JL_LOCK(&m->lock);
jl_module_t *imp = module_usings_getidx(m, i);
jl_module_t *imp = module_usings_getmod(m, i);
JL_UNLOCK(&m->lock);
jl_binding_t *tempb = jl_get_module_binding(imp, var, 0);
if (tempb != NULL && tempb->exportp) {
Expand Down Expand Up @@ -746,19 +736,24 @@ JL_DLLEXPORT void jl_module_use_as(jl_module_t *to, jl_module_t *from, jl_sym_t
module_import_(to, from, asname, s, 0);
}


JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from)
{
if (to == from)
return;
JL_LOCK(&to->lock);
for (size_t i = 0; i < to->usings.len; i++) {
if (from == to->usings.items[i]) {
for (size_t i = 0; i < module_usings_length(to); i++) {
if (from == module_usings_getmod(to, i)) {
JL_UNLOCK(&to->lock);
return;
}
}
arraylist_push(&to->usings, from);
struct _jl_module_using new_item = {
.mod = from,
.min_world = 0,
.max_world = (size_t)-1
};
arraylist_grow(&to->usings, sizeof(struct _jl_module_using)/sizeof(void*));
memcpy(&to->usings.items[to->usings.len-3], &new_item, sizeof(struct _jl_module_using));
Comment on lines +755 to +756
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, you don't always access this state with a lock (e.g. in jl_module_names), so this is a concurrency issue. The simplest fix is to make sure there add a lock (like using_resolve_binding does). The alternative fix is to use the similar functions in mtarraylist.c which should allow concurrent reading

jl_gc_wb(to, from);
JL_UNLOCK(&to->lock);

Expand Down Expand Up @@ -1096,12 +1091,12 @@ JL_DLLEXPORT jl_value_t *jl_checked_assignonce(jl_binding_t *b, jl_module_t *mod
JL_DLLEXPORT jl_value_t *jl_module_usings(jl_module_t *m)
{
JL_LOCK(&m->lock);
int j = m->usings.len;
int j = module_usings_length(m);
jl_array_t *a = jl_alloc_array_1d(jl_array_any_type, j);
JL_GC_PUSH1(&a);
for (int i = 0; j > 0; i++) {
j--;
jl_module_t *imp = (jl_module_t*)m->usings.items[i];
jl_module_t *imp = module_usings_getmod(m, i);
jl_array_ptr_set(a, j, (jl_value_t*)imp);
}
JL_UNLOCK(&m->lock); // may gc
Expand Down Expand Up @@ -1156,10 +1151,8 @@ JL_DLLEXPORT jl_value_t *jl_module_names(jl_module_t *m, int all, int imported,
if (usings) {
// If `usings` is specified, traverse the list of `using`-ed modules and incorporate
// the names exported by those modules into the list.
for(int i=(int)m->usings.len-1; i >= 0; --i) {
jl_module_t *usinged = module_usings_getidx(m, i);
append_exported_names(a, usinged, all);
}
for (int i = module_usings_length(m)-1; i >= 0; i--)
append_exported_names(a, module_usings_getmod(m, i), all);
}
JL_GC_POP();
return (jl_value_t*)a;
Expand Down
41 changes: 29 additions & 12 deletions src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -811,8 +811,8 @@ static void jl_queue_module_for_serialization(jl_serializer_state *s, jl_module_
}
}

for (size_t i = 0; i < m->usings.len; i++) {
jl_queue_for_serialization(s, (jl_value_t*)m->usings.items[i]);
for (size_t i = 0; i < module_usings_length(m); i++) {
jl_queue_for_serialization(s, module_usings_getmod(m, i));
}
}

Expand Down Expand Up @@ -1266,27 +1266,44 @@ static void jl_write_module(jl_serializer_state *s, uintptr_t item, jl_module_t
// write out the usings list
memset(&newm->usings._space, 0, sizeof(newm->usings._space));
if (m->usings.items == &m->usings._space[0]) {
newm->usings.items = (void**)offsetof(jl_module_t, usings._space);
// Push these relocations here, to keep them in order. This pairs with the `newm->usings.items = ` below.
arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, usings.items)));
arraylist_push(&s->relocs_list, (void*)(((uintptr_t)DataRef << RELOC_TAG_OFFSET) + item));
size_t i;
for (i = 0; i < m->usings.len; i++) {
arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, usings._space[i])));
arraylist_push(&s->relocs_list, (void*)backref_id(s, m->usings._space[i], s->link_ids_relocs));
for (i = 0; i < module_usings_length(m); i++) {
struct _jl_module_using *newm_data = module_usings_getidx(newm, i);
struct _jl_module_using *data = module_usings_getidx(m, i);
// TODO: Remove dead entries
newm_data->min_world = data->min_world;
newm_data->max_world = data->max_world;
if (s->incremental) {
if (data->max_world != (size_t)-1)
newm_data->max_world = 0;
newm_data->min_world = 0;
}
arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, usings._space[3*i])));
arraylist_push(&s->relocs_list, (void*)backref_id(s, data->mod, s->link_ids_relocs));
}
newm->usings.items = (void**)offsetof(jl_module_t, usings._space);
}
else {
newm->usings.items = (void**)tot;
arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, usings.items)));
arraylist_push(&s->relocs_list, (void*)(((uintptr_t)DataRef << RELOC_TAG_OFFSET) + item));
size_t i;
for (i = 0; i < m->usings.len; i++) {
write_pointerfield(s, (jl_value_t*)m->usings.items[i]);
tot += sizeof(void*);
}
for (; i < m->usings.max; i++) {
for (i = 0; i < module_usings_length(m); i++) {
struct _jl_module_using *data = module_usings_getidx(m, i);
write_pointerfield(s, (jl_value_t*)data->mod);
write_uint(s->s, data->min_world);
write_uint(s->s, data->max_world);
static_assert(sizeof(struct _jl_module_using) == 3*sizeof(void*), "_jl_module_using mismatch");
tot += sizeof(struct _jl_module_using);
}
for (; i < module_usings_max(m); i++) {
write_pointer(s->s);
tot += sizeof(void*);
write_uint(s->s, 0);
write_uint(s->s, 0);
tot += sizeof(struct _jl_module_using);
}
}
assert(ios_pos(s->s) - reloc_offset == tot);
Expand Down
2 changes: 1 addition & 1 deletion test/clangsa/MissingRoots.c
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ void scopes() {
jl_module_t *propagation(jl_module_t *m JL_PROPAGATES_ROOT);
void module_member(jl_module_t *m)
{
for(int i=(int)m->usings.len-1; i >= 0; --i) {
for(int i=(int)m->usings.len-1; i >= 0; i -= 3) {
jl_module_t *imp = propagation(m);
jl_gc_safepoint();
look_at_value((jl_value_t*)imp);
Expand Down