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