Merge pull request #13995 from JuliaLang/yyc/gc/eager-finalize

Free objects with finalizer more eagerly

src/gc-debug.c

@@ -171,8 +171,10 @@ static void gc_verify_track(void)
         jl_printf(JL_STDERR, "Now looking for %p =======\n", lostval);
         clear_mark(GC_CLEAN);
         pre_mark();
-        post_mark(&finalizer_list, 1);
-        post_mark(&finalizer_list_marked, 1);
+        gc_mark_object_list(&to_finalize, 0);
+        gc_mark_object_list(&finalizer_list, 0);
+        gc_mark_object_list(&finalizer_list_marked, 0);
+        visit_mark_stack();
         if (lostval_parents.len == 0) {
             jl_printf(JL_STDERR, "Could not find the missing link. We missed a toplevel root. This is odd.\n");
             break;
@@ -212,8 +214,10 @@ void gc_verify(void)
     clear_mark(GC_CLEAN);
     gc_verifying = 1;
     pre_mark();
-    post_mark(&finalizer_list, 1);
-    post_mark(&finalizer_list_marked, 1);
+    gc_mark_object_list(&to_finalize, 0);
+    gc_mark_object_list(&finalizer_list, 0);
+    gc_mark_object_list(&finalizer_list_marked, 0);
+    visit_mark_stack();
     int clean_len = bits_save[GC_CLEAN].len;
     for(int i = 0; i < clean_len + bits_save[GC_QUEUED].len; i++) {
         gcval_t *v = (gcval_t*)bits_save[i >= clean_len ? GC_QUEUED : GC_CLEAN].items[i >= clean_len ? i - clean_len : i];

src/gc.c

@@ -262,6 +262,12 @@ static size_t max_collect_interval =  500000000UL;
 
 // global variables for GC stats
 
+// Resetting the object to a young object, this is used when marking the
+// finalizer list to collect them the next time because the object is very
+// likely dead. This also won't break the GC invariance since these objects
+// are not reachable from anywhere else.
+static int mark_reset_age = 0;
+
 /*
  * The state transition looks like :
  *
@@ -354,18 +360,22 @@ static inline int gc_setmark_big(void *o, int mark_mode)
     assert(find_region(o,1) == NULL);
     bigval_t *hdr = bigval_header(o);
     int bits = gc_bits(o);
-    if (bits == GC_QUEUED || bits == GC_MARKED)
-        mark_mode = GC_MARKED;
-    if ((mark_mode == GC_MARKED) & (bits != GC_MARKED)) {
-        // Move hdr from big_objects list to big_objects_marked list
-        *hdr->prev = hdr->next;
-        if (hdr->next)
-            hdr->next->prev = hdr->prev;
-        hdr->next = big_objects_marked;
-        hdr->prev = &big_objects_marked;
-        if (big_objects_marked)
-            big_objects_marked->prev = &hdr->next;
-        big_objects_marked = hdr;
+    if (mark_reset_age && !(bits & GC_MARKED)) {
+        // Reset the object as if it was just allocated
+        hdr->age = 0;
+        gc_big_object_unlink(hdr);
+        gc_big_object_link(hdr, &jl_thread_heap.big_objects);
+        bits = GC_CLEAN;
+        mark_mode = GC_MARKED_NOESC;
+    }
+    else {
+        if (bits == GC_QUEUED || bits == GC_MARKED)
+            mark_mode = GC_MARKED;
+        if ((mark_mode == GC_MARKED) & (bits != GC_MARKED)) {
+            // Move hdr from big_objects list to big_objects_marked list
+            gc_big_object_unlink(hdr);
+            gc_big_object_link(hdr, &big_objects_marked);
+        }
     }
     if (!(bits & GC_MARKED)) {
         if (mark_mode == GC_MARKED)
@@ -391,7 +401,17 @@ static inline int gc_setmark_pool(void *o, int mark_mode)
     }
     jl_gc_pagemeta_t *page = page_metadata(o);
     int bits = gc_bits(o);
-    if (bits == GC_QUEUED || bits == GC_MARKED) {
+    if (mark_reset_age && !(bits & GC_MARKED)) {
+        // Reset the object as if it was just allocated
+        bits = GC_CLEAN;
+        mark_mode = GC_MARKED_NOESC;
+        page->has_young = 1;
+        char *page_begin = gc_page_data(o) + GC_PAGE_OFFSET;
+        int obj_id = (((char*)o) - page_begin) / page->osize;
+        uint8_t *ages = page->ages + obj_id / 8;
+        *ages &= ~(1 << (obj_id % 8));
+    }
+    else if (bits == GC_QUEUED || bits == GC_MARKED) {
         mark_mode = GC_MARKED;
     }
     if (!(bits & GC_MARKED)) {
@@ -509,11 +529,7 @@ static NOINLINE void *alloc_big(size_t sz)
     v->sz = allocsz;
     v->flags = 0;
     v->age = 0;
-    v->next = jl_thread_heap.big_objects;
-    v->prev = &jl_thread_heap.big_objects;
-    if (v->next)
-        v->next->prev = &v->next;
-    jl_thread_heap.big_objects = v;
+    gc_big_object_link(v, &jl_thread_heap.big_objects);
     return (void*)&v->header;
 }
 
@@ -1202,6 +1218,12 @@ NOINLINE static void gc_mark_task(jl_task_t *ta, int d)
     gc_mark_task_stack(ta, d);
 }
 
+void gc_mark_object_list(arraylist_t *list, size_t start)
+{
+    for (size_t i = start;i < list->len;i++) {
+        gc_push_root(list->items[i], 0);
+    }
+}
 
 // for chasing down unwanted references
 /*
@@ -1389,7 +1411,7 @@ static int push_root(jl_value_t *v, int d, int bits)
     return bits;
 }
 
-static void visit_mark_stack(void)
+void visit_mark_stack(void)
 {
     while (mark_sp > 0 && !should_timeout()) {
         jl_value_t *v = mark_stack[--mark_sp];
@@ -1434,11 +1456,6 @@ void pre_mark(void)
     gc_push_root(jl_anytuple_type_type, 0);
     gc_push_root(jl_ANY_flag, 0);
 
-    // objects currently being finalized
-    for(i=0; i < to_finalize.len; i++) {
-        gc_push_root(to_finalize.items[i], 0);
-    }
-
     jl_mark_box_caches();
     //gc_push_root(jl_unprotect_stack_func, 0);
     gc_push_root(jl_typetype_type, 0);
@@ -1453,16 +1470,16 @@ void pre_mark(void)
 
 // find unmarked objects that need to be finalized from the finalizer list "list".
 // this must happen last in the mark phase.
-// if dryrun == 1, it does not schedule any actual finalization and only marks finalizers
-void post_mark(arraylist_t *list, int dryrun)
+static void post_mark(arraylist_t *list)
 {
     for(size_t i=0; i < list->len; i+=2) {
         jl_value_t *v = (jl_value_t*)list->items[i];
         jl_value_t *fin = (jl_value_t*)list->items[i+1];
         int isfreed = !gc_marked(jl_astaggedvalue(v));
-        gc_push_root(fin, 0);
-        int isold = list == &finalizer_list && gc_bits(jl_astaggedvalue(v)) == GC_MARKED && gc_bits(jl_astaggedvalue(fin)) == GC_MARKED;
-        if (!dryrun && (isfreed || isold)) {
+        int isold = (list != &finalizer_list_marked &&
+                     gc_bits(jl_astaggedvalue(v)) == GC_MARKED &&
+                     gc_bits(jl_astaggedvalue(fin)) == GC_MARKED);
+        if (isfreed || isold) {
             // remove from this list
             if (i < list->len - 2) {
                 list->items[i] = list->items[list->len-2];
@@ -1475,19 +1492,19 @@ void post_mark(arraylist_t *list, int dryrun)
             // schedule finalizer or execute right away if it is not julia code
             if (gc_typeof(fin) == (jl_value_t*)jl_voidpointer_type) {
                 void *p = jl_unbox_voidpointer(fin);
-                if (!dryrun && p)
+                if (p)
                     ((void (*)(void*))p)(jl_data_ptr(v));
                 continue;
             }
-            gc_push_root(v, 0);
-            if (!dryrun) schedule_finalization(v, fin);
+            schedule_finalization(v, fin);
         }
-        if (!dryrun && isold) {
+        if (isold) {
+            // The caller relies on the new objects to be pushed to the end of
+            // the list!!
             arraylist_push(&finalizer_list_marked, v);
             arraylist_push(&finalizer_list_marked, fin);
         }
     }
-    visit_mark_stack();
 }
 
 // collector entry point and control
@@ -1594,9 +1611,28 @@ static void _jl_gc_collect(int full, char *stack_hi)
     int64_t actual_allocd = gc_num.since_sweep;
     // marking is over
     // 4. check for objects to finalize
-    post_mark(&finalizer_list, 0);
-    if (prev_sweep_full)
-        post_mark(&finalizer_list_marked, 0);
+    // Record the length of the marked list since we need to
+    // mark the object moved to the marked list from the
+    // `finalizer_list` by `post_mark`
+    size_t orig_marked_len = finalizer_list_marked.len;
+    post_mark(&finalizer_list);
+    if (prev_sweep_full) {
+        post_mark(&finalizer_list_marked);
+        orig_marked_len = 0;
+    }
+    gc_mark_object_list(&finalizer_list, 0);
+    gc_mark_object_list(&finalizer_list_marked, orig_marked_len);
+    // "Flush" the mark stack before flipping the reset_age bit
+    // so that the objects are not incorrectly resetted.
+    visit_mark_stack();
+    mark_reset_age = 1;
+    // Reset the age and old bit for any unmarked objects referenced by the
+    // `to_finalize` list. These objects are only reachable from this list
+    // and should not be referenced by any old objects so this won't break
+    // the GC invariant.
+    gc_mark_object_list(&to_finalize, 0);
+    visit_mark_stack();
+    mark_reset_age = 0;
     gc_settime_postmark_end();
 
     int64_t live_sz_ub = live_bytes + actual_allocd;

src/gc.h

@@ -253,8 +253,26 @@ STATIC_INLINE jl_gc_pagemeta_t *page_metadata(void *data)
     return &r->meta[pg_idx];
 }
 
+STATIC_INLINE void gc_big_object_unlink(const bigval_t *hdr)
+{
+    *hdr->prev = hdr->next;
+    if (hdr->next) {
+        hdr->next->prev = hdr->prev;
+    }
+}
+
+STATIC_INLINE void gc_big_object_link(bigval_t *hdr, bigval_t **list)
+{
+    hdr->next = *list;
+    hdr->prev = list;
+    if (*list)
+        (*list)->prev = &hdr->next;
+    *list = hdr;
+}
+
 void pre_mark(void);
-void post_mark(arraylist_t *list, int dryrun);
+void gc_mark_object_list(arraylist_t *list, size_t start);
+void visit_mark_stack(void);
 void gc_debug_init(void);
 
 #define jl_thread_heap (jl_get_ptls_states()->heap)

test/core.jl

@@ -1895,7 +1895,7 @@ type ObjMember
     member::DateRange6387
 end
 
-obj = ObjMember(DateRange6387{Int64}())
+obj6387 = ObjMember(DateRange6387{Int64}())
 
 function v6387{T}(r::Range{T})
     a = Array{T}(1)
@@ -1908,7 +1908,7 @@ function day_in(obj::ObjMember)
     @test isa(x, Vector{Date6387{Int64}})
     @test isa(x[1], Date6387{Int64})
 end
-day_in(obj)
+day_in(obj6387)
 
 # issue #6784
 @test ndims(Array{Array{Float64}}(3,5)) == 2
@@ -3788,6 +3788,31 @@ let ary = Vector{Any}(10)
     end
 end
 
+# check if we can run multiple finalizers at the same time
+# Use a `@noinline` function to make sure the inefficient gc root generation
+# doesn't keep the object alive.
+@noinline function create_dead_object13995(finalized)
+    obj = Ref(1)
+    finalizer(obj, (x)->(finalized[1] = true))
+    finalizer(obj, (x)->(finalized[2] = true))
+    finalizer(obj, (x)->(finalized[3] = true))
+    finalizer(obj, (x)->(finalized[4] = true))
+    nothing
+end
+# disable GC to make sure no collection/promotion happens
+# when we are constructing the objects
+let gc_enabled13995 = gc_enable(false)
+    finalized13995 = [false, false, false, false]
+    create_dead_object13995(finalized13995)
+    gc_enable(true)
+    # obj is unreachable and young, a single young gc should collect it
+    # and trigger all the finalizers.
+    gc(false)
+    gc_enable(false)
+    @test finalized13995 == [true, true, true, true]
+    gc_enable(gc_enabled13995)
+end
+
 # issue #15283
 j15283 = 0
 let