Skip to content

Commit

Permalink
Fix global ivtbl handling
Browse files Browse the repository at this point in the history
The upstream now updates global ivtbl entries when updating weak tables.
While this works well for the default mark-and-compact GC, it is not
idiomatic to separate the marking and the updating unless we are
implementing a compacting GC.

We restored the previous eager-updating strategy when using MMTk.  For
this, we resurrected `rb_mv_generic_ivar`, too.
  • Loading branch information
wks committed Feb 11, 2025
1 parent 504f310 commit a38a525
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 17 deletions.
12 changes: 12 additions & 0 deletions gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2760,9 +2760,13 @@ rb_mmtk_scan_yjit_roots(void)
void
rb_gc_mark_children(void *objspace, VALUE obj)
{
// When using MMTK, we rely on rb_gc_update_object_references to trace generic ivars.
// We still need to visit generic ivars in rb_gc_mark_children when traversing (not during GC).
WHEN_USING_MMTK_CONDITIONAL(GET_VM()->gc.mark_func_data != NULL, {
if (FL_TEST(obj, FL_EXIVAR)) {
rb_mark_generic_ivar(obj);
}
})

switch (BUILTIN_TYPE(obj)) {
case T_FLOAT:
Expand Down Expand Up @@ -3853,6 +3857,14 @@ rb_mmtk_gc_ref_update_string(void * objspace, VALUE str)
void
rb_gc_update_object_references(void *objspace, VALUE obj)
{
WHEN_USING_MMTK({
// When using MMTk, we eagerly trace and update the generic ivars when scanning an object.
// In the default GC, we update the global ivar table during weak ref processing.
if (FL_TEST(obj, FL_EXIVAR)) {
rb_mmtk_update_generic_ivar(obj);
}
})

switch (BUILTIN_TYPE(obj)) {
case T_CLASS:
if (FL_TEST(obj, FL_SINGLETON)) {
Expand Down
2 changes: 2 additions & 0 deletions internal/mmtk_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
#define WHEN_USING_MMTK(a) if (rb_mmtk_enabled_p()) { a }
#define WHEN_NOT_USING_MMTK(a) if (!rb_mmtk_enabled_p()) { a }
#define WHEN_USING_MMTK2(a, b) if (rb_mmtk_enabled_p()) { a } else { b }
#define WHEN_USING_MMTK_CONDITIONAL(cond, a) if (!rb_mmtk_enabled_p() || (cond)) { a }
#else
#define IF_USE_MMTK(a)
#define IF_NOT_USE_MMTK(a) a
#define IF_USE_MMTK2(a, b) b
#define WHEN_USING_MMTK(a)
#define WHEN_NOT_USING_MMTK(a) a
#define WHEN_USING_MMTK2(a, b) b
#define WHEN_USING_MMTK_CONDITIONAL(cond, a) a
#endif

#endif /* INTERNAL_MMTK_MACROS_H */
3 changes: 3 additions & 0 deletions internal/variable.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ void rb_evict_ivars_to_hash(VALUE obj);
RUBY_SYMBOL_EXPORT_BEGIN
/* variable.c (export) */
void rb_mark_generic_ivar(VALUE obj);
#if USE_MMTK
void rb_mmtk_update_generic_ivar(VALUE obj);
#endif
VALUE rb_const_missing(VALUE klass, VALUE name);
int rb_class_ivar_set(VALUE klass, ID vid, VALUE value);
void rb_iv_tbl_copy(VALUE dst, VALUE src);
Expand Down
12 changes: 10 additions & 2 deletions mmtk_support.c
Original file line number Diff line number Diff line change
Expand Up @@ -1682,9 +1682,18 @@ rb_mmtk_get_original_givtbl(MMTk_ObjectReference object) {
}
}

struct st_table *rb_generic_ivtbl_get(void); // Defined in variable.c

st_table*
rb_mmtk_get_generic_iv_tbl(void) {
return rb_generic_ivtbl_get();
}

void rb_mmtk_mv_generic_ivar(VALUE src, VALUE dst); // Defined in variable.c

static void
rb_mmtk_move_givtbl(MMTk_ObjectReference old_objref, MMTk_ObjectReference new_objref) {
rb_mv_generic_ivar((VALUE)old_objref, (VALUE)new_objref);
rb_mmtk_mv_generic_ivar((VALUE)old_objref, (VALUE)new_objref);
}

void rb_mmtk_cleanup_generic_iv_tbl(void); // Defined in variable.c
Expand All @@ -1694,7 +1703,6 @@ void rb_mmtk_update_global_symbols_table(void); // Defined in gc.c
void rb_mmtk_update_overloaded_cme_table(void); // Defined in default.c
void rb_mmtk_update_ci_table(void); // Defined in default.c

st_table* rb_mmtk_get_generic_iv_tbl(void); // Defined in variable.c
st_table* rb_mmtk_get_frozen_strings_table(void); // Defined in default.c
st_table* rb_mmtk_get_finalizer_table(void); // Defined in default.c
st_table* rb_mmtk_get_obj_to_id_table(void); // Defined in default.c
Expand Down
44 changes: 29 additions & 15 deletions variable.c
Original file line number Diff line number Diff line change
Expand Up @@ -1129,20 +1129,14 @@ gen_ivtbl_resize(struct gen_ivtbl *old, uint32_t n)
void
rb_mark_generic_ivar(VALUE obj)
{
st_data_t data;

int r = 0;
WHEN_USING_MMTK2({
// When using copying GC, if `obj` is alredy moved, we won't find it in `generic_iv_tbl_`
// because `generic_iv_tbl_` is not updated yet. The Rust part of the binding maintains
// another table for moved objects. We call into the Rust part.
data = (st_data_t)mmtk_get_givtbl_during_gc((MMTk_ObjectReference)obj);
r = (data != 0);
}, {
r = st_lookup(generic_ivtbl_no_ractor_check(obj), (st_data_t)obj, &data);
WHEN_USING_MMTK({
// In MMTk, we only call this function when traversing.
// During GC, we call rb_mmtk_update_generic_ivar.
RUBY_ASSERT(GET_VM()->gc.mark_func_data != NULL);
})

if (r) {
st_data_t data;
if (st_lookup(generic_ivtbl_no_ractor_check(obj), (st_data_t)obj, &data)) {
struct gen_ivtbl *ivtbl = (struct gen_ivtbl *)data;
if (rb_shape_obj_too_complex(obj)) {
rb_mark_tbl_no_pin(ivtbl->as.complex.table);
Expand All @@ -1156,10 +1150,30 @@ rb_mark_generic_ivar(VALUE obj)
}

#if USE_MMTK
st_table*
rb_mmtk_get_generic_iv_tbl(void)
void
rb_mmtk_update_generic_ivar(VALUE obj)
{
return generic_iv_tbl_;
struct gen_ivtbl *ivtbl = mmtk_get_givtbl_during_gc((MMTk_ObjectReference)obj);
if (ivtbl != NULL) {
if (rb_shape_obj_too_complex(obj)) {
rb_gc_update_tbl_refs(ivtbl->as.complex.table);
}
else {
for (uint32_t i = 0; i < ivtbl->as.shape.numiv; i++) {
ivtbl->as.shape.ivptr[i] = rb_gc_location(ivtbl->as.shape.ivptr[i]);
}
}
}
}

void
rb_mmtk_mv_generic_ivar(VALUE rsrc, VALUE dst)
{
st_data_t key = (st_data_t)rsrc;
st_data_t ivtbl;

if (st_delete(generic_ivtbl_no_ractor_check(rsrc), &key, &ivtbl))
st_insert(generic_ivtbl_no_ractor_check(dst), (st_data_t)dst, ivtbl);
}

static int
Expand Down

0 comments on commit a38a525

Please sign in to comment.