Skip to content

Commit

Permalink
Make gc_mark_ptr have a single responsibility
Browse files Browse the repository at this point in the history
Currently the GC's marking functions functions (gc_mark, gc_mark_ptr,
gc_mark_children) are responsible for both marking and object tracing.
This becomes awkward when we need to use object tracing for non-GC
related purposes (such as dumping the heap).

The current implementation relies on `gc_mark_ptr` having multiple
responsibilites depending on whether it's being called during GC or
during mutator work - it switches based on `objspace->flags.during_gc`.

If that flag is set it runs marking, and if not it passes the pointer in
question to `reachable_objects_from_callback`, which then looks at the
current ractor for a function pointer to execute with the pointer as an
argument.

This PR removes the conditional from `gc_mark_ptr`. Making it
unconditionally mark the pointer.

It then pre-configures each ractor to use `gc_mark_ptr` as it's default
mark function pointer, and replaces previous calls to `gc_mark_ptr` with
calls to `reachable_objects_from_callback` - essentially unifying the
two approaces to object tracing - In order to walk the object graph we
now need to do the same steps:

1. make sure there is a function pointer attached to the ractor
2. use the iterator function `reachable_objects_from_callback`

One of the challenges involved in walking the object graph, is that we
cannot change the api for marking T_DATA objects. The nice thing about
the original approach, is that we didn't have to - all marking callbacks
in C extension objects will eventually use gc_mark_ptr - because it's
used by `rb_gc_mark` which is part of the public API.

This new approach also doesn't require changing the public API. We
implement `rb_gc_mark` on top of `reachable_objects_from_callback` and
no user facing changes have to be made to any code in C extensions.

This new approach is the first step in separating gc marking from object
graph traversal. This is necessary for us to start implementing
alternate GC's so that we can implement custom marking and tracing
functions independantly.
  • Loading branch information
eightbitraptor committed Jul 20, 2023
1 parent a712774 commit 3be4bf8
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 47 deletions.
92 changes: 50 additions & 42 deletions gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -6842,37 +6842,33 @@ gc_aging(rb_objspace_t *objspace, VALUE obj)
objspace->marked_slots++;
}

NOINLINE(static void gc_mark_ptr(rb_objspace_t *objspace, VALUE obj));
NOINLINE(static void gc_mark_ptr(VALUE obj, void *data));
static void reachable_objects_from_callback(VALUE obj);

static void
gc_mark_ptr(rb_objspace_t *objspace, VALUE obj)
gc_mark_ptr(VALUE obj, void *data)
{
if (LIKELY(during_gc)) {
rgengc_check_relation(objspace, obj);
if (!gc_mark_set(objspace, obj)) return; /* already marked */
rb_objspace_t *objspace = (rb_objspace_t *)data;
rgengc_check_relation(objspace, obj);
if (!gc_mark_set(objspace, obj)) return; /* already marked */

if (0) { // for debug GC marking miss
if (objspace->rgengc.parent_object) {
RUBY_DEBUG_LOG("%p (%s) parent:%p (%s)",
(void *)obj, obj_type_name(obj),
(void *)objspace->rgengc.parent_object, obj_type_name(objspace->rgengc.parent_object));
}
else {
RUBY_DEBUG_LOG("%p (%s)", (void *)obj, obj_type_name(obj));
}
if (0) { // for debug GC marking miss
if (objspace->rgengc.parent_object) {
RUBY_DEBUG_LOG("%p (%s) parent:%p (%s)",
(void *)obj, obj_type_name(obj),
(void *)objspace->rgengc.parent_object, obj_type_name(objspace->rgengc.parent_object));
}

if (UNLIKELY(RB_TYPE_P(obj, T_NONE))) {
rp(obj);
rb_bug("try to mark T_NONE object"); /* check here will help debugging */
else {
RUBY_DEBUG_LOG("%p (%s)", (void *)obj, obj_type_name(obj));
}
gc_aging(objspace, obj);
gc_grey(objspace, obj);
}
else {
reachable_objects_from_callback(obj);

if (UNLIKELY(RB_TYPE_P(obj, T_NONE))) {
rp(obj);
rb_bug("try to mark T_NONE object"); /* check here will help debugging */
}
gc_aging(objspace, obj);
gc_grey(objspace, obj);
}

static inline void
Expand All @@ -6891,14 +6887,14 @@ gc_mark_and_pin(rb_objspace_t *objspace, VALUE obj)
{
if (!is_markable_object(obj)) return;
gc_pin(objspace, obj);
gc_mark_ptr(objspace, obj);
reachable_objects_from_callback(obj);
}

static inline void
gc_mark(rb_objspace_t *objspace, VALUE obj)
{
if (!is_markable_object(obj)) return;
gc_mark_ptr(objspace, obj);
reachable_objects_from_callback(obj);
}

void
Expand Down Expand Up @@ -6926,7 +6922,7 @@ rb_gc_mark_and_move(VALUE *ptr)
*ptr = rb_gc_location(*ptr);
}
else {
gc_mark_ptr(objspace, *ptr);
reachable_objects_from_callback(*ptr);
}
}

Expand Down Expand Up @@ -7074,7 +7070,6 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj)
{
register RVALUE *any = RANY(obj);
gc_mark_set_parent(objspace, obj);

if (FL_TEST(obj, FL_EXIVAR)) {
rb_mark_and_update_generic_ivar(obj);
}
Expand Down Expand Up @@ -7289,6 +7284,7 @@ gc_mark_stacked_objects(rb_objspace_t *objspace, int incremental, size_t count)
if (RGENGC_CHECK_MODE && !RVALUE_MARKED(obj)) {
rb_bug("gc_mark_stacked_objects: %s is not marked.", obj_info(obj));
}

gc_mark_children(objspace, obj);

if (incremental) {
Expand Down Expand Up @@ -7375,16 +7371,16 @@ gc_mark_roots(rb_objspace_t *objspace, const char **categoryp)
objspace->rgengc.parent_object = Qfalse;

#if PRINT_ROOT_TICKS
#define MARK_CHECKPOINT_PRINT_TICK(category) do { \
if (prev_category) { \
tick_t t = tick(); \
mark_ticks[tick_count] = t - start_tick; \
mark_ticks_categories[tick_count] = prev_category; \
tick_count++; \
} \
prev_category = category; \
start_tick = tick(); \
} while (0)
#define MARK_CHECKPOINT_PRINT_TICK(category) do { \
if (prev_category) { \
tick_t t = tick(); \
mark_ticks[tick_count] = t - start_tick; \
mark_ticks_categories[tick_count] = prev_category; \
tick_count++; \
} \
prev_category = category; \
start_tick = tick(); \
} while (0)
#else /* PRINT_ROOT_TICKS */
#define MARK_CHECKPOINT_PRINT_TICK(category)
#endif
Expand Down Expand Up @@ -8536,7 +8532,6 @@ gc_marks(rb_objspace_t *objspace, int full_mark)
bool marking_finished = false;

/* setup marking */

gc_marks_start(objspace, full_mark);
if (!is_incremental_marking(objspace)) {
gc_marks_rest(objspace);
Expand Down Expand Up @@ -9210,6 +9205,10 @@ gc_start(rb_objspace_t *objspace, unsigned int reason)
/* reason may be clobbered, later, so keep set immediate_sweep here */
objspace->flags.immediate_sweep = !!(reason & GPR_FLAG_IMMEDIATE_SWEEP);

rb_ractor_t *cr = GET_RACTOR();
struct gc_mark_func_data_struct prev_mfd = cr->mfd;
rb_ractor_init_mfd(cr);

/* Explicitly enable compaction (GC.compact) */
if (do_full_mark && ruby_enable_autocompact) {
objspace->flags.during_compacting = TRUE;
Expand Down Expand Up @@ -9316,6 +9315,8 @@ gc_start(rb_objspace_t *objspace, unsigned int reason)

gc_exit(objspace, gc_enter_event_start, &lock_lev);
return TRUE;

cr->mfd = prev_mfd;
}

static void
Expand Down Expand Up @@ -11671,7 +11672,7 @@ static void
reachable_objects_from_callback(VALUE obj)
{
rb_ractor_t *cr = GET_RACTOR();
cr->mfd->mark_func(obj, cr->mfd->data);
cr->mfd.mark_func(obj, cr->mfd.data);
}

void
Expand All @@ -11688,9 +11689,9 @@ rb_objspace_reachable_objects_from(VALUE obj, void (func)(VALUE, void *), void *
struct gc_mark_func_data_struct mfd = {
.mark_func = func,
.data = data,
}, *prev_mfd = cr->mfd;
}, prev_mfd = cr->mfd;

cr->mfd = &mfd;
cr->mfd = mfd;
gc_mark_children(objspace, obj);
cr->mfd = prev_mfd;
}
Expand Down Expand Up @@ -11731,9 +11732,9 @@ objspace_reachable_objects_from_root(rb_objspace_t *objspace, void (func)(const
struct gc_mark_func_data_struct mfd = {
.mark_func = root_objects_from,
.data = &data,
}, *prev_mfd = cr->mfd;
}, prev_mfd = cr->mfd;

cr->mfd = &mfd;
cr->mfd = mfd;
gc_mark_roots(objspace, &data.category);
cr->mfd = prev_mfd;
}
Expand Down Expand Up @@ -13770,6 +13771,13 @@ rb_gcdebug_remove_stress_to_class(int argc, VALUE *argv, VALUE self)
return Qnil;
}

void
rb_ractor_init_mfd(rb_ractor_t *r)
{
r->mfd.mark_func = gc_mark_ptr;
r->mfd.data = (void *)&rb_objspace;
}

/*
* Document-module: ObjectSpace
*
Expand Down
8 changes: 8 additions & 0 deletions internal/gc.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,13 @@ const char *rb_raw_obj_info(char *const buff, const size_t buff_size, VALUE obj)

size_t rb_size_pool_slot_size(unsigned char pool_id);

typedef void (*rb_ractor_value_visitor_func)(VALUE v, void *data);

struct gc_mark_func_data_struct {
void *data;
rb_ractor_value_visitor_func mark_func;
};

struct rb_execution_context_struct; /* in vm_core.h */
struct rb_objspace; /* in vm_core.h */

Expand Down Expand Up @@ -234,6 +241,7 @@ VALUE rb_gc_id2ref_obj_tbl(VALUE objid);
VALUE rb_define_finalizer_no_check(VALUE obj, VALUE block);

void rb_gc_mark_and_move(VALUE *ptr);
void rb_ractor_init_mfd(rb_ractor_t *r);

#define rb_gc_mark_and_move_ptr(ptr) do { \
VALUE _obj = (VALUE)*(ptr); \
Expand Down
3 changes: 2 additions & 1 deletion ractor.c
Original file line number Diff line number Diff line change
Expand Up @@ -1903,6 +1903,7 @@ ractor_alloc(VALUE klass)
VALUE rv = TypedData_Make_Struct(klass, rb_ractor_t, &ractor_data_type, r);
FL_SET_RAW(rv, RUBY_FL_SHAREABLE);
r->pub.self = rv;
rb_ractor_init_mfd(r);
VM_ASSERT(ractor_status_p(r, ractor_created));
return rv;
}
Expand All @@ -1921,7 +1922,7 @@ rb_ractor_main_alloc(void)
r->name = Qnil;
r->pub.self = Qnil;
ruby_single_main_ractor = r;

rb_ractor_init_mfd(r);
return r;
}

Expand Down
5 changes: 1 addition & 4 deletions ractor_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,7 @@ struct rb_ractor_struct {
rb_ractor_newobj_cache_t newobj_cache;

// gc.c rb_objspace_reachable_objects_from
struct gc_mark_func_data_struct {
void *data;
void (*mark_func)(VALUE v, void *data);
} *mfd;
struct gc_mark_func_data_struct mfd;
}; // rb_ractor_t is defined in vm_core.h


Expand Down

0 comments on commit 3be4bf8

Please sign in to comment.