Trace pinned objects as roots (#91)

Following the suggestion in #89 (comment), trace recently added pinned objects as roots. When we pass those objects as 'nodes' to MMTk, MMTk does not know the slots so MMTk cannot update the reference, thus MMTk will not move those objects. This is essentially the same as pinning those objects before a GC, and unpinning after a GC.

Author: qinsoon

src/ast.c

@@ -136,18 +136,19 @@ typedef struct _jl_ast_context_t {
     value_t ssavalue_sym;
     value_t slot_sym;
     jl_module_t *module; // context module for `current-julia-module-counter`
-    arraylist_t pinned_objects;
+    // These are essentially roots for ast context.
+    arraylist_t ast_roots;
 } jl_ast_context_t;
 
 // FIXME: Ugly hack to get a pointer to the pinned objects
-arraylist_t *extract_pinned_objects_from_ast_ctx(void *ctx)
+arraylist_t *extract_ast_roots_from_ast_ctx(void *ctx)
 {
     // This is used to extract pinned objects from the context
     // for the purpose of pinning them in MMTk.
     if (ctx == NULL)
         return NULL;
     jl_ast_context_t *jl_ctx = (jl_ast_context_t*)ctx;
-    return &jl_ctx->pinned_objects;
+    return &jl_ctx->ast_roots;
 }
 
 static jl_ast_context_t jl_ast_main_ctx;
@@ -290,7 +291,7 @@ static void jl_init_ast_ctx(jl_ast_context_t *ctx) JL_NOTSAFEPOINT
     ctx->slot_sym = symbol(fl_ctx, "slot");
     ctx->module = NULL;
     set(symbol(fl_ctx, "*scopewarn-opt*"), fixnum(jl_options.warn_scope));
-    arraylist_new(&ctx->pinned_objects, 0);
+    arraylist_new(&ctx->ast_roots, 0);
 }
 
 // There should be no GC allocation while holding this lock
@@ -321,7 +322,7 @@ static void jl_ast_ctx_leave(jl_ast_context_t *ctx)
 {
     uv_mutex_lock(&flisp_lock);
     ctx->module = NULL;
-    ctx->pinned_objects.len = 0; // clear pinned objects
+    ctx->ast_roots.len = 0; // clear root objects
     arraylist_pop(&jl_ast_ctx_used);
     arraylist_push(&jl_ast_ctx_freed, ctx);
     uv_mutex_unlock(&flisp_lock);
@@ -802,7 +803,7 @@ static value_t julia_to_scm_(jl_ast_context_t *ctx, jl_value_t *v, int check_val
 {
     // The following code will take internal pointers to v's fields. We need to make sure
     // that v will not be moved by GC.
-    arraylist_push(&ctx->pinned_objects, v);
+    arraylist_push(&ctx->ast_roots, v);
     value_t retval;
     fl_context_t *fl_ctx = &ctx->fl;
     if (julia_to_scm_noalloc1(fl_ctx, v, &retval))

src/engine.cpp

@@ -64,15 +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);
-            arraylist_push(&gc_pinned_objects, owner);
+            arraylist_push(&extra_gc_roots, 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(&gc_pinned_objects);
+                    arraylist_pop(&extra_gc_roots);
                     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(&gc_pinned_objects);
+                        arraylist_pop(&extra_gc_roots);
                         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(&gc_pinned_objects);
+            arraylist_pop(&extra_gc_roots);
         })())
         ct->ptls->engine_nqueued--;
     JL_GC_POP();

src/gc-common.c

@@ -687,7 +687,7 @@ JL_DLLEXPORT int jl_gc_enable(int on)
 // MISC
 // =========================================================================== //
 
-arraylist_t gc_pinned_objects;
+arraylist_t extra_gc_roots;
 
 JL_DLLEXPORT jl_weakref_t *jl_gc_new_weakref(jl_value_t *value)
 {

src/gc-mmtk.c

@@ -74,7 +74,7 @@ void jl_gc_init(void) {
 
     jl_set_check_alive_type(mmtk_is_reachable_object);
 
-    arraylist_new(&gc_pinned_objects, 0);
+    arraylist_new(&extra_gc_roots, 0);
     arraylist_new(&to_finalize, 0);
     arraylist_new(&finalizer_list_marked, 0);
     gc_num.interval = default_collect_interval;
@@ -254,36 +254,6 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection) {
     // print_fragmentation();
 }
 
-void gc_pin_objects_from_compiler_frontend(arraylist_t *objects_pinned_by_call)
-{
-    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);
-        }
-    }
-    for (size_t i = 0; i < jl_ast_ctx_used.len; i++) {
-        void *ctx = jl_ast_ctx_used.items[i];
-        arraylist_t *pinned_objects = extract_pinned_objects_from_ast_ctx(ctx);
-        for (size_t j = 0; j < pinned_objects->len; j++) {
-            void *obj = pinned_objects->items[j];
-            unsigned char got_pinned = mmtk_pin_object(obj);
-            if (got_pinned) {
-                arraylist_push(objects_pinned_by_call, obj);
-            }
-        }
-    }
-}
-
-void gc_unpin_objects_from_compiler_frontend(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`
 JL_DLLEXPORT void jl_gc_prepare_to_collect(void)
@@ -346,12 +316,7 @@ 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_compiler_frontend(&objects_pinned_by_call);
         mmtk_block_thread_for_gc();
-        gc_unpin_objects_from_compiler_frontend(&objects_pinned_by_call);
-        arraylist_free(&objects_pinned_by_call);
 #endif
         JL_UNLOCK_NOGC(&finalizers_lock);
     }
@@ -838,6 +803,22 @@ JL_DLLEXPORT void jl_gc_scan_vm_specific_roots(RootsWorkClosure* closure)
         }
     }
 
+    // Trace objects in extra_gc_roots
+    for (size_t i = 0; i < extra_gc_roots.len; i++) {
+        void* obj = extra_gc_roots.items[i];
+        add_node_to_roots_buffer(closure, &buf, &len, obj);
+    }
+
+    // Trace objects in jl_ast_ctx_used
+    for (size_t i = 0; i < jl_ast_ctx_used.len; i++) {
+        void *ctx = jl_ast_ctx_used.items[i];
+        arraylist_t *ast_roots = extract_ast_roots_from_ast_ctx(ctx);
+        for (size_t j = 0; j < ast_roots->len; j++) {
+            void *obj = ast_roots->items[j];
+            add_node_to_roots_buffer(closure, &buf, &len, obj);
+        }
+    }
+
     // // add module
     // add_node_to_roots_buffer(closure, &buf, &len, jl_main_module);
 

src/julia.h

@@ -1346,7 +1346,9 @@ JL_DLLEXPORT JL_CONST_FUNC jl_gcframe_t **(jl_get_pgcstack)(void) JL_GLOBALLY_RO
 
 // object pinning  ------------------------------------------------------------
 
-extern arraylist_t gc_pinned_objects;
+// These 'new roots' are added for moving GCs.
+// We could consider merging this list with global roots list if we can push and pop from global roots list in the same way.
+extern arraylist_t extra_gc_roots;
 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);
@@ -2439,7 +2441,7 @@ JL_DLLEXPORT void jl_register_newmeth_tracer(void (*callback)(jl_method_t *trace
 
 // AST access
 JL_DLLEXPORT jl_value_t *jl_copy_ast(jl_value_t *expr JL_MAYBE_UNROOTED);
-arraylist_t *extract_pinned_objects_from_ast_ctx(void *ctx);
+arraylist_t *extract_ast_roots_from_ast_ctx(void *ctx);
 extern arraylist_t jl_ast_ctx_used;
 
 // IR representation