Skip to content

Commit

Permalink
Add a hook before yielding for GC to save context
Browse files Browse the repository at this point in the history
  • Loading branch information
qinsoon committed Feb 7, 2025
1 parent e65af91 commit d219282
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/gc-interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ JL_DLLEXPORT const char* jl_gc_active_impl(void);
// each GC should implement it but it will most likely not be used by other code in the runtime.
// It still needs to be annotated with JL_DLLEXPORT since it is called from Rust by MMTk.
JL_DLLEXPORT void jl_gc_sweep_stack_pools_and_mtarraylist_buffers(jl_ptls_t ptls) JL_NOTSAFEPOINT;
// Notifies the GC that the given thread is about to yield for a GC. ctx is the ucontext for the thread
// if it is already fetched by the caller, otherwise it is NULL.
JL_DLLEXPORT void jl_gc_notify_thread_yield(jl_ptls_t ptls, void* ctx);

// ========================================================================= //
// Metrics
Expand Down
6 changes: 6 additions & 0 deletions src/gc-mmtk.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,8 @@ JL_DLLEXPORT void jl_gc_prepare_to_collect(void)
gc_num.total_time_to_safepoint += duration;

if (!jl_atomic_load_acquire(&jl_gc_disable_counter)) {
// This thread will yield.
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__
mmtk_block_thread_for_gc();
Expand Down Expand Up @@ -323,6 +325,10 @@ JL_DLLEXPORT void jl_gc_prepare_to_collect(void)
errno = last_errno;
}

JL_DLLEXPORT void jl_gc_notify_thread_yield(jl_ptls_t ptls, void* ctx) {
// TODO: Save the context of this thread for conservative scanning.
}

// ========================================================================= //
// GC Statistics
// ========================================================================= //
Expand Down
9 changes: 9 additions & 0 deletions src/gc-stock.c
Original file line number Diff line number Diff line change
Expand Up @@ -3458,6 +3458,11 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection)
gc_cblist_pre_gc, (collection));

if (!jl_atomic_load_acquire(&jl_gc_disable_counter)) {
// This thread will yield.
// jl_gc_notify_thread_yield does nothing for the stock GC at the point, but it may be non empty in the future,
// and this is a place where we should call jl_gc_notify_thread_yield.
// TODO: This call can be removed if requested.
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__
if (_jl_gc_collect(ptls, collection)) {
Expand Down Expand Up @@ -4080,6 +4085,10 @@ JL_DLLEXPORT const char* jl_gc_active_impl(void) {
return "Built with stock GC";
}

JL_DLLEXPORT void jl_gc_notify_thread_yield(jl_ptls_t ptls, void* ctx) {
// Do nothing before a thread yields
}

#ifdef __cplusplus
}
#endif
2 changes: 2 additions & 0 deletions src/signals-unix.c
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,8 @@ JL_NO_ASAN static void segv_handler(int sig, siginfo_t *info, void *context)
return;
}
if (sig == SIGSEGV && info->si_code == SEGV_ACCERR && jl_addr_is_safepoint((uintptr_t)info->si_addr) && !is_write_fault(context)) {
// TODO: We should do the same for other platforms
jl_gc_notify_thread_yield(ct->ptls, context);
jl_set_gc_and_wait(ct);
// Do not raise sigint on worker thread
if (jl_atomic_load_relaxed(&ct->tid) != 0)
Expand Down

0 comments on commit d219282

Please sign in to comment.