ruby-changes:63404
From: Koichi <ko1@a...>
Date: Wed, 21 Oct 2020 16:15:38 +0900 (JST)
Subject: [ruby-changes:63404] b59077eecf (master): Ractor-safe rb_objspace_reachable_objects_from
https://git.ruby-lang.org/ruby.git/commit/?id=b59077eecf From b59077eecf912a16efefc0256f6e94a000ce3888 Mon Sep 17 00:00:00 2001 From: Koichi Sasada <ko1@a...> Date: Wed, 21 Oct 2020 13:53:46 +0900 Subject: Ractor-safe rb_objspace_reachable_objects_from rb_objspace_reachable_objects_from(obj) is used to traverse all reachable objects from obj. This function modify objspace but it is not ractor-safe (thread-safe). This patch fix the problem. Strategy: (1) call GC mark process during_gc (2) call Ractor-local custom mark func when !during_gc diff --git a/gc.c b/gc.c index f72d76e..6c3fc50 100644 --- a/gc.c +++ b/gc.c @@ -690,11 +690,6 @@ typedef struct rb_objspace { https://github.com/ruby/ruby/blob/trunk/gc.c#L690 rb_atomic_t finalizing; } atomic_flags; - struct mark_func_data_struct { - void *data; - void (*mark_func)(VALUE v, void *data); - } *mark_func_data; - mark_stack_t mark_stack; size_t marked_slots; @@ -1072,12 +1067,6 @@ static inline void gc_prof_set_heap_info(rb_objspace_t *); https://github.com/ruby/ruby/blob/trunk/gc.c#L1067 PRINTF_ARGS(static void gc_report_body(int level, rb_objspace_t *objspace, const char *fmt, ...), 3, 4); static const char *obj_info(VALUE obj); -#define PUSH_MARK_FUNC_DATA(v) do { \ - struct mark_func_data_struct *prev_mark_func_data = objspace->mark_func_data; \ - objspace->mark_func_data = (v); - -#define POP_MARK_FUNC_DATA() objspace->mark_func_data = prev_mark_func_data;} while (0) - /* * 1 - TSC (H/W Time Stamp Counter) * 2 - getrusage @@ -5138,7 +5127,7 @@ mark_hash(rb_objspace_t *objspace, VALUE hash) https://github.com/ruby/ruby/blob/trunk/gc.c#L5127 } if (RHASH_AR_TABLE_P(hash)) { - if (objspace->mark_func_data == NULL && RHASH_TRANSIENT_P(hash)) { + if (LIKELY(during_gc) && RHASH_TRANSIENT_P(hash)) { rb_transient_heap_mark(hash, RHASH_AR_TABLE(hash)); } } @@ -5453,11 +5442,12 @@ gc_aging(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L5442 } NOINLINE(static void gc_mark_ptr(rb_objspace_t *objspace, VALUE obj)); +static void reachable_objects_from_callback(VALUE obj); static void gc_mark_ptr(rb_objspace_t *objspace, VALUE obj) { - if (LIKELY(objspace->mark_func_data == NULL)) { + if (LIKELY(during_gc)) { rgengc_check_relation(objspace, obj); if (!gc_mark_set(objspace, obj)) return; /* already marked */ if (RB_TYPE_P(obj, T_NONE)) { @@ -5468,7 +5458,7 @@ gc_mark_ptr(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L5458 gc_grey(objspace, obj); } else { - objspace->mark_func_data->mark_func(obj, objspace->mark_func_data->data); + reachable_objects_from_callback(obj); } } @@ -5477,7 +5467,7 @@ gc_pin(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L5467 { GC_ASSERT(is_markable_object(objspace, obj)); if (UNLIKELY(objspace->flags.during_compacting)) { - if (LIKELY(objspace->mark_func_data == NULL)) { + if (LIKELY(during_gc)) { MARK_IN_BITMAP(GET_HEAP_PINNED_BITS(obj), obj); } } @@ -5678,7 +5668,7 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L5668 gc_mark(objspace, ptr[i]); } - if (objspace->mark_func_data == NULL) { + if (LIKELY(during_gc)) { if (!FL_TEST_RAW(obj, RARRAY_EMBED_FLAG) && RARRAY_TRANSIENT_P(obj)) { rb_transient_heap_mark(obj, ptr); @@ -5719,7 +5709,7 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L5709 gc_mark(objspace, ptr[i]); } - if (objspace->mark_func_data == NULL && + if (LIKELY(during_gc) && ROBJ_TRANSIENT_P(obj)) { rb_transient_heap_mark(obj, ptr); } @@ -5770,7 +5760,7 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L5760 gc_mark(objspace, ptr[i]); } - if (objspace->mark_func_data == NULL && + if (LIKELY(during_gc) && RSTRUCT_TRANSIENT_P(obj)) { rb_transient_heap_mark(obj, ptr); } @@ -6429,7 +6419,7 @@ gc_verify_internal_consistency_m(VALUE dummy) https://github.com/ruby/ruby/blob/trunk/gc.c#L6419 } static void -gc_verify_internal_consistency(rb_objspace_t *objspace) +gc_verify_internal_consistency_(rb_objspace_t *objspace) { struct verify_internal_consistency_struct data = {0}; @@ -6502,6 +6492,17 @@ gc_verify_internal_consistency(rb_objspace_t *objspace) https://github.com/ruby/ruby/blob/trunk/gc.c#L6492 gc_report(5, objspace, "gc_verify_internal_consistency: OK\n"); } +static void +gc_verify_internal_consistency(rb_objspace_t *objspace) +{ + unsigned int prev_during_gc = during_gc; + during_gc = FALSE; // stop gc here + { + gc_verify_internal_consistency_(objspace); + } + during_gc = prev_during_gc; +} + void rb_gc_verify_internal_consistency(void) { @@ -6780,35 +6781,31 @@ gc_marks_continue(rb_objspace_t *objspace, rb_heap_t *heap) https://github.com/ruby/ruby/blob/trunk/gc.c#L6781 unsigned int lock_lev; gc_enter(objspace, "marks_continue", &lock_lev); - PUSH_MARK_FUNC_DATA(NULL); - { - int slots = 0; - const char *from; + int slots = 0; + const char *from; - if (heap->pooled_pages) { - while (heap->pooled_pages && slots < HEAP_PAGE_OBJ_LIMIT) { - struct heap_page *page = heap_move_pooled_pages_to_free_pages(heap); - slots += page->free_slots; - } - from = "pooled-pages"; - } - else if (heap_increment(objspace, heap)) { - slots = heap->free_pages->free_slots; - from = "incremented-pages"; - } + if (heap->pooled_pages) { + while (heap->pooled_pages && slots < HEAP_PAGE_OBJ_LIMIT) { + struct heap_page *page = heap_move_pooled_pages_to_free_pages(heap); + slots += page->free_slots; + } + from = "pooled-pages"; + } + else if (heap_increment(objspace, heap)) { + slots = heap->free_pages->free_slots; + from = "incremented-pages"; + } - if (slots > 0) { - gc_report(2, objspace, "gc_marks_continue: provide %d slots from %s.\n", - slots, from); - gc_marks_step(objspace, objspace->rincgc.step_slots); - } - else { - gc_report(2, objspace, "gc_marks_continue: no more pooled pages (stack depth: %"PRIdSIZE").\n", - mark_stack_size(&objspace->mark_stack)); - gc_marks_rest(objspace); - } + if (slots > 0) { + gc_report(2, objspace, "gc_marks_continue: provide %d slots from %s.\n", + slots, from); + gc_marks_step(objspace, objspace->rincgc.step_slots); + } + else { + gc_report(2, objspace, "gc_marks_continue: no more pooled pages (stack depth: %"PRIdSIZE").\n", + mark_stack_size(&objspace->mark_stack)); + gc_marks_rest(objspace); } - POP_MARK_FUNC_DATA(); gc_exit(objspace, "marks_continue", &lock_lev); #endif @@ -6819,23 +6816,19 @@ gc_marks(rb_objspace_t *objspace, int full_mark) https://github.com/ruby/ruby/blob/trunk/gc.c#L6816 { gc_prof_mark_timer_start(objspace); - PUSH_MARK_FUNC_DATA(NULL); - { - /* setup marking */ + /* setup marking */ - gc_marks_start(objspace, full_mark); - if (!is_incremental_marking(objspace)) { - gc_marks_rest(objspace); - } + gc_marks_start(objspace, full_mark); + if (!is_incremental_marking(objspace)) { + gc_marks_rest(objspace); + } #if RGENGC_PROFILE > 0 - if (gc_prof_record(objspace)) { - gc_profile_record *record = gc_prof_record(objspace); - record->old_objects = objspace->rgengc.old_objects; - } -#endif + if (gc_prof_record(objspace)) { + gc_profile_record *record = gc_prof_record(objspace); + record->old_objects = objspace->rgengc.old_objects; } - POP_MARK_FUNC_DATA(); +#endif gc_prof_mark_timer_stop(objspace); } @@ -7677,10 +7670,8 @@ gc_rest(rb_objspace_t *objspace) https://github.com/ruby/ruby/blob/trunk/gc.c#L7670 if (RGENGC_CHECK_MODE >= 2) gc_verify_internal_consistency(objspace); if (is_incremental_marking(objspace)) { - PUSH_MARK_FUNC_DATA(NULL); - gc_marks_rest(objspace); - POP_MARK_FUNC_DATA(); - } + gc_marks_rest(objspace); + } if (is_lazy_sweeping(heap_eden)) { gc_sweep_rest(objspace); } @@ -9805,18 +9796,30 @@ ruby_gc_set_params(void) https://github.com/ruby/ruby/blob/trunk/gc.c#L9796 #endif } +static void +reachable_objects_from_callback(VALUE obj) +{ + rb_ractor_t *cr = GET_RACTOR(); + cr->mfd->mark_func(obj, cr->mfd->data); +} + void rb_objspace_reachable_objects_from(VALUE obj, void (func)(VALUE, void *), void *data) { rb_objspace_t *objspace = &rb_objspace; + if (during_gc) rb_bug("rb_objspace_reachable_objects_from() is not supported while during_gc == true"); + if (is_markable_object(objspace, obj)) { - struct mark_func_data_struct mfd; - mfd.mark_func = func; - mfd.data = data; - PUSH_MARK_FUNC_DATA(&mfd); + rb_ractor_t *cr = GET_RACTOR(); + struct gc_mark_func_data_struct mfd = { + .mark_func = func, + .data = data, + }, *prev_mfd = cr->mfd; + + cr->mfd = &mfd; gc_mark_children(objspace, obj); - POP_MARK_FUNC_DATA(); + cr->mfd = prev_mfd; } } @@ -9843,18 +9846,21 @@ rb_objspace_reachable_objects_from_root(void (func)(const char *category, VALUE, https://github.com/ruby/ruby/blob/trunk/gc.c#L9846 static void objspace_reachable_objects_from_root(rb_objspace_t *objspace, void (func)(const char *category, VALUE, void *), void *passing_data) { - struct root_objects_data data; - struct mark_func_data_struct mfd; - - d (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/