From 9388a826427f62b09d4abcd638f40e22bf4176e5 Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Fri, 26 Feb 2016 14:21:47 -0500 Subject: [PATCH 1/3] Mark to_finalize after marking the finalizer list --- src/gc-debug.c | 2 ++ src/gc.c | 28 ++++++++++------------------ src/gc.h | 18 ++++++++++++++++++ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/gc-debug.c b/src/gc-debug.c index 2c7cb3d8f7647..e017349d6f4d4 100644 --- a/src/gc-debug.c +++ b/src/gc-debug.c @@ -171,6 +171,7 @@ static void gc_verify_track(void) jl_printf(JL_STDERR, "Now looking for %p =======\n", lostval); clear_mark(GC_CLEAN); pre_mark(); + gc_mark_object_list(&to_finalize); post_mark(&finalizer_list, 1); post_mark(&finalizer_list_marked, 1); if (lostval_parents.len == 0) { @@ -212,6 +213,7 @@ void gc_verify(void) clear_mark(GC_CLEAN); gc_verifying = 1; pre_mark(); + gc_mark_object_list(&to_finalize); post_mark(&finalizer_list, 1); post_mark(&finalizer_list_marked, 1); int clean_len = bits_save[GC_CLEAN].len; diff --git a/src/gc.c b/src/gc.c index bf397dc504279..659fb0e951580 100644 --- a/src/gc.c +++ b/src/gc.c @@ -358,14 +358,8 @@ static inline int gc_setmark_big(void *o, int mark_mode) 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; + gc_big_object_unlink(hdr); + gc_big_object_link(hdr, &big_objects_marked); } if (!(bits & GC_MARKED)) { if (mark_mode == GC_MARKED) @@ -509,11 +503,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 +1192,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) +{ + for (size_t i = 0;i < list->len;i++) { + gc_push_root(list->items[i], 0); + } +} // for chasing down unwanted references /* @@ -1434,11 +1430,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); @@ -1597,6 +1588,7 @@ static void _jl_gc_collect(int full, char *stack_hi) post_mark(&finalizer_list, 0); if (prev_sweep_full) post_mark(&finalizer_list_marked, 0); + gc_mark_object_list(&to_finalize); gc_settime_postmark_end(); int64_t live_sz_ub = live_bytes + actual_allocd; diff --git a/src/gc.h b/src/gc.h index 905b47e2899cb..614ec02225bee 100644 --- a/src/gc.h +++ b/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); void gc_debug_init(void); #define jl_thread_heap (jl_get_ptls_states()->heap) From f5ac1f240f4d5946365e9cab24442da10cfaf3bc Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Fri, 26 Feb 2016 17:56:09 -0500 Subject: [PATCH 2/3] Free objects with finalizer more eagerly * Run all the finalizers at the same time in a dead reference tree * Reset objects that are only reachable from finalizer list as young and clean so that they can be collect during next quick GC without being promoted to old gen. Closes #14127 --- src/gc-debug.c | 14 ++++---- src/gc.c | 90 +++++++++++++++++++++++++++++++++++++------------- src/gc.h | 4 +-- 3 files changed, 77 insertions(+), 31 deletions(-) diff --git a/src/gc-debug.c b/src/gc-debug.c index e017349d6f4d4..55d2f63b1c4fe 100644 --- a/src/gc-debug.c +++ b/src/gc-debug.c @@ -171,9 +171,10 @@ static void gc_verify_track(void) jl_printf(JL_STDERR, "Now looking for %p =======\n", lostval); clear_mark(GC_CLEAN); pre_mark(); - gc_mark_object_list(&to_finalize); - 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; @@ -213,9 +214,10 @@ void gc_verify(void) clear_mark(GC_CLEAN); gc_verifying = 1; pre_mark(); - gc_mark_object_list(&to_finalize); - 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]; diff --git a/src/gc.c b/src/gc.c index 659fb0e951580..5139144d253fa 100644 --- a/src/gc.c +++ b/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,12 +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 + 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, &big_objects_marked); + 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) @@ -385,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)) { @@ -1192,9 +1218,9 @@ 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) +void gc_mark_object_list(arraylist_t *list, size_t start) { - for (size_t i = 0;i < list->len;i++) { + for (size_t i = start;i < list->len;i++) { gc_push_root(list->items[i], 0); } } @@ -1385,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]; @@ -1444,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]; @@ -1466,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 @@ -1585,10 +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); - gc_mark_object_list(&to_finalize); + // 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; diff --git a/src/gc.h b/src/gc.h index 614ec02225bee..42ef9d35c12e8 100644 --- a/src/gc.h +++ b/src/gc.h @@ -271,8 +271,8 @@ STATIC_INLINE void gc_big_object_link(bigval_t *hdr, bigval_t **list) } void pre_mark(void); -void post_mark(arraylist_t *list, int dryrun); -void gc_mark_object_list(arraylist_t *list); +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) From 7042e36fb9cca95423c37498d0f3a4313ad7be13 Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Sat, 14 Nov 2015 21:00:09 -0500 Subject: [PATCH 3/3] Add test for multiple finalizers --- test/core.jl | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/test/core.jl b/test/core.jl index a7a3433bd5d40..188165432b3c0 100644 --- a/test/core.jl +++ b/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