From 8b0868cbb106c49d8761536abc408dae0b2e1c1c Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 27 May 2025 09:36:33 +0200 Subject: [PATCH] Refactor `rb_shape_rebuild_shape` to stop exposing `rb_shape_t` --- object.c | 77 ++++++++++++++++++++---------------------------------- shape.c | 53 +++++++++++++++++++++++++++++++++++++ shape.h | 13 +++++++-- variable.c | 56 ++++++++++----------------------------- variable.h | 1 + 5 files changed, 107 insertions(+), 93 deletions(-) diff --git a/object.c b/object.c index 5044a91dc0..d49ffd96d6 100644 --- a/object.c +++ b/object.c @@ -322,7 +322,6 @@ void rb_obj_copy_ivar(VALUE dest, VALUE obj) { RUBY_ASSERT(!RB_TYPE_P(obj, T_CLASS) && !RB_TYPE_P(obj, T_MODULE)); - RUBY_ASSERT(BUILTIN_TYPE(dest) == BUILTIN_TYPE(obj)); unsigned long src_num_ivs = rb_ivar_count(obj); @@ -330,28 +329,21 @@ rb_obj_copy_ivar(VALUE dest, VALUE obj) return; } - rb_shape_t *src_shape = rb_obj_shape(obj); - - if (rb_shape_too_complex_p(src_shape)) { - // obj is TOO_COMPLEX so we can copy its iv_hash - st_table *table = st_copy(ROBJECT_FIELDS_HASH(obj)); - if (rb_shape_has_object_id(src_shape)) { - st_data_t id = (st_data_t)ruby_internal_object_id; - st_delete(table, &id, NULL); - } - rb_obj_init_too_complex(dest, table); + shape_id_t src_shape_id = RBASIC_SHAPE_ID(obj); + if (rb_shape_id_too_complex_p(src_shape_id)) { + rb_shape_copy_complex_ivars(dest, obj, src_shape_id, ROBJECT_FIELDS_HASH(obj)); return; } - rb_shape_t *shape_to_set_on_dest = src_shape; - rb_shape_t *initial_shape = rb_obj_shape(dest); + shape_id_t dest_shape_id = src_shape_id; + shape_id_t initial_shape_id = RBASIC_SHAPE_ID(dest); - if (initial_shape->heap_index != src_shape->heap_index || !rb_shape_canonical_p(src_shape)) { - RUBY_ASSERT(initial_shape->type == SHAPE_T_OBJECT); + if (RSHAPE(initial_shape_id)->heap_index != RSHAPE(src_shape_id)->heap_index || !rb_shape_id_canonical_p(src_shape_id)) { + RUBY_ASSERT(RSHAPE(initial_shape_id)->type == SHAPE_T_OBJECT); - shape_to_set_on_dest = rb_shape_rebuild_shape(initial_shape, src_shape); - if (UNLIKELY(rb_shape_too_complex_p(shape_to_set_on_dest))) { + dest_shape_id = rb_shape_rebuild(initial_shape_id, src_shape_id); + if (UNLIKELY(rb_shape_id_too_complex_p(dest_shape_id))) { st_table *table = rb_st_init_numtable_with_size(src_num_ivs); rb_obj_copy_ivs_to_hash_table(obj, table); rb_obj_init_too_complex(dest, table); @@ -363,36 +355,14 @@ rb_obj_copy_ivar(VALUE dest, VALUE obj) VALUE *src_buf = ROBJECT_FIELDS(obj); VALUE *dest_buf = ROBJECT_FIELDS(dest); - RUBY_ASSERT(src_num_ivs <= shape_to_set_on_dest->capacity); - if (initial_shape->capacity < shape_to_set_on_dest->capacity) { - rb_ensure_iv_list_size(dest, initial_shape->capacity, shape_to_set_on_dest->capacity); + RUBY_ASSERT(src_num_ivs <= RSHAPE(dest_shape_id)->capacity); + if (RSHAPE(initial_shape_id)->capacity < RSHAPE(dest_shape_id)->capacity) { + rb_ensure_iv_list_size(dest, RSHAPE(initial_shape_id)->capacity, RSHAPE(dest_shape_id)->capacity); dest_buf = ROBJECT_FIELDS(dest); } - if (src_shape->next_field_index == shape_to_set_on_dest->next_field_index) { - // Happy path, we can just memcpy the fields content - MEMCPY(dest_buf, src_buf, VALUE, src_num_ivs); - - // Fire write barriers - for (uint32_t i = 0; i < src_num_ivs; i++) { - RB_OBJ_WRITTEN(dest, Qundef, dest_buf[i]); - } - } - else { - rb_shape_t *dest_shape = shape_to_set_on_dest; - while (src_shape->parent_id != INVALID_SHAPE_ID) { - if (src_shape->type == SHAPE_IVAR) { - while (dest_shape->edge_name != src_shape->edge_name) { - dest_shape = RSHAPE(dest_shape->parent_id); - } - - RB_OBJ_WRITE(dest, &dest_buf[dest_shape->next_field_index - 1], src_buf[src_shape->next_field_index - 1]); - } - src_shape = RSHAPE(src_shape->parent_id); - } - } - - rb_shape_set_shape(dest, shape_to_set_on_dest); + rb_shape_copy_fields(dest, dest_buf, dest_shape_id, obj, src_buf, src_shape_id); + rb_shape_set_shape_id(dest, dest_shape_id); } static void @@ -404,11 +374,20 @@ init_copy(VALUE dest, VALUE obj) RBASIC(dest)->flags &= ~(T_MASK|FL_EXIVAR); // Copies the shape id from obj to dest RBASIC(dest)->flags |= RBASIC(obj)->flags & (T_MASK|FL_EXIVAR); - if (RB_TYPE_P(obj, T_OBJECT)) { - rb_obj_copy_ivar(dest, obj); - } - else { - rb_copy_generic_ivar(dest, obj); + switch (BUILTIN_TYPE(obj)) { + case T_IMEMO: + rb_bug("Unreacheable"); + break; + case T_CLASS: + case T_MODULE: + // noop: handled in class.c: rb_mod_init_copy + break; + case T_OBJECT: + rb_obj_copy_ivar(dest, obj); + break; + default: + rb_copy_generic_ivar(dest, obj); + break; } rb_gc_copy_attributes(dest, obj); } diff --git a/shape.c b/shape.c index 739b2a8b9d..86eed80a93 100644 --- a/shape.c +++ b/shape.c @@ -685,6 +685,12 @@ rb_shape_has_object_id(rb_shape_t *shape) return shape->flags & SHAPE_FL_HAS_OBJECT_ID; } +bool +rb_shape_id_has_object_id(shape_id_t shape_id) +{ + return rb_shape_has_object_id(RSHAPE(shape_id)); +} + shape_id_t rb_shape_transition_object_id(VALUE obj) { @@ -1032,6 +1038,53 @@ rb_shape_rebuild_shape(rb_shape_t *initial_shape, rb_shape_t *dest_shape) return midway_shape; } +shape_id_t +rb_shape_rebuild(shape_id_t initial_shape_id, shape_id_t dest_shape_id) +{ + return rb_shape_id(rb_shape_rebuild_shape(RSHAPE(initial_shape_id), RSHAPE(dest_shape_id))); +} + +void +rb_shape_copy_fields(VALUE dest, VALUE *dest_buf, shape_id_t dest_shape_id, VALUE src, VALUE *src_buf, shape_id_t src_shape_id) +{ + rb_shape_t *dest_shape = RSHAPE(dest_shape_id); + rb_shape_t *src_shape = RSHAPE(src_shape_id); + + if (src_shape->next_field_index == dest_shape->next_field_index) { + // Happy path, we can just memcpy the ivptr content + MEMCPY(dest_buf, src_buf, VALUE, dest_shape->next_field_index); + + // Fire write barriers + for (uint32_t i = 0; i < dest_shape->next_field_index; i++) { + RB_OBJ_WRITTEN(dest, Qundef, dest_buf[i]); + } + } + else { + while (src_shape->parent_id != INVALID_SHAPE_ID) { + if (src_shape->type == SHAPE_IVAR) { + while (dest_shape->edge_name != src_shape->edge_name) { + dest_shape = RSHAPE(dest_shape->parent_id); + } + + RB_OBJ_WRITE(dest, &dest_buf[dest_shape->next_field_index - 1], src_buf[src_shape->next_field_index - 1]); + } + src_shape = RSHAPE(src_shape->parent_id); + } + } +} + +void +rb_shape_copy_complex_ivars(VALUE dest, VALUE obj, shape_id_t src_shape_id, st_table *fields_table) +{ + // obj is TOO_COMPLEX so we can copy its iv_hash + st_table *table = st_copy(fields_table); + if (rb_shape_id_has_object_id(src_shape_id)) { + st_data_t id = (st_data_t)ruby_internal_object_id; + st_delete(table, &id, NULL); + } + rb_obj_init_too_complex(dest, table); +} + RUBY_FUNC_EXPORTED bool rb_shape_obj_too_complex_p(VALUE obj) { diff --git a/shape.h b/shape.h index 0acb4a5f70..9a84835e65 100644 --- a/shape.h +++ b/shape.h @@ -127,6 +127,8 @@ bool rb_shape_get_iv_index_with_hint(shape_id_t shape_id, ID id, attr_index_t *v RUBY_FUNC_EXPORTED bool rb_shape_obj_too_complex_p(VALUE obj); bool rb_shape_too_complex_p(rb_shape_t *shape); bool rb_shape_id_too_complex_p(shape_id_t shape_id); +bool rb_shape_has_object_id(rb_shape_t *shape); +bool rb_shape_id_has_object_id(shape_id_t shape_id); void rb_shape_set_shape(VALUE obj, rb_shape_t *shape); shape_id_t rb_shape_transition_frozen(VALUE obj); @@ -136,10 +138,11 @@ shape_id_t rb_shape_transition_add_ivar(VALUE obj, ID id); shape_id_t rb_shape_transition_add_ivar_no_warnings(VALUE obj, ID id); shape_id_t rb_shape_transition_object_id(VALUE obj); -bool rb_shape_has_object_id(rb_shape_t *shape); void rb_shape_free_all(void); -rb_shape_t *rb_shape_rebuild_shape(rb_shape_t *initial_shape, rb_shape_t *dest_shape); +shape_id_t rb_shape_rebuild(shape_id_t initial_shape_id, shape_id_t dest_shape_id); +void rb_shape_copy_fields(VALUE dest, VALUE *dest_buf, shape_id_t dest_shape_id, VALUE src, VALUE *src_buf, shape_id_t src_shape_id); +void rb_shape_copy_complex_ivars(VALUE dest, VALUE obj, shape_id_t src_shape_id, st_table *fields_table); static inline rb_shape_t * rb_obj_shape(VALUE obj) @@ -153,6 +156,12 @@ rb_shape_canonical_p(rb_shape_t *shape) return !shape->flags; } +static inline bool +rb_shape_id_canonical_p(shape_id_t shape_id) +{ + return rb_shape_canonical_p(RSHAPE(shape_id)); +} + static inline shape_id_t rb_shape_root(size_t heap_id) { diff --git a/variable.c b/variable.c index 1f77976640..1f948fb356 100644 --- a/variable.c +++ b/variable.c @@ -2330,7 +2330,7 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj) goto clear; } - rb_shape_t *src_shape = rb_obj_shape(obj); + shape_id_t src_shape_id = rb_obj_shape_id(obj); if (rb_gen_fields_tbl_get(obj, 0, &obj_fields_tbl)) { if (gen_fields_tbl_count(obj, obj_fields_tbl) == 0) @@ -2338,26 +2338,19 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj) FL_SET(dest, FL_EXIVAR); - if (rb_shape_too_complex_p(src_shape)) { - // obj is TOO_COMPLEX so we can copy its iv_hash - st_table *table = st_copy(obj_fields_tbl->as.complex.table); - if (rb_shape_has_object_id(src_shape)) { - st_data_t id = (st_data_t)ruby_internal_object_id; - st_delete(table, &id, NULL); - } - rb_obj_init_too_complex(dest, table); - + if (rb_shape_id_too_complex_p(src_shape_id)) { + rb_shape_copy_complex_ivars(dest, obj, src_shape_id, obj_fields_tbl->as.complex.table); return; } - rb_shape_t *shape_to_set_on_dest = src_shape; - rb_shape_t *initial_shape = rb_obj_shape(dest); + shape_id_t dest_shape_id = src_shape_id; + shape_id_t initial_shape_id = rb_obj_shape_id(dest); - if (!rb_shape_canonical_p(src_shape)) { - RUBY_ASSERT(initial_shape->type == SHAPE_ROOT); + if (!rb_shape_id_canonical_p(src_shape_id)) { + RUBY_ASSERT(RSHAPE(initial_shape_id)->type == SHAPE_ROOT); - shape_to_set_on_dest = rb_shape_rebuild_shape(initial_shape, src_shape); - if (UNLIKELY(rb_shape_too_complex_p(shape_to_set_on_dest))) { + dest_shape_id = rb_shape_rebuild(initial_shape_id, src_shape_id); + if (UNLIKELY(rb_shape_id_too_complex_p(dest_shape_id))) { st_table *table = rb_st_init_numtable_with_size(src_num_ivs); rb_obj_copy_ivs_to_hash_table(obj, table); rb_obj_init_too_complex(dest, table); @@ -2366,39 +2359,18 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj) } } - if (!shape_to_set_on_dest->capacity) { - rb_shape_set_shape(dest, shape_to_set_on_dest); + if (!RSHAPE(dest_shape_id)->capacity) { + rb_shape_set_shape_id(dest, dest_shape_id); FL_UNSET(dest, FL_EXIVAR); return; } - new_fields_tbl = gen_fields_tbl_resize(0, shape_to_set_on_dest->capacity); + new_fields_tbl = gen_fields_tbl_resize(0, RSHAPE(dest_shape_id)->capacity); VALUE *src_buf = obj_fields_tbl->as.shape.fields; VALUE *dest_buf = new_fields_tbl->as.shape.fields; - if (src_shape->next_field_index == shape_to_set_on_dest->next_field_index) { - // Happy path, we can just memcpy the ivptr content - MEMCPY(dest_buf, src_buf, VALUE, shape_to_set_on_dest->next_field_index); - - // Fire write barriers - for (uint32_t i = 0; i < shape_to_set_on_dest->next_field_index; i++) { - RB_OBJ_WRITTEN(dest, Qundef, dest_buf[i]); - } - } - else { - rb_shape_t *dest_shape = shape_to_set_on_dest; - while (src_shape->parent_id != INVALID_SHAPE_ID) { - if (src_shape->type == SHAPE_IVAR) { - while (dest_shape->edge_name != src_shape->edge_name) { - dest_shape = RSHAPE(dest_shape->parent_id); - } - - RB_OBJ_WRITE(dest, &dest_buf[dest_shape->next_field_index - 1], src_buf[src_shape->next_field_index - 1]); - } - src_shape = RSHAPE(src_shape->parent_id); - } - } + rb_shape_copy_fields(dest, dest_buf, dest_shape_id, obj, src_buf, src_shape_id); /* * c.fields_tbl may change in gen_fields_copy due to realloc, @@ -2409,7 +2381,7 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj) st_insert(generic_fields_tbl_no_ractor_check(obj), (st_data_t)dest, (st_data_t)new_fields_tbl); } - rb_shape_set_shape(dest, shape_to_set_on_dest); + rb_shape_set_shape_id(dest, dest_shape_id); } return; diff --git a/variable.h b/variable.h index b00b0c5757..a95fcc563d 100644 --- a/variable.h +++ b/variable.h @@ -25,6 +25,7 @@ struct gen_fields_tbl { }; int rb_ivar_generic_fields_tbl_lookup(VALUE obj, struct gen_fields_tbl **); +void rb_copy_complex_ivars(VALUE dest, VALUE obj, shape_id_t src_shape_id, st_table *fields_table); void rb_free_rb_global_tbl(void); void rb_free_generic_fields_tbl_(void);