From 89240eb2fbdbd9a46788b4976cd4bdf4cc58ada2 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 21 Jan 2025 16:36:03 -0500 Subject: [PATCH] Add generic ivar reference updating step Previously, generic ivars worked differently than the other global tables during compaction. The other global tables had their references updated through iteration during rb_gc_update_vm_references. Generic ivars updated the keys when the object moved and updated the values while reference updating the object. This is inefficient as this required one lookup for every moved object and one lookup for every object with generic ivars. Instead, this commit changes it to iterate over the generic ivar table to update both the keys and values. --- gc.c | 5 +---- gc/default/default.c | 9 --------- internal/variable.h | 3 +-- variable.c | 43 ++++++++++++++++++++++++++----------------- 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/gc.c b/gc.c index 08060f5bf9..e4b564a38d 100644 --- a/gc.c +++ b/gc.c @@ -3492,6 +3492,7 @@ rb_gc_update_vm_references(void *objspace) rb_vm_t *vm = rb_ec_vm_ptr(ec); rb_vm_update_references(vm); + rb_generic_ivar_update_references(); rb_gc_update_global_tbl(); global_symbols.ids = gc_location_internal(objspace, global_symbols.ids); global_symbols.dsymbol_fstr_hash = gc_location_internal(objspace, global_symbols.dsymbol_fstr_hash); @@ -3509,10 +3510,6 @@ rb_gc_update_vm_references(void *objspace) void rb_gc_update_object_references(void *objspace, VALUE obj) { - if (FL_TEST(obj, FL_EXIVAR)) { - rb_ref_update_generic_ivar(obj); - } - switch (BUILTIN_TYPE(obj)) { case T_CLASS: if (FL_TEST(obj, FL_SINGLETON)) { diff --git a/gc/default/default.c b/gc/default/default.c index e8d43986b9..a05a2ca29c 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -6927,15 +6927,6 @@ gc_move(rb_objspace_t *objspace, VALUE src, VALUE dest, size_t src_slot_size, si CLEAR_IN_BITMAP(GET_HEAP_UNCOLLECTIBLE_BITS(src), src); CLEAR_IN_BITMAP(GET_HEAP_PAGE(src)->remembered_bits, src); - if (FL_TEST(src, FL_EXIVAR)) { - /* Resizing the st table could cause a malloc */ - DURING_GC_COULD_MALLOC_REGION_START(); - { - rb_mv_generic_ivar(src, dest); - } - DURING_GC_COULD_MALLOC_REGION_END(); - } - if (FL_TEST(src, FL_SEEN_OBJ_ID)) { /* If the source object's object_id has been seen, we need to update * the object to object id mapping. */ diff --git a/internal/variable.h b/internal/variable.h index b2a30c7c58..d7b30d6f73 100644 --- a/internal/variable.h +++ b/internal/variable.h @@ -50,12 +50,11 @@ int rb_gen_ivtbl_get(VALUE obj, ID id, struct gen_ivtbl **ivtbl); void rb_obj_copy_ivs_to_hash_table(VALUE obj, st_table *table); void rb_obj_convert_to_too_complex(VALUE obj, st_table *table); void rb_evict_ivars_to_hash(VALUE obj); +void rb_generic_ivar_update_references(void); RUBY_SYMBOL_EXPORT_BEGIN /* variable.c (export) */ void rb_mark_generic_ivar(VALUE obj); -void rb_ref_update_generic_ivar(VALUE); -void rb_mv_generic_ivar(VALUE src, VALUE dst); 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); diff --git a/variable.c b/variable.c index 66b960cabe..9def7645ae 100644 --- a/variable.c +++ b/variable.c @@ -1136,31 +1136,40 @@ rb_mark_generic_ivar(VALUE obj) } } -void -rb_ref_update_generic_ivar(VALUE obj) +static int +rb_generic_ivar_update_references_i(st_data_t key, st_data_t val, st_data_t _data) { - struct gen_ivtbl *ivtbl; + VALUE orig_obj = (VALUE)key; + VALUE obj = rb_gc_location(orig_obj); + struct gen_ivtbl *ivtbl = (struct gen_ivtbl *)val; - if (rb_gen_ivtbl_get(obj, 0, &ivtbl)) { - if (rb_shape_obj_too_complex(obj)) { - rb_gc_ref_update_table_values_only(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]); - } + if (rb_shape_obj_too_complex(obj)) { + rb_gc_ref_update_table_values_only(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]); } } + + if (obj != orig_obj) { + st_insert(generic_iv_tbl_, (st_data_t)obj, (st_data_t)ivtbl); + + return ST_DELETE; + } + else { + return ST_CONTINUE; + } } void -rb_mv_generic_ivar(VALUE rsrc, VALUE dst) +rb_generic_ivar_update_references(void) { - 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); + DURING_GC_COULD_MALLOC_REGION_START(); + { + st_foreach(generic_iv_tbl_, rb_generic_ivar_update_references_i, (st_data_t)0); + } + DURING_GC_COULD_MALLOC_REGION_END(); } void