Merge pull request #89 from mmtk/dcn-optimize-pinning-in-inference-engine

Don't pin objects permanently in engine.cpp

Author: d-netto

src/engine.cpp

@@ -18,26 +18,24 @@ 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_ROOT jl_value_t *owner = nullptr;
 };
 
 template<> struct llvm::DenseMapInfo<InferKey> {
   using FirstInfo = DenseMapInfo<jl_method_instance_t*>;
   using SecondInfo = DenseMapInfo<jl_value_t*>;
 
   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,22 +64,26 @@ 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(&gc_pinned_objects, 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);
                     return false;
                 }
                 // before waiting, need to run deadlock/cycle detection
                 // there is a cycle if the thread holding our lease is blocked
                 // 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(&gc_pinned_objects);
                         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(&gc_pinned_objects);
         })())
         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);

src/gc-common.c

@@ -687,6 +687,8 @@ JL_DLLEXPORT int jl_gc_enable(int on)
 // MISC
 // =========================================================================== //
 
+arraylist_t gc_pinned_objects;
+
 JL_DLLEXPORT jl_weakref_t *jl_gc_new_weakref(jl_value_t *value)
 {
     jl_ptls_t ptls = jl_current_task->ptls;

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(&gc_pinned_objects, 0);
     arraylist_new(&to_finalize, 0);
     arraylist_new(&finalizer_list_marked, 0);
     gc_num.interval = default_collect_interval;
@@ -252,6 +254,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 < 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);
+        }
+    }
+}
+
+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`
@@ -315,7 +335,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);
     }

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;

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 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);
 JL_DLLEXPORT void jl_print_pinning_log(void);
 
@@ -1396,14 +1397,14 @@ class pinned_ref {
     explicit pinned_ref(void* p) : ptr(static_cast<T*>(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<T>
 #define jl_pinned_ref_assume(T, ptr) pinned_ref<T>::assume(ptr)
-#define jl_pinned_ref_create(T, ptr) pinned_ref<T>::create(ptr)
+#define jl_pinned_ref_create(T, ptr) pinned_ref<T>::create(ptr, __FILE__, __LINE__)
 #define jl_pinned_ref_get(ref) (ref).get()
 
 #else

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