Skip to content

Commit

Permalink
staticdata: corrected implementation of jl_collect_new_roots (#57407)
Browse files Browse the repository at this point in the history 
This seems basically the same as #57212, but from computing and using
`method_roots_list` wrong instead here (specifically the recursively
part). Fix it by integrating `jl_collect_new_roots` with the places we
actually need it.

Fixes #56994
  • Loading branch information
@vtjnash
vtjnash authored Feb 14, 2025
1 parent be574cd commit f5f6d41
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 71 deletions.
95 changes: 69 additions & 26 deletions src/staticdata.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,9 @@ typedef struct {
jl_array_t *link_ids_gctags;
jl_array_t *link_ids_gvars;
jl_array_t *link_ids_external_fnvars;
jl_array_t *method_roots_list;
htable_t method_roots_index;
uint64_t worklist_key;
jl_ptls_t ptls;
jl_image_t *image;
int8_t incremental;
Expand Down Expand Up @@ -931,22 +934,57 @@ static void jl_insert_into_serialization_queue(jl_serializer_state *s, jl_value_
int is_relocatable = jl_is_code_info(inferred) ||
(jl_is_string(inferred) && jl_string_len(inferred) > 0 && jl_string_data(inferred)[jl_string_len(inferred) - 1]);
if (!is_relocatable) {
record_field_change((jl_value_t**)&ci->inferred, jl_nothing);
inferred = jl_nothing;
}
else if (def->source == NULL) {
// don't delete code from optimized opaque closures that can't be reconstructed (and builtins)
}
else if (jl_atomic_load_relaxed(&ci->max_world) != ~(size_t)0 || // delete all code that cannot run
jl_atomic_load_relaxed(&ci->invoke) == jl_fptr_const_return) { // delete all code that just returns a constant
record_field_change((jl_value_t**)&ci->inferred, jl_nothing);
inferred = jl_nothing;
}
else if (native_functions && // don't delete any code if making a ji file
(ci->owner == jl_nothing) && // don't delete code for external interpreters
!effects_foldable(jl_atomic_load_relaxed(&ci->ipo_purity_bits)) && // don't delete code we may want for irinterp
jl_ir_inlining_cost(inferred) == UINT16_MAX) { // don't delete inlineable code
// delete the code now: if we thought it was worth keeping, it would have been converted to object code
inferred = jl_nothing;
}
if (inferred == jl_nothing) {
record_field_change((jl_value_t**)&ci->inferred, jl_nothing);
}
else if (jl_is_string(inferred)) {
// New roots for external methods
if (jl_object_in_image((jl_value_t*)def)) {
void **pfound = ptrhash_bp(&s->method_roots_index, def);
if (*pfound == HT_NOTFOUND) {
*pfound = def;
size_t nwithkey = nroots_with_key(def, s->worklist_key);
if (nwithkey) {
jl_array_ptr_1d_push(s->method_roots_list, (jl_value_t*)def);
jl_array_t *newroots = jl_alloc_vec_any(nwithkey);
jl_array_ptr_1d_push(s->method_roots_list, (jl_value_t*)newroots);
rle_iter_state rootiter = rle_iter_init(0);
uint64_t *rletable = NULL;
size_t nblocks2 = 0;
size_t nroots = jl_array_nrows(def->roots);
size_t k = 0;
if (def->root_blocks) {
rletable = jl_array_data(def->root_blocks, uint64_t);
nblocks2 = jl_array_nrows(def->root_blocks);
}
while (rle_iter_increment(&rootiter, nroots, rletable, nblocks2)) {
if (rootiter.key == s->worklist_key) {
jl_value_t *newroot = jl_array_ptr_ref(def->roots, rootiter.i);
jl_queue_for_serialization(s, newroot);
jl_array_ptr_set(newroots, k++, newroot);
}
}
assert(k == nwithkey);
}
}
}
}
}
}
}
Expand Down Expand Up @@ -2861,10 +2899,9 @@ JL_DLLEXPORT jl_value_t *jl_as_global_root(jl_value_t *val, int insert)
return val;
}

static void jl_prepare_serialization_data(jl_array_t *mod_array, jl_array_t *newly_inferred, uint64_t worklist_key,
static void jl_prepare_serialization_data(jl_array_t *mod_array, jl_array_t *newly_inferred,
/* outputs */ jl_array_t **extext_methods JL_REQUIRE_ROOTED_SLOT,
jl_array_t **new_ext_cis JL_REQUIRE_ROOTED_SLOT,
jl_array_t **method_roots_list JL_REQUIRE_ROOTED_SLOT,
jl_array_t **edges JL_REQUIRE_ROOTED_SLOT)
{
// extext_methods: [method1, ...], worklist-owned "extending external" methods added to functions owned by modules outside the worklist
Expand All @@ -2888,24 +2925,20 @@ static void jl_prepare_serialization_data(jl_array_t *mod_array, jl_array_t *new
}

if (edges) {
// Extract `edges` now (from info prepared by jl_collect_methcache_from_mod)
size_t world = jl_atomic_load_acquire(&jl_world_counter);
// Extract `new_ext_cis` and `edges` now (from info prepared by jl_collect_methcache_from_mod)
*method_roots_list = jl_alloc_vec_any(0);
// Collect the new method roots for external specializations
jl_collect_new_roots(*method_roots_list, *new_ext_cis, worklist_key);
*edges = jl_alloc_vec_any(0);
jl_collect_internal_cis(*edges, world);
}
internal_methods = NULL;
internal_methods = NULL; // global

JL_GC_POP();
}

// In addition to the system image (where `worklist = NULL`), this can also save incremental images with external linkage
static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
jl_array_t *worklist, jl_array_t *extext_methods,
jl_array_t *new_ext_cis, jl_array_t *method_roots_list,
jl_array_t *edges) JL_GC_DISABLED
jl_array_t *new_ext_cis, jl_array_t *edges)
{
htable_new(&field_replace, 0);
htable_new(&bits_replace, 0);
Expand Down Expand Up @@ -3035,9 +3068,14 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
s.link_ids_gctags = jl_alloc_array_1d(jl_array_int32_type, 0);
s.link_ids_gvars = jl_alloc_array_1d(jl_array_int32_type, 0);
s.link_ids_external_fnvars = jl_alloc_array_1d(jl_array_int32_type, 0);
s.method_roots_list = NULL;
htable_new(&s.method_roots_index, 0);
if (worklist) {
s.method_roots_list = jl_alloc_vec_any(0);
s.worklist_key = jl_worklist_key(worklist);
}
jl_value_t **const*const tags = get_tags(); // worklist == NULL ? get_tags() : NULL;


if (worklist == NULL) {
// empty!(Core.ARGS)
if (jl_core_module != NULL) {
Expand Down Expand Up @@ -3082,8 +3120,6 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
jl_queue_for_serialization(&s, extext_methods);
// Queue the new specializations
jl_queue_for_serialization(&s, new_ext_cis);
// Queue the new roots
jl_queue_for_serialization(&s, method_roots_list);
// Queue the edges
jl_queue_for_serialization(&s, edges);
}
Expand All @@ -3094,7 +3130,15 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
if (jl_options.trim)
record_gvars(&s, &MIs);
jl_serialize_reachable(&s);
// step 1.3: prune (garbage collect) special weak references from the jl_global_roots_list
// Beyond this point, all content should already have been visited, so now we can prune
// the rest and add some internal root arrays.
// step 1.3: include some other special roots
if (s.incremental) {
// Queue the new roots array
jl_queue_for_serialization(&s, s.method_roots_list);
jl_serialize_reachable(&s);
}
// step 1.4: prune (garbage collect) special weak references from the jl_global_roots_list
if (worklist == NULL) {
global_roots_list = jl_alloc_memory_any(0);
global_roots_keyset = jl_alloc_memory_any(0);
Expand All @@ -3110,7 +3154,7 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
jl_queue_for_serialization(&s, global_roots_keyset);
jl_serialize_reachable(&s);
}
// step 1.4: prune (garbage collect) some special weak references from
// step 1.5: prune (garbage collect) some special weak references from
// built-in type caches too
for (i = 0; i < serialization_queue.len; i++) {
jl_value_t *v = (jl_value_t*)serialization_queue.items[i];
Expand All @@ -3130,7 +3174,8 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
if (ptrhash_get(&serialization_order, mi) == HT_NOTFOUND)
jl_svecset(specializations, i, jl_nothing);
}
} else if (jl_is_module(v)) {
}
else if (jl_is_module(v)) {
jl_prune_module_bindings((jl_module_t*)v);
}
}
Expand Down Expand Up @@ -3262,7 +3307,7 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
jl_write_value(&s, jl_module_init_order);
jl_write_value(&s, extext_methods);
jl_write_value(&s, new_ext_cis);
jl_write_value(&s, method_roots_list);
jl_write_value(&s, s.method_roots_list);
jl_write_value(&s, edges);
}
write_uint32(f, jl_array_len(s.link_ids_gctags));
Expand Down Expand Up @@ -3293,6 +3338,7 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
arraylist_free(&s.gctags_list);
arraylist_free(&gvars);
arraylist_free(&external_fns);
htable_free(&s.method_roots_index);
htable_free(&field_replace);
htable_free(&bits_replace);
htable_free(&serialization_order);
Expand Down Expand Up @@ -3353,18 +3399,17 @@ JL_DLLEXPORT void jl_create_system_image(void **_native_data, jl_array_t *workli
}

jl_array_t *mod_array = NULL, *extext_methods = NULL, *new_ext_cis = NULL;
jl_array_t *method_roots_list = NULL, *edges = NULL;
jl_array_t *edges = NULL;
int64_t checksumpos = 0;
int64_t checksumpos_ff = 0;
int64_t datastartpos = 0;
JL_GC_PUSH5(&mod_array, &extext_methods, &new_ext_cis, &method_roots_list, &edges);
JL_GC_PUSH4(&mod_array, &extext_methods, &new_ext_cis, &edges);

if (worklist) {
mod_array = jl_get_loaded_modules(); // __toplevel__ modules loaded in this session (from Base.loaded_modules_array)
// Generate _native_data`
if (_native_data != NULL) {
jl_prepare_serialization_data(mod_array, newly_inferred, jl_worklist_key(worklist),
&extext_methods, &new_ext_cis, NULL, NULL);
jl_prepare_serialization_data(mod_array, newly_inferred, &extext_methods, &new_ext_cis, NULL);
jl_precompile_toplevel_module = (jl_module_t*)jl_array_ptr_ref(worklist, jl_array_len(worklist)-1);
*_native_data = jl_precompile_worklist(worklist, extext_methods, new_ext_cis);
jl_precompile_toplevel_module = NULL;
Expand Down Expand Up @@ -3395,8 +3440,7 @@ JL_DLLEXPORT void jl_create_system_image(void **_native_data, jl_array_t *workli
assert((ct->reentrant_timing & 0b1110) == 0);
ct->reentrant_timing |= 0b1000;
if (worklist) {
jl_prepare_serialization_data(mod_array, newly_inferred, jl_worklist_key(worklist),
&extext_methods, &new_ext_cis, &method_roots_list, &edges);
jl_prepare_serialization_data(mod_array, newly_inferred, &extext_methods, &new_ext_cis, &edges);
if (!emit_split) {
write_int32(f, 0); // No clone_targets
write_padding(f, LLT_ALIGN(ios_pos(f), JL_CACHE_BYTE_ALIGNMENT) - ios_pos(f));
Expand All @@ -3408,7 +3452,7 @@ JL_DLLEXPORT void jl_create_system_image(void **_native_data, jl_array_t *workli
}
if (_native_data != NULL)
native_functions = *_native_data;
jl_save_system_image_to_stream(ff, mod_array, worklist, extext_methods, new_ext_cis, method_roots_list, edges);
jl_save_system_image_to_stream(ff, mod_array, worklist, extext_methods, new_ext_cis, edges);
if (_native_data != NULL)
native_functions = NULL;
// make sure we don't run any Julia code concurrently before this point
Expand Down Expand Up @@ -4185,7 +4229,6 @@ static jl_value_t *jl_restore_package_image_from_stream(void* pkgimage_handle, i
&edges, &base, &ccallable_list, &cachesizes);
JL_SIGATOMIC_END();

// No special processing of `new_ext_cis` is required because recaching handled it
// Add roots to methods
jl_copy_roots(method_roots_list, jl_worklist_key((jl_array_t*)restored));
// Insert method extensions and handle edges
Expand Down
45 changes: 0 additions & 45 deletions src/staticdata_utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,51 +261,6 @@ static jl_array_t *queue_external_cis(jl_array_t *list)
return new_ext_cis;
}

// New roots for external methods
static void jl_collect_new_roots(jl_array_t *roots, jl_array_t *new_ext_cis, uint64_t key)
{
htable_t mset;
htable_new(&mset, 0);
size_t l = new_ext_cis ? jl_array_nrows(new_ext_cis) : 0;
for (size_t i = 0; i < l; i++) {
jl_code_instance_t *ci = (jl_code_instance_t*)jl_array_ptr_ref(new_ext_cis, i);
assert(jl_is_code_instance(ci));
jl_method_t *m = jl_get_ci_mi(ci)->def.method;
assert(jl_is_method(m));
ptrhash_put(&mset, (void*)m, (void*)m);
}
int nwithkey;
void *const *table = mset.table;
jl_array_t *newroots = NULL;
JL_GC_PUSH1(&newroots);
for (size_t i = 0; i < mset.size; i += 2) {
if (table[i+1] != HT_NOTFOUND) {
jl_method_t *m = (jl_method_t*)table[i];
assert(jl_is_method(m));
nwithkey = nroots_with_key(m, key);
if (nwithkey) {
jl_array_ptr_1d_push(roots, (jl_value_t*)m);
newroots = jl_alloc_vec_any(nwithkey);
jl_array_ptr_1d_push(roots, (jl_value_t*)newroots);
rle_iter_state rootiter = rle_iter_init(0);
uint64_t *rletable = NULL;
size_t nblocks2 = 0, nroots = jl_array_nrows(m->roots), k = 0;
if (m->root_blocks) {
rletable = jl_array_data(m->root_blocks, uint64_t);
nblocks2 = jl_array_nrows(m->root_blocks);
}
while (rle_iter_increment(&rootiter, nroots, rletable, nblocks2))
if (rootiter.key == key)
jl_array_ptr_set(newroots, k++, jl_array_ptr_ref(m->roots, rootiter.i));
assert(k == nwithkey);
}
}
}
JL_GC_POP();
htable_free(&mset);
}


// For every method:
// - if the method is owned by a worklist module, add it to the list of things to be
// verified on reloading
Expand Down

0 comments on commit f5f6d41

Please sign in to comment.