From 5dbfdc72e9bc1d03ffd1288265a6eb6be3ce5bb1 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Sun, 2 Nov 2025 23:01:37 +0000 Subject: [PATCH 1/2] Pin key/value in ptrhash_bp --- src/precompile_utils.c | 2 ++ src/staticdata.c | 10 +++++++++- src/staticdata_utils.c | 4 +++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/precompile_utils.c b/src/precompile_utils.c index a34a3838c6ddc..c85e94669c0d6 100644 --- a/src/precompile_utils.c +++ b/src/precompile_utils.c @@ -418,9 +418,11 @@ static void jl_rebuild_methtables(arraylist_t *MIs, htable_t *mtables) JL_GC_DIS jl_method_instance_t *mi = (jl_method_instance_t*)MIs->items[i]; jl_method_t *m = mi->def.method; // Check if the method is already in the new table, if not then insert it there + OBJHASH_PIN(m); void **inserted = ptrhash_bp(&ms, m); if (*inserted != HT_NOTFOUND) continue; + OBJHASH_PIN(m); *inserted = (void*)m; jl_methtable_t *old_mt = jl_method_get_table(m); if ((jl_value_t *)old_mt == jl_nothing) diff --git a/src/staticdata.c b/src/staticdata.c index eb4c17504ff57..f7aa273366167 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -680,7 +680,9 @@ static jl_value_t *get_replaceable_field(jl_value_t **addr, int mutabl) JL_GC_DI void **nullval = ptrhash_bp(&nullptrs, (void*)jl_typeof(fld)); if (*nullval == HT_NOTFOUND) { void *C_NULL = NULL; - *nullval = (void*)jl_new_bits(jl_typeof(fld), &C_NULL); + jl_value_t *new_fld = jl_new_bits(jl_typeof(fld), &C_NULL); + OBJHASH_PIN(new_fld); + *nullval = (void*)new_fld; } fld = (jl_value_t*)*nullval; } @@ -691,6 +693,7 @@ static jl_value_t *get_replaceable_field(jl_value_t **addr, int mutabl) JL_GC_DI static uintptr_t jl_fptr_id(void *fptr) { + PTRHASH_PIN(fptr); void **pbp = ptrhash_bp(&fptr_to_id, fptr); if (*pbp == HT_NOTFOUND || fptr == NULL) return 0; @@ -960,6 +963,7 @@ static void jl_insert_into_serialization_queue(jl_serializer_state *s, jl_value_ if (jl_object_in_image((jl_value_t*)def)) { void **pfound = ptrhash_bp(&s->method_roots_index, def); if (*pfound == HT_NOTFOUND) { + OBJHASH_PIN(def); *pfound = def; size_t nwithkey = nroots_with_key(def, s->worklist_key); if (nwithkey) { @@ -1087,6 +1091,7 @@ static void jl_insert_into_serialization_queue(jl_serializer_state *s, jl_value_ done_fields: ; // We've encountered an item we need to cache + OBJHASH_PIN(v); void **bp = ptrhash_bp(&serialization_order, v); assert(*bp == (void*)(uintptr_t)-2); arraylist_push(&serialization_queue, (void*) v); @@ -1148,6 +1153,7 @@ static void jl_queue_for_serialization_(jl_serializer_state *s, jl_value_t *v, i immediate = 1; } + OBJHASH_PIN(v); void **bp = ptrhash_bp(&serialization_order, v); assert(!immediate || *bp != (void*)(uintptr_t)-2); if (*bp == HT_NOTFOUND) @@ -1181,6 +1187,7 @@ static void jl_serialize_reachable(jl_serializer_state *s) JL_GC_DISABLED } prevlen = --object_worklist.len; jl_value_t *v = (jl_value_t*)object_worklist.items[prevlen]; + OBJHASH_PIN(v); void **bp = ptrhash_bp(&serialization_order, (void*)v); assert(*bp != HT_NOTFOUND && *bp != (void*)(uintptr_t)-2); if (*bp == (void*)(uintptr_t)-1) { // might have been eagerly handled for post-order while in the lazy pre-order queue @@ -1255,6 +1262,7 @@ static uintptr_t _backref_id(jl_serializer_state *s, jl_value_t *v, jl_array_t * { assert(v != NULL && "cannot get backref to NULL object"); if (jl_is_symbol(v)) { + OBJHASH_PIN(v); void **pidx = ptrhash_bp(&symbol_table, v); void *idx = *pidx; if (idx == HT_NOTFOUND) { diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c index c3d4ace606f9b..c9901e7a608b3 100644 --- a/src/staticdata_utils.c +++ b/src/staticdata_utils.c @@ -189,8 +189,10 @@ static int type_in_worklist(jl_value_t *v, jl_query_cache *cache) JL_NOTSAFEPOIN } // Memoize result - if (cache != NULL) + if (cache != NULL) { + OBJHASH_PIN(v); ptrhash_put(&cache->type_in_worklist, (void*)v, result ? (void*)v : NULL); + } return result; } From 3e312c3574e4c835c6f0c4f56bebff1038d665e8 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Mon, 3 Nov 2025 03:05:21 +0000 Subject: [PATCH 2/2] comments --- src/precompile_utils.c | 3 +-- src/staticdata.c | 19 +++++++++---------- src/staticdata_utils.c | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/precompile_utils.c b/src/precompile_utils.c index c85e94669c0d6..6b3ac4ffd7a37 100644 --- a/src/precompile_utils.c +++ b/src/precompile_utils.c @@ -418,11 +418,10 @@ static void jl_rebuild_methtables(arraylist_t *MIs, htable_t *mtables) JL_GC_DIS jl_method_instance_t *mi = (jl_method_instance_t*)MIs->items[i]; jl_method_t *m = mi->def.method; // Check if the method is already in the new table, if not then insert it there - OBJHASH_PIN(m); + OBJHASH_PIN(m); // m is inserted as the key and the value to the hashtable void **inserted = ptrhash_bp(&ms, m); if (*inserted != HT_NOTFOUND) continue; - OBJHASH_PIN(m); *inserted = (void*)m; jl_methtable_t *old_mt = jl_method_get_table(m); if ((jl_value_t *)old_mt == jl_nothing) diff --git a/src/staticdata.c b/src/staticdata.c index f7aa273366167..0d087b60f14d2 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -677,11 +677,11 @@ static jl_value_t *get_replaceable_field(jl_value_t **addr, int mutabl) JL_GC_DI if (fld == HT_NOTFOUND) { fld = *addr; if (mutabl && fld && jl_is_cpointer_type(jl_typeof(fld)) && jl_unbox_voidpointer(fld) != NULL && jl_unbox_voidpointer(fld) != (void*)(uintptr_t)-1) { - void **nullval = ptrhash_bp(&nullptrs, (void*)jl_typeof(fld)); + void **nullval = ptrhash_bp(&nullptrs, (void*)jl_typeof(fld)); // jl_typeof(fld) is nonmoving, no need to pin it if (*nullval == HT_NOTFOUND) { void *C_NULL = NULL; jl_value_t *new_fld = jl_new_bits(jl_typeof(fld), &C_NULL); - OBJHASH_PIN(new_fld); + OBJHASH_PIN(new_fld); // new_fld is stored in htables, pin it. *nullval = (void*)new_fld; } fld = (jl_value_t*)*nullval; @@ -693,8 +693,7 @@ static jl_value_t *get_replaceable_field(jl_value_t **addr, int mutabl) JL_GC_DI static uintptr_t jl_fptr_id(void *fptr) { - PTRHASH_PIN(fptr); - void **pbp = ptrhash_bp(&fptr_to_id, fptr); + void **pbp = ptrhash_bp(&fptr_to_id, fptr); // fptr is not a heap object, no need to pin it. if (*pbp == HT_NOTFOUND || fptr == NULL) return 0; else @@ -963,7 +962,7 @@ static void jl_insert_into_serialization_queue(jl_serializer_state *s, jl_value_ if (jl_object_in_image((jl_value_t*)def)) { void **pfound = ptrhash_bp(&s->method_roots_index, def); if (*pfound == HT_NOTFOUND) { - OBJHASH_PIN(def); + OBJHASH_PIN(def); // def is stored in htables as both key and value, pin it. *pfound = def; size_t nwithkey = nroots_with_key(def, s->worklist_key); if (nwithkey) { @@ -1091,13 +1090,13 @@ static void jl_insert_into_serialization_queue(jl_serializer_state *s, jl_value_ done_fields: ; // We've encountered an item we need to cache - OBJHASH_PIN(v); + OBJHASH_PIN(v); // v is stored in htables as the key, pin it. void **bp = ptrhash_bp(&serialization_order, v); assert(*bp == (void*)(uintptr_t)-2); arraylist_push(&serialization_queue, (void*) v); size_t idx = serialization_queue.len - 1; assert(serialization_queue.len < ((uintptr_t)1 << RELOC_TAG_OFFSET) && "too many items to serialize"); - *bp = to_seroder_entry(idx); + *bp = to_seroder_entry(idx); // the entry that gets stored is not an object reference, we don't need to pin it for htables. // DataType is very unusual, in that some of the fields need to be pre-order, and some // (notably super) must not be (even if `jl_queue_for_serialization_` would otherwise @@ -1153,7 +1152,7 @@ static void jl_queue_for_serialization_(jl_serializer_state *s, jl_value_t *v, i immediate = 1; } - OBJHASH_PIN(v); + OBJHASH_PIN(v); // v is stoored as the key in htables, pin it. The value that gets stored later is not an object reference. void **bp = ptrhash_bp(&serialization_order, v); assert(!immediate || *bp != (void*)(uintptr_t)-2); if (*bp == HT_NOTFOUND) @@ -1187,7 +1186,7 @@ static void jl_serialize_reachable(jl_serializer_state *s) JL_GC_DISABLED } prevlen = --object_worklist.len; jl_value_t *v = (jl_value_t*)object_worklist.items[prevlen]; - OBJHASH_PIN(v); + OBJHASH_PIN(v); // v is stoored as the key in htables, pin it. The value that gets stored later is not an object reference. void **bp = ptrhash_bp(&serialization_order, (void*)v); assert(*bp != HT_NOTFOUND && *bp != (void*)(uintptr_t)-2); if (*bp == (void*)(uintptr_t)-1) { // might have been eagerly handled for post-order while in the lazy pre-order queue @@ -1262,7 +1261,7 @@ static uintptr_t _backref_id(jl_serializer_state *s, jl_value_t *v, jl_array_t * { assert(v != NULL && "cannot get backref to NULL object"); if (jl_is_symbol(v)) { - OBJHASH_PIN(v); + OBJHASH_PIN(v); // v is stoored as the key in htables, pin it. The value that gets stored later is not an object reference. void **pidx = ptrhash_bp(&symbol_table, v); void *idx = *pidx; if (idx == HT_NOTFOUND) { diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c index c9901e7a608b3..19e6a5d02c9a3 100644 --- a/src/staticdata_utils.c +++ b/src/staticdata_utils.c @@ -190,7 +190,7 @@ static int type_in_worklist(jl_value_t *v, jl_query_cache *cache) JL_NOTSAFEPOIN // Memoize result if (cache != NULL) { - OBJHASH_PIN(v); + OBJHASH_PIN(v); // v is stored as the key in htables, pin it. ptrhash_put(&cache->type_in_worklist, (void*)v, result ? (void*)v : NULL); }