Refactoring pinning functions and adding comments

Author: udesou

src/array.c

@@ -239,9 +239,11 @@ JL_DLLEXPORT jl_array_t *jl_reshape_array(jl_value_t *atype, jl_array_t *data,
     jl_array_t *owner = (jl_array_t*)jl_array_owner(data);
     jl_array_data_owner(a) = (jl_value_t*)owner;
 
-    if(mmtk_object_is_managed_by_mmtk(owner)) {
-        mmtk_pin_object(owner);
-    }
+    // For array objects with an owner point (a->flags.how == 3), we would need to
+    // introspect the object to update the a->data field. To avoid doing that and
+    // making scan_object much more complex we simply enforce that both owner and
+    // buffers are always pinned
+    mmtk_pin_object(owner);
     a->flags.how = 3;
     a->data = data->data;
     a->flags.isshared = 1;
@@ -290,9 +292,11 @@ JL_DLLEXPORT jl_array_t *jl_string_to_array(jl_value_t *str)
     a->flags.ptrarray = 0;
     a->flags.hasptr = 0;
     jl_array_data_owner(a) = str;
-    if(mmtk_object_is_managed_by_mmtk(str)) {
-        mmtk_pin_object(str);
-    }
+    // For array objects with an owner point (a->flags.how == 3), we would need to
+    // introspect the object to update the a->data field. To avoid doing that and
+    // making scan_object much more complex we simply enforce that both owner and
+    // buffers are always pinned
+    mmtk_pin_object(str);
     a->flags.how = 3;
     a->flags.isshared = 1;
     size_t l = jl_string_len(str);
@@ -689,9 +693,11 @@ static int NOINLINE array_resize_buffer(jl_array_t *a, size_t newlen)
         else {
             s = jl_gc_realloc_string(jl_array_data_owner(a), nbytes - (elsz == 1));
         }
-        if(mmtk_object_is_managed_by_mmtk(s)) {
-            mmtk_pin_object(s);
-        }
+        // For array objects with an owner point (a->flags.how == 3), we would need to
+        // introspect the object to update the a->data field. To avoid doing that and
+        // making scan_object much more complex we simply enforce that both owner and
+        // buffers are always pinned
+        mmtk_pin_object(s);
         jl_array_data_owner(a) = s;
         jl_gc_wb(a, s);
         a->data = jl_string_data(s);

src/builtins.c

@@ -344,9 +344,9 @@ static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOIN
             i++;
             pe = pe->prev;
         }
-        if(mmtk_object_is_managed_by_mmtk(v)) {
-            mmtk_pin_object(v);
-        }
+        // FIXME: Pinning objects that get hashed
+        // until we implement address space hashing.
+        mmtk_pin_object(v);
         return inthash((uintptr_t)v);
     }
     if (tv == jl_uniontype_type) {
@@ -395,9 +395,10 @@ static uintptr_t immut_id_(jl_datatype_t *dt, jl_value_t *v, uintptr_t h) JL_NOT
         return ~h;
     size_t f, nf = jl_datatype_nfields(dt);
     if (nf == 0 || (!dt->layout->haspadding && dt->layout->npointers == 0)) {
-        if(mmtk_object_is_managed_by_mmtk(v)) {
-            mmtk_pin_object(v);
-        }
+
+        // FIXME: Pinning objects that get hashed
+        // until we implement address space hashing.
+        mmtk_pin_object(v);
         // operate element-wise if there are unused bits inside,
         // otherwise just take the whole data block at once
         // a few select pointers (notably symbol) also have special hash values
@@ -459,9 +460,9 @@ static uintptr_t NOINLINE jl_object_id__cold(jl_datatype_t *dt, jl_value_t *v) J
         return m->hash;
     }
     if (dt->name->mutabl) {
-        if(mmtk_object_is_managed_by_mmtk(v)) {
-            mmtk_pin_object(v);
-        }
+        // FIXME: Pinning objects that get hashed
+        // until we implement address space hashing.
+        mmtk_pin_object(v);
         return inthash((uintptr_t)v);
     }
     return immut_id_(dt, v, dt->hash);

src/datatype.c

@@ -65,6 +65,8 @@ JL_DLLEXPORT jl_typename_t *jl_new_typename_in(jl_sym_t *name, jl_module_t *modu
     jl_typename_t *tn =
         (jl_typename_t*)jl_gc_alloc(ct->ptls, sizeof(jl_typename_t),
                                     jl_typename_type);
+    // Typenames should be pinned since they are used as metadata, and are
+    // read during scan_object
     mmtk_pin_object(tn);
     tn->name = name;
     tn->module = module;
@@ -97,6 +99,8 @@ jl_datatype_t *jl_new_uninitialized_datatype(void)
 {
     jl_task_t *ct = jl_current_task;
     jl_datatype_t *t = (jl_datatype_t*)jl_gc_alloc(ct->ptls, sizeof(jl_datatype_t), jl_datatype_type);
+    // Types should be pinned since they are used as metadata, and are
+    // read during scan_object
     mmtk_pin_object(t);
     jl_set_typetagof(t, jl_datatype_tag, 0);
     t->hash = 0;

src/julia.h

@@ -9,10 +9,10 @@ extern "C" {
 
 extern int mmtk_object_is_managed_by_mmtk(void* addr);
 extern unsigned char mmtk_pin_object(void* obj);
+// FIXME: Pinning objects that get hashed in the ptrhash table
+// until we implement address space hashing.
 #define PTRHASH_PIN(key)                        \
-    if (mmtk_object_is_managed_by_mmtk(key)) {  \
         mmtk_pin_object(key);                   \
-    }                                           \
 
 #ifdef __cplusplus
 }

src/julia_internal.h

@@ -533,6 +533,10 @@ typedef void jl_gc_tracked_buffer_t; // For the benefit of the static analyzer
 STATIC_INLINE jl_gc_tracked_buffer_t *jl_gc_alloc_buf(jl_ptls_t ptls, size_t sz)
 {
     jl_gc_tracked_buffer_t *buf = jl_gc_alloc(ptls, sz, (void*)jl_buff_tag);
+    // For array objects with an owner point (a->flags.how == 3), we would need to
+    // introspect the object to update the a->data field. To avoid doing that and
+    // making scan_object much more complex we simply enforce that both owner and
+    // buffers are always pinned
     mmtk_pin_object(buf);
     return buf;
 }

src/mmtk-gc.c

@@ -432,6 +432,7 @@ jl_value_t *jl_gc_realloc_string(jl_value_t *s, size_t sz)
     jl_value_t *snew = jl_alloc_string(sz);
     memcpy(jl_string_data(snew), jl_string_data(s), sz <= len ? sz : len);
     if(mmtk_is_pinned(s)) {
+        // if the source string was pinned, we also pin the new one
         mmtk_pin_object(snew);
     }
     return snew;