From e19c0da18c5a7633f4b1d8b4b5d7c80e41a312d6 Mon Sep 17 00:00:00 2001 From: Diogo Correia Netto Date: Mon, 16 Jun 2025 17:49:57 -0300 Subject: [PATCH 1/2] Don't pin objects permanently in engine.cpp --- src/engine.cpp | 23 +++++++++++++---------- src/gc-common.c | 2 ++ src/gc-mmtk.c | 27 ++++++++++++++++++++++++++- src/gc-pinning-log.cpp | 8 ++++---- src/julia.h | 9 +++++---- 5 files changed, 50 insertions(+), 19 deletions(-) diff --git a/src/engine.cpp b/src/engine.cpp index 08a9c300c4668..e6a8d2c452fed 100644 --- a/src/engine.cpp +++ b/src/engine.cpp @@ -18,7 +18,7 @@ struct ReservationInfo { struct InferKey { jl_method_instance_t *mi = nullptr; - jl_pinned_ref(jl_value_t) owner = jl_pinned_ref_assume(jl_value_t, nullptr); + jl_value_t *owner = nullptr; }; template<> struct llvm::DenseMapInfo { @@ -26,18 +26,16 @@ template<> struct llvm::DenseMapInfo { using SecondInfo = DenseMapInfo; static inline InferKey getEmptyKey() { - return InferKey{FirstInfo::getEmptyKey(), - jl_pinned_ref_assume(jl_value_t, SecondInfo::getEmptyKey())}; + return InferKey{FirstInfo::getEmptyKey(), SecondInfo::getEmptyKey()}; } static inline InferKey getTombstoneKey() { - return InferKey{FirstInfo::getTombstoneKey(), - jl_pinned_ref_assume(jl_value_t, SecondInfo::getTombstoneKey())}; + return InferKey{FirstInfo::getTombstoneKey(), SecondInfo::getTombstoneKey()}; } static unsigned getHashValue(const InferKey& PairVal) { return detail::combineHashValue(FirstInfo::getHashValue(PairVal.mi), - SecondInfo::getHashValue(jl_pinned_ref_get(PairVal.owner))); + SecondInfo::getHashValue(PairVal.owner)); } static bool isEqual(const InferKey &LHS, const InferKey &RHS) { @@ -66,13 +64,15 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner auto tid = jl_atomic_load_relaxed(&ct->tid); if (([tid, m, owner, ci] () -> bool { // necessary scope block / lambda for unique_lock jl_unique_gcsafe_lock lock(engine_lock); - InferKey key{m, jl_pinned_ref_create(jl_value_t, owner)}; + arraylist_push(&objects_pinned_by_inference_engine, owner); + InferKey key{m, owner}; if ((signed)Awaiting.size() < tid + 1) Awaiting.resize(tid + 1); while (1) { auto record = Reservations.find(key); if (record == Reservations.end()) { Reservations[key] = ReservationInfo{tid, ci}; + arraylist_pop(&objects_pinned_by_inference_engine); return false; } // before waiting, need to run deadlock/cycle detection @@ -80,8 +80,10 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner // and waiting for (transitively) any lease that is held by this thread auto wait_tid = record->second.tid; while (1) { - if (wait_tid == tid) + if (wait_tid == tid) { + arraylist_pop(&objects_pinned_by_inference_engine); return true; + } if ((signed)Awaiting.size() <= wait_tid) break; // no cycle, since it is running (and this should be unreachable) auto key2 = Awaiting[wait_tid]; @@ -97,6 +99,7 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner lock.wait(engine_wait); Awaiting[tid] = InferKey{}; } + arraylist_pop(&objects_pinned_by_inference_engine); })()) ct->ptls->engine_nqueued--; JL_GC_POP(); @@ -106,7 +109,7 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner int jl_engine_hasreserved(jl_method_instance_t *m, jl_value_t *owner) { jl_task_t *ct = jl_current_task; - InferKey key = {m, jl_pinned_ref_create(jl_value_t, owner)}; + InferKey key = {m, owner}; std::unique_lock lock(engine_lock); auto record = Reservations.find(key); return record != Reservations.end() && record->second.tid == jl_atomic_load_relaxed(&ct->tid); @@ -139,7 +142,7 @@ void jl_engine_fulfill(jl_code_instance_t *ci, jl_code_info_t *src) { jl_task_t *ct = jl_current_task; std::unique_lock lock(engine_lock); - auto record = Reservations.find(InferKey{jl_get_ci_mi(ci), jl_pinned_ref_create(jl_value_t, ci->owner)}); + auto record = Reservations.find(InferKey{jl_get_ci_mi(ci), ci->owner}); if (record == Reservations.end() || record->second.ci != ci) return; assert(jl_atomic_load_relaxed(&ct->tid) == record->second.tid); diff --git a/src/gc-common.c b/src/gc-common.c index 0816db696fdb0..ce71d197d35c1 100644 --- a/src/gc-common.c +++ b/src/gc-common.c @@ -687,6 +687,8 @@ JL_DLLEXPORT int jl_gc_enable(int on) // MISC // =========================================================================== // +arraylist_t objects_pinned_by_inference_engine; + JL_DLLEXPORT jl_weakref_t *jl_gc_new_weakref(jl_value_t *value) { jl_ptls_t ptls = jl_current_task->ptls; diff --git a/src/gc-mmtk.c b/src/gc-mmtk.c index 9e9f0b248ef32..8c6ec4877241d 100644 --- a/src/gc-mmtk.c +++ b/src/gc-mmtk.c @@ -55,6 +55,7 @@ extern void mmtk_post_alloc(void* mutator, void* refer, size_t bytes, int alloca extern void mmtk_store_obj_size_c(void* obj, size_t size); extern bool mmtk_is_pinned(void* obj); extern unsigned char mmtk_pin_object(void* obj); +extern unsigned char mmtk_unpin_object(void* obj); extern bool mmtk_is_reachable_object(void* obj); extern bool mmtk_is_live_object(void* obj); extern bool mmtk_is_object_pinned(void* obj); @@ -71,8 +72,9 @@ void jl_gc_init(void) { // TODO: use jl_options.heap_size_hint to set MMTk's fixed heap size? (see issue: https://github.com/mmtk/mmtk-julia/issues/167) JL_MUTEX_INIT(&finalizers_lock, "finalizers_lock"); - jl_set_check_alive_fn(mmtk_is_reachable_object); + jl_set_check_alive_type(mmtk_is_reachable_object); + arraylist_new(&objects_pinned_by_inference_engine, 0); arraylist_new(&to_finalize, 0); arraylist_new(&finalizer_list_marked, 0); gc_num.interval = default_collect_interval; @@ -247,6 +249,24 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection) { // print_fragmentation(); } +void gc_pin_objects_from_inference_engine(arraylist_t *objects_pinned_by_call) +{ + for (size_t i = 0; i < objects_pinned_by_inference_engine.len; i++) { + void *obj = objects_pinned_by_inference_engine.items[i]; + unsigned char got_pinned = mmtk_pin_object(obj); + if (got_pinned) { + arraylist_push(objects_pinned_by_call, obj); + } + } +} + +void gc_unpin_objects_from_inference_engine(arraylist_t *objects_pinned_by_call) +{ + for (size_t i = 0; i < objects_pinned_by_call->len; i++) { + void *obj = objects_pinned_by_call->items[i]; + mmtk_unpin_object(obj); + } +} // Based on jl_gc_collect from gc-stock.c // called when stopping the thread in `mmtk_block_for_gc` @@ -310,7 +330,12 @@ JL_DLLEXPORT void jl_gc_prepare_to_collect(void) jl_gc_notify_thread_yield(ptls, NULL); JL_LOCK_NOGC(&finalizers_lock); // all the other threads are stopped, so this does not make sense, right? otherwise, failing that, this seems like plausibly a deadlock #ifndef __clang_gcanalyzer__ + arraylist_t objects_pinned_by_call; + arraylist_new(&objects_pinned_by_call, 0); + gc_pin_objects_from_inference_engine(&objects_pinned_by_call); mmtk_block_thread_for_gc(); + gc_unpin_objects_from_inference_engine(&objects_pinned_by_call); + arraylist_free(&objects_pinned_by_call); #endif JL_UNLOCK_NOGC(&finalizers_lock); } diff --git a/src/gc-pinning-log.cpp b/src/gc-pinning-log.cpp index 15349c31beeae..dc6f478ef18d2 100644 --- a/src/gc-pinning-log.cpp +++ b/src/gc-pinning-log.cpp @@ -72,7 +72,7 @@ struct coalesced_pinning_log_t { struct pinning_log_t { linear_pinning_log_t linear_log; coalesced_pinning_log_t coalesced_log; - check_alive_fn is_alive; + check_alive_fn_type is_alive; std::mutex mu; pinning_log_entry_t *alloc_pinning_log_entry(void *pinned_object) { pinning_log_entry_t *e; @@ -92,7 +92,7 @@ struct pinning_log_t { linear_log.reset_log_buffer(); mu.unlock(); } - void set_check_alive_fn(check_alive_fn fn) { + void set_check_alive_fn_type(check_alive_fn_type fn) { mu.lock(); is_alive = fn; mu.unlock(); @@ -157,8 +157,8 @@ extern "C" { int pinning_log_enabled; -JL_DLLEXPORT void jl_set_check_alive_fn(check_alive_fn fn) { - pinning_log.set_check_alive_fn(fn); +JL_DLLEXPORT void jl_set_check_alive_type(check_alive_fn_type fn) { + pinning_log.set_check_alive_fn_type(fn); } JL_DLLEXPORT void jl_enable_pinning_log(void) { pinning_log_enabled = 1; diff --git a/src/julia.h b/src/julia.h index 011c6c1780763..3a9c83325a2b4 100644 --- a/src/julia.h +++ b/src/julia.h @@ -1346,8 +1346,9 @@ JL_DLLEXPORT JL_CONST_FUNC jl_gcframe_t **(jl_get_pgcstack)(void) JL_GLOBALLY_RO // object pinning ------------------------------------------------------------ -typedef bool (*check_alive_fn)(void *); -JL_DLLEXPORT void jl_set_check_alive_fn(check_alive_fn fn); +extern arraylist_t objects_pinned_by_inference_engine; +typedef bool (*check_alive_fn_type)(void *); +JL_DLLEXPORT void jl_set_check_alive_type(check_alive_fn_type fn); JL_DLLEXPORT void jl_log_pinning_event(void *pinned_object, const char *filename, int lineno); JL_DLLEXPORT void jl_print_pinning_log(void); @@ -1396,14 +1397,14 @@ class pinned_ref { explicit pinned_ref(void* p) : ptr(static_cast(p)) {} operator void*() const { return ptr; } T* get() const { return ptr; } - static pinned_ref create(void* p) { OBJ_PIN(p); return pinned_ref(p); } + static pinned_ref create(void* p, const char *file, int line) { jl_log_pinning_event(p, file, line); jl_gc_pin_object(p); return pinned_ref(p); } static pinned_ref assume(void* p) { return pinned_ref(p); } }; // Redefine macros for C++ to use the template version #define jl_pinned_ref(T) pinned_ref #define jl_pinned_ref_assume(T, ptr) pinned_ref::assume(ptr) -#define jl_pinned_ref_create(T, ptr) pinned_ref::create(ptr) +#define jl_pinned_ref_create(T, ptr) pinned_ref::create(ptr, __FILE__, __LINE__) #define jl_pinned_ref_get(ref) (ref).get() #else From 98ded5d8e2aa04ebd8a3fe284e163f9e3df2946a Mon Sep 17 00:00:00 2001 From: Diogo Correia Netto Date: Tue, 17 Jun 2025 22:43:15 -0300 Subject: [PATCH 2/2] suggestions from Yi' review --- src/engine.cpp | 10 +++++----- src/gc-common.c | 2 +- src/gc-mmtk.c | 6 +++--- src/julia.h | 2 +- src/support/analyzer_annotations.h | 2 ++ 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/engine.cpp b/src/engine.cpp index e6a8d2c452fed..324dc9f12347c 100644 --- a/src/engine.cpp +++ b/src/engine.cpp @@ -18,7 +18,7 @@ struct ReservationInfo { struct InferKey { jl_method_instance_t *mi = nullptr; - jl_value_t *owner = nullptr; + JL_ROOT jl_value_t *owner = nullptr; }; template<> struct llvm::DenseMapInfo { @@ -64,7 +64,7 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner auto tid = jl_atomic_load_relaxed(&ct->tid); if (([tid, m, owner, ci] () -> bool { // necessary scope block / lambda for unique_lock jl_unique_gcsafe_lock lock(engine_lock); - arraylist_push(&objects_pinned_by_inference_engine, owner); + arraylist_push(&gc_pinned_objects, owner); InferKey key{m, owner}; if ((signed)Awaiting.size() < tid + 1) Awaiting.resize(tid + 1); @@ -72,7 +72,7 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner auto record = Reservations.find(key); if (record == Reservations.end()) { Reservations[key] = ReservationInfo{tid, ci}; - arraylist_pop(&objects_pinned_by_inference_engine); + arraylist_pop(&gc_pinned_objects); return false; } // before waiting, need to run deadlock/cycle detection @@ -81,7 +81,7 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner auto wait_tid = record->second.tid; while (1) { if (wait_tid == tid) { - arraylist_pop(&objects_pinned_by_inference_engine); + arraylist_pop(&gc_pinned_objects); return true; } if ((signed)Awaiting.size() <= wait_tid) @@ -99,7 +99,7 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner lock.wait(engine_wait); Awaiting[tid] = InferKey{}; } - arraylist_pop(&objects_pinned_by_inference_engine); + arraylist_pop(&gc_pinned_objects); })()) ct->ptls->engine_nqueued--; JL_GC_POP(); diff --git a/src/gc-common.c b/src/gc-common.c index ce71d197d35c1..6d23db820be01 100644 --- a/src/gc-common.c +++ b/src/gc-common.c @@ -687,7 +687,7 @@ JL_DLLEXPORT int jl_gc_enable(int on) // MISC // =========================================================================== // -arraylist_t objects_pinned_by_inference_engine; +arraylist_t gc_pinned_objects; JL_DLLEXPORT jl_weakref_t *jl_gc_new_weakref(jl_value_t *value) { diff --git a/src/gc-mmtk.c b/src/gc-mmtk.c index 8c6ec4877241d..63f35d81df6fc 100644 --- a/src/gc-mmtk.c +++ b/src/gc-mmtk.c @@ -74,7 +74,7 @@ void jl_gc_init(void) { jl_set_check_alive_type(mmtk_is_reachable_object); - arraylist_new(&objects_pinned_by_inference_engine, 0); + arraylist_new(&gc_pinned_objects, 0); arraylist_new(&to_finalize, 0); arraylist_new(&finalizer_list_marked, 0); gc_num.interval = default_collect_interval; @@ -251,8 +251,8 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection) { void gc_pin_objects_from_inference_engine(arraylist_t *objects_pinned_by_call) { - for (size_t i = 0; i < objects_pinned_by_inference_engine.len; i++) { - void *obj = objects_pinned_by_inference_engine.items[i]; + for (size_t i = 0; i < gc_pinned_objects.len; i++) { + void *obj = gc_pinned_objects.items[i]; unsigned char got_pinned = mmtk_pin_object(obj); if (got_pinned) { arraylist_push(objects_pinned_by_call, obj); diff --git a/src/julia.h b/src/julia.h index 3a9c83325a2b4..00c0b992e75de 100644 --- a/src/julia.h +++ b/src/julia.h @@ -1346,7 +1346,7 @@ JL_DLLEXPORT JL_CONST_FUNC jl_gcframe_t **(jl_get_pgcstack)(void) JL_GLOBALLY_RO // object pinning ------------------------------------------------------------ -extern arraylist_t objects_pinned_by_inference_engine; +extern arraylist_t gc_pinned_objects; typedef bool (*check_alive_fn_type)(void *); JL_DLLEXPORT void jl_set_check_alive_type(check_alive_fn_type fn); JL_DLLEXPORT void jl_log_pinning_event(void *pinned_object, const char *filename, int lineno); diff --git a/src/support/analyzer_annotations.h b/src/support/analyzer_annotations.h index 69827e4d77f37..5f8b9b70c1762 100644 --- a/src/support/analyzer_annotations.h +++ b/src/support/analyzer_annotations.h @@ -15,6 +15,7 @@ #define JL_NOTSAFEPOINT_ENTER __attribute__((annotate("julia_notsafepoint_enter"))) #define JL_NOTSAFEPOINT_LEAVE __attribute__((annotate("julia_notsafepoint_leave"))) #define JL_MAYBE_UNROOTED __attribute__((annotate("julia_maybe_unrooted"))) +#define JL_ROOT __attribute__((annotate("julia_rooted"))) #define JL_GLOBALLY_ROOTED __attribute__((annotate("julia_globally_rooted"))) #define JL_ROOTING_ARGUMENT __attribute__((annotate("julia_rooting_argument"))) #define JL_ROOTED_ARGUMENT __attribute__((annotate("julia_rooted_argument"))) @@ -38,6 +39,7 @@ extern "C" { #define JL_NOTSAFEPOINT_ENTER #define JL_NOTSAFEPOINT_LEAVE #define JL_MAYBE_UNROOTED +#define JL_ROOT #define JL_GLOBALLY_ROOTED #define JL_ROOTING_ARGUMENT #define JL_ROOTED_ARGUMENT