Skip to content

Commit 8dc5535

Browse files
authored
Save thread context before yielding for GC (#78)
This PR ports mmtk/mmtk-julia#159 to `dev`. The difference is that this PR adds a general call to the GC interface `jl_gc_notify_thread_yield`. In this case, each GC will do what they need in the call, and the context is saved in the GC specific TLS.
1 parent dd0a1c3 commit 8dc5535

File tree

5 files changed

+30
-0
lines changed

5 files changed

+30
-0
lines changed

src/gc-interface.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ JL_DLLEXPORT int gc_is_collector_thread(int tid) JL_NOTSAFEPOINT;
102102
JL_DLLEXPORT unsigned char jl_gc_pin_object(void* obj);
103103
// Returns the version of which GC implementation is being used according to the list of supported GCs
104104
JL_DLLEXPORT const char* jl_active_gc_impl(void);
105+
// Notifies the GC that the given thread is about to yield for a GC. ctx is the ucontext for the thread
106+
// if it is already fetched by the caller, otherwise it is NULL.
107+
JL_DLLEXPORT void jl_gc_notify_thread_yield(jl_ptls_t ptls, void* ctx);
105108

106109
// TODO: The preserve hook functions may be temporary. We should see the performance impact of the change.
107110

src/gc-mmtk.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,8 @@ JL_DLLEXPORT void jl_mmtk_prepare_to_collect(void)
270270
gc_num.total_time_to_safepoint += duration;
271271

272272
if (!jl_atomic_load_acquire(&jl_gc_disable_counter)) {
273+
// This thread will yield.
274+
jl_gc_notify_thread_yield(ptls, NULL);
273275
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
274276
#ifndef __clang_gcanalyzer__
275277
mmtk_block_thread_for_gc();
@@ -303,6 +305,19 @@ JL_DLLEXPORT unsigned char jl_gc_pin_object(void* obj) {
303305
return mmtk_pin_object(obj);
304306
}
305307

308+
JL_DLLEXPORT void jl_gc_notify_thread_yield(jl_ptls_t ptls, void* ctx) {
309+
if (ctx == NULL) {
310+
// Save the context for the thread as it was running at the time of the call
311+
int r = getcontext(&ptls->gc_tls.ctx_at_the_time_gc_started);
312+
if (r == -1) {
313+
jl_safe_printf("Failed to save context for conservative scanning\n");
314+
abort();
315+
}
316+
return;
317+
}
318+
memcpy(&ptls->gc_tls.ctx_at_the_time_gc_started, ctx, sizeof(ucontext_t));
319+
}
320+
306321
// ========================================================================= //
307322
// GC Statistics
308323
// ========================================================================= //

src/gc-stock.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3361,6 +3361,11 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection)
33613361
gc_cblist_pre_gc, (collection));
33623362

33633363
if (!jl_atomic_load_acquire(&jl_gc_disable_counter)) {
3364+
// This thread will yield.
3365+
// jl_gc_notify_thread_yield does nothing for the stock GC at the point, but it may be non empty in the future,
3366+
// and this is a place where we should call jl_gc_notify_thread_yield.
3367+
// TODO: This call can be removed if requested.
3368+
jl_gc_notify_thread_yield(ptls, NULL);
33643369
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
33653370
#ifndef __clang_gcanalyzer__
33663371
if (_jl_gc_collect(ptls, collection)) {
@@ -4022,6 +4027,10 @@ JL_DLLEXPORT const char* jl_active_gc_impl(void) {
40224027
return "";
40234028
}
40244029

4030+
JL_DLLEXPORT void jl_gc_notify_thread_yield(jl_ptls_t ptls, void* ctx) {
4031+
// Do nothing before a thread yields
4032+
}
4033+
40254034
#ifdef __cplusplus
40264035
}
40274036
#endif

src/gc-tls-mmtk.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ extern "C" {
1010
typedef struct {
1111
MMTkMutatorContext mmtk_mutator;
1212
size_t malloc_sz_since_last_poll;
13+
ucontext_t ctx_at_the_time_gc_started;
1314
} jl_gc_tls_states_t;
1415

1516
#ifdef __cplusplus

src/signals-unix.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,8 @@ JL_NO_ASAN static void segv_handler(int sig, siginfo_t *info, void *context)
388388
return;
389389
}
390390
if (sig == SIGSEGV && info->si_code == SEGV_ACCERR && jl_addr_is_safepoint((uintptr_t)info->si_addr) && !is_write_fault(context)) {
391+
// TODO: We should do the same for other platforms
392+
jl_gc_notify_thread_yield(ct->ptls, context);
391393
jl_set_gc_and_wait();
392394
// Do not raise sigint on worker thread
393395
if (jl_atomic_load_relaxed(&ct->tid) != 0)

0 commit comments

Comments
 (0)