From 5257e1298c4dc4e854eaa0a9fe5e6dc5c1495c91 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 26 Aug 2025 17:43:11 +0200 Subject: [PATCH] Replace ROBJECT_EMBED by ROBJECT_HEAP The embed layout is way more common than the heap one, especially since WVA. I think it makes for more readable code to inverse the flag. --- ext/objspace/objspace_dump.c | 2 +- gc.c | 75 +++++++++++++++------------- imemo.c | 30 ++++++----- include/ruby/internal/core/robject.h | 20 ++++---- internal/imemo.h | 12 ++--- object.c | 9 ++-- ractor.c | 2 +- shape.c | 2 +- shape.h | 4 +- variable.c | 20 ++++---- yjit/src/codegen.rs | 2 +- yjit/src/cruby.rs | 2 +- yjit/src/cruby_bindings.inc.rs | 2 +- zjit/src/cruby.rs | 2 +- zjit/src/cruby_bindings.inc.rs | 2 +- 15 files changed, 97 insertions(+), 89 deletions(-) diff --git a/ext/objspace/objspace_dump.c b/ext/objspace/objspace_dump.c index 94a9d43f98..30829d5ee1 100644 --- a/ext/objspace/objspace_dump.c +++ b/ext/objspace/objspace_dump.c @@ -587,7 +587,7 @@ dump_object(VALUE obj, struct dump_config *dc) break; case T_OBJECT: - if (FL_TEST(obj, ROBJECT_EMBED)) { + if (!FL_TEST_RAW(obj, ROBJECT_HEAP)) { dump_append(dc, ", \"embedded\":true"); } diff --git a/gc.c b/gc.c index d9efe43de5..72328678c5 100644 --- a/gc.c +++ b/gc.c @@ -1264,16 +1264,18 @@ rb_gc_obj_free(void *objspace, VALUE obj) switch (BUILTIN_TYPE(obj)) { case T_OBJECT: - if (rb_shape_obj_too_complex_p(obj)) { - RB_DEBUG_COUNTER_INC(obj_obj_too_complex); - st_free_table(ROBJECT_FIELDS_HASH(obj)); - } - else if (RBASIC(obj)->flags & ROBJECT_EMBED) { - RB_DEBUG_COUNTER_INC(obj_obj_embed); + if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { + if (rb_shape_obj_too_complex_p(obj)) { + RB_DEBUG_COUNTER_INC(obj_obj_too_complex); + st_free_table(ROBJECT_FIELDS_HASH(obj)); + } + else { + xfree(ROBJECT(obj)->as.heap.fields); + RB_DEBUG_COUNTER_INC(obj_obj_ptr); + } } else { - xfree(ROBJECT(obj)->as.heap.fields); - RB_DEBUG_COUNTER_INC(obj_obj_ptr); + RB_DEBUG_COUNTER_INC(obj_obj_embed); } break; case T_MODULE: @@ -2313,11 +2315,13 @@ rb_obj_memsize_of(VALUE obj) switch (BUILTIN_TYPE(obj)) { case T_OBJECT: - if (rb_shape_obj_too_complex_p(obj)) { - size += rb_st_memsize(ROBJECT_FIELDS_HASH(obj)); - } - else if (!(RBASIC(obj)->flags & ROBJECT_EMBED)) { - size += ROBJECT_FIELDS_CAPACITY(obj) * sizeof(VALUE); + if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { + if (rb_shape_obj_too_complex_p(obj)) { + size += rb_st_memsize(ROBJECT_FIELDS_HASH(obj)); + } + else { + size += ROBJECT_FIELDS_CAPACITY(obj) * sizeof(VALUE); + } } break; case T_MODULE: @@ -3543,19 +3547,21 @@ gc_ref_update_object(void *objspace, VALUE v) { VALUE *ptr = ROBJECT_FIELDS(v); - if (rb_shape_obj_too_complex_p(v)) { - gc_ref_update_table_values_only(ROBJECT_FIELDS_HASH(v)); - return; - } + if (FL_TEST_RAW(v, ROBJECT_HEAP)) { + if (rb_shape_obj_too_complex_p(v)) { + gc_ref_update_table_values_only(ROBJECT_FIELDS_HASH(v)); + return; + } - size_t slot_size = rb_gc_obj_slot_size(v); - size_t embed_size = rb_obj_embedded_size(ROBJECT_FIELDS_CAPACITY(v)); - if (slot_size >= embed_size && !RB_FL_TEST_RAW(v, ROBJECT_EMBED)) { - // Object can be re-embedded - memcpy(ROBJECT(v)->as.ary, ptr, sizeof(VALUE) * ROBJECT_FIELDS_COUNT(v)); - RB_FL_SET_RAW(v, ROBJECT_EMBED); - xfree(ptr); - ptr = ROBJECT(v)->as.ary; + size_t slot_size = rb_gc_obj_slot_size(v); + size_t embed_size = rb_obj_embedded_size(ROBJECT_FIELDS_CAPACITY(v)); + if (slot_size >= embed_size) { + // Object can be re-embedded + memcpy(ROBJECT(v)->as.ary, ptr, sizeof(VALUE) * ROBJECT_FIELDS_COUNT(v)); + FL_UNSET_RAW(v, ROBJECT_HEAP); + xfree(ptr); + ptr = ROBJECT(v)->as.ary; + } } for (uint32_t i = 0; i < ROBJECT_FIELDS_COUNT(v); i++) { @@ -4773,21 +4779,18 @@ rb_raw_obj_info_buitin_type(char *const buff, const size_t buff_size, const VALU } case T_OBJECT: { - if (rb_shape_obj_too_complex_p(obj)) { - size_t hash_len = rb_st_table_size(ROBJECT_FIELDS_HASH(obj)); - APPEND_F("(too_complex) len:%zu", hash_len); - } - else { - uint32_t len = ROBJECT_FIELDS_CAPACITY(obj); - - if (RBASIC(obj)->flags & ROBJECT_EMBED) { - APPEND_F("(embed) len:%d", len); + if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { + if (rb_shape_obj_too_complex_p(obj)) { + size_t hash_len = rb_st_table_size(ROBJECT_FIELDS_HASH(obj)); + APPEND_F("(too_complex) len:%zu", hash_len); } else { - VALUE *ptr = ROBJECT_FIELDS(obj); - APPEND_F("len:%d ptr:%p", len, (void *)ptr); + APPEND_F("(embed) len:%d", ROBJECT_FIELDS_CAPACITY(obj)); } } + else { + APPEND_F("len:%d ptr:%p", ROBJECT_FIELDS_CAPACITY(obj), (void *)ROBJECT_FIELDS(obj)); + } } break; case T_DATA: { diff --git a/imemo.c b/imemo.c index b4ca1fdced..0ffabe8018 100644 --- a/imemo.c +++ b/imemo.c @@ -116,12 +116,12 @@ imemo_fields_new(VALUE owner, size_t capa) if (rb_gc_size_allocatable_p(embedded_size)) { VALUE fields = rb_imemo_new(imemo_fields, owner, embedded_size); RUBY_ASSERT(IMEMO_TYPE_P(fields, imemo_fields)); - FL_SET_RAW(fields, OBJ_FIELD_EMBED); return fields; } else { VALUE fields = rb_imemo_new(imemo_fields, owner, sizeof(struct rb_fields)); IMEMO_OBJ_FIELDS(fields)->as.external.ptr = ALLOC_N(VALUE, capa); + FL_SET_RAW(fields, OBJ_FIELD_HEAP); return fields; } } @@ -137,7 +137,7 @@ imemo_fields_new_complex(VALUE owner, size_t capa) { VALUE fields = imemo_fields_new(owner, 1); IMEMO_OBJ_FIELDS(fields)->as.complex.table = st_init_numtable_with_size(capa); - FL_UNSET_RAW(fields, OBJ_FIELD_EMBED); + FL_SET_RAW(fields, OBJ_FIELD_HEAP); return fields; } @@ -166,8 +166,8 @@ VALUE rb_imemo_fields_new_complex_tbl(VALUE owner, st_table *tbl) { VALUE fields = imemo_fields_new(owner, sizeof(struct rb_fields)); - FL_UNSET_RAW(fields, OBJ_FIELD_EMBED); IMEMO_OBJ_FIELDS(fields)->as.complex.table = tbl; + FL_SET_RAW(fields, OBJ_FIELD_HEAP); st_foreach(tbl, imemo_fields_trigger_wb_i, (st_data_t)fields); return fields; } @@ -257,11 +257,13 @@ rb_imemo_memsize(VALUE obj) break; case imemo_fields: - if (rb_shape_obj_too_complex_p(obj)) { - size += st_memsize(IMEMO_OBJ_FIELDS(obj)->as.complex.table); - } - else if (!FL_TEST_RAW(obj, OBJ_FIELD_EMBED)) { - size += RSHAPE_CAPACITY(RBASIC_SHAPE_ID(obj)) * sizeof(VALUE); + if (FL_TEST_RAW(obj, OBJ_FIELD_HEAP)) { + if (rb_shape_obj_too_complex_p(obj)) { + size += st_memsize(IMEMO_OBJ_FIELDS(obj)->as.complex.table); + } + else { + size += RSHAPE_CAPACITY(RBASIC_SHAPE_ID(obj)) * sizeof(VALUE); + } } break; default: @@ -535,11 +537,13 @@ rb_free_const_table(struct rb_id_table *tbl) static inline void imemo_fields_free(struct rb_fields *fields) { - if (rb_shape_obj_too_complex_p((VALUE)fields)) { - st_free_table(fields->as.complex.table); - } - else if (!FL_TEST_RAW((VALUE)fields, OBJ_FIELD_EMBED)) { - xfree(fields->as.external.ptr); + if (FL_TEST_RAW((VALUE)fields, OBJ_FIELD_HEAP)) { + if (rb_shape_obj_too_complex_p((VALUE)fields)) { + st_free_table(fields->as.complex.table); + } + else { + xfree(fields->as.external.ptr); + } } } diff --git a/include/ruby/internal/core/robject.h b/include/ruby/internal/core/robject.h index fc5b28fd67..4892faab2b 100644 --- a/include/ruby/internal/core/robject.h +++ b/include/ruby/internal/core/robject.h @@ -43,7 +43,7 @@ #define ROBJECT(obj) RBIMPL_CAST((struct RObject *)(obj)) /** @cond INTERNAL_MACRO */ #define ROBJECT_EMBED_LEN_MAX ROBJECT_EMBED_LEN_MAX -#define ROBJECT_EMBED ROBJECT_EMBED +#define ROBJECT_HEAP ROBJECT_HEAP #define ROBJECT_FIELDS_CAPACITY ROBJECT_FIELDS_CAPACITY #define ROBJECT_FIELDS ROBJECT_FIELDS /** @endcond */ @@ -55,10 +55,12 @@ */ enum ruby_robject_flags { /** - * This flag has something to do with memory footprint. If the object is - * "small" enough, ruby tries to be creative to abuse padding bits of - * struct ::RObject for storing instance variables. This flag denotes that - * situation. + * This flag has marks that the object's instance variables are stored in an + * external heap buffer. + * Normally, instance variable references are stored inside the object slot, + * but if it overflow, Ruby may have to allocate a separate buffer and spills + * the instance variables there. + * This flag denotes that situation. * * @warning This bit has to be considered read-only. Setting/clearing * this bit without corresponding fix up must cause immediate @@ -71,7 +73,7 @@ enum ruby_robject_flags { * 3rd parties must not be aware that there even is more than one way to * store instance variables. Might better be hidden. */ - ROBJECT_EMBED = RUBY_FL_USER4 + ROBJECT_HEAP = RUBY_FL_USER4 }; struct st_table; @@ -129,11 +131,11 @@ ROBJECT_FIELDS(VALUE obj) struct RObject *const ptr = ROBJECT(obj); - if (RB_FL_ANY_RAW(obj, ROBJECT_EMBED)) { - return ptr->as.ary; + if (RB_UNLIKELY(RB_FL_ANY_RAW(obj, ROBJECT_HEAP))) { + return ptr->as.heap.fields; } else { - return ptr->as.heap.fields; + return ptr->as.ary; } } diff --git a/internal/imemo.h b/internal/imemo.h index c5afcefc97..f7bd620238 100644 --- a/internal/imemo.h +++ b/internal/imemo.h @@ -272,8 +272,8 @@ struct rb_fields { // IMEMO/fields and T_OBJECT have exactly the same layout. // This is useful for JIT and common codepaths. -#define OBJ_FIELD_EMBED ROBJECT_EMBED -STATIC_ASSERT(imemo_fields_flags, OBJ_FIELD_EMBED == IMEMO_FL_USER0); +#define OBJ_FIELD_HEAP ROBJECT_HEAP +STATIC_ASSERT(imemo_fields_flags, OBJ_FIELD_HEAP == IMEMO_FL_USER0); STATIC_ASSERT(imemo_fields_embed_offset, offsetof(struct RObject, as.ary) == offsetof(struct rb_fields, as.embed.fields)); STATIC_ASSERT(imemo_fields_embed_offset, offsetof(struct RObject, as.heap.fields) == offsetof(struct rb_fields, as.external.ptr)); STATIC_ASSERT(imemo_fields_embed_offset, offsetof(struct RObject, as.heap.fields) == offsetof(struct rb_fields, as.complex.table)); @@ -303,11 +303,11 @@ rb_imemo_fields_ptr(VALUE fields_obj) RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields) || RB_TYPE_P(fields_obj, T_OBJECT)); - if (RB_LIKELY(FL_TEST_RAW(fields_obj, OBJ_FIELD_EMBED))) { - return IMEMO_OBJ_FIELDS(fields_obj)->as.embed.fields; + if (UNLIKELY(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP))) { + return IMEMO_OBJ_FIELDS(fields_obj)->as.external.ptr; } else { - return IMEMO_OBJ_FIELDS(fields_obj)->as.external.ptr; + return IMEMO_OBJ_FIELDS(fields_obj)->as.embed.fields; } } @@ -319,7 +319,7 @@ rb_imemo_fields_complex_tbl(VALUE fields_obj) } RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields) || RB_TYPE_P(fields_obj, T_OBJECT)); - RUBY_ASSERT(!FL_TEST_RAW(fields_obj, OBJ_FIELD_EMBED)); + RUBY_ASSERT(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP)); // Some codepaths unconditionally access the fields_ptr, and assume it can be used as st_table if the // shape is too_complex. diff --git a/object.c b/object.c index cbe3a5269d..a7def50cd7 100644 --- a/object.c +++ b/object.c @@ -46,10 +46,9 @@ /* Flags of RObject * - * 4: ROBJECT_EMBED - * The object has its instance variables embedded (the array of - * instance variables directly follow the object, rather than being - * on a separately allocated buffer). + * 4: ROBJECT_HEAP + * The object has its instance variables in a separately allocated buffer. + * This can be either a flat buffer of reference, or an st_table for complex objects. */ /*! @@ -126,7 +125,7 @@ rb_class_allocate_instance(VALUE klass) } NEWOBJ_OF(o, struct RObject, klass, - T_OBJECT | ROBJECT_EMBED | (RGENGC_WB_PROTECTED_OBJECT ? FL_WB_PROTECTED : 0), size, 0); + T_OBJECT | (RGENGC_WB_PROTECTED_OBJECT ? FL_WB_PROTECTED : 0), size, 0); VALUE obj = (VALUE)o; RUBY_ASSERT(RSHAPE_TYPE_P(RBASIC_SHAPE_ID(obj), SHAPE_ROOT)); diff --git a/ractor.c b/ractor.c index d40d0e61bf..57c9c4178f 100644 --- a/ractor.c +++ b/ractor.c @@ -1913,7 +1913,7 @@ move_leave(VALUE obj, struct obj_traverse_replace_data *data) rb_replace_generic_ivar(data->replacement, obj); } - VALUE flags = T_OBJECT | FL_FREEZE | ROBJECT_EMBED | (RBASIC(obj)->flags & FL_PROMOTED); + VALUE flags = T_OBJECT | FL_FREEZE | (RBASIC(obj)->flags & FL_PROMOTED); // Avoid mutations using bind_call, etc. MEMZERO((char *)obj, char, sizeof(struct RBasic)); diff --git a/shape.c b/shape.c index 05aea7905c..599b774226 100644 --- a/shape.c +++ b/shape.c @@ -1273,7 +1273,7 @@ rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id) // Ensure complex object don't appear as embedded if (RB_TYPE_P(obj, T_OBJECT) || IMEMO_TYPE_P(obj, imemo_fields)) { - RUBY_ASSERT(!FL_TEST_RAW(obj, ROBJECT_EMBED)); + RUBY_ASSERT(FL_TEST_RAW(obj, ROBJECT_HEAP)); } } else { diff --git a/shape.h b/shape.h index 3541b7baf4..1f444c545f 100644 --- a/shape.h +++ b/shape.h @@ -351,7 +351,7 @@ ROBJECT_FIELDS_HASH(VALUE obj) { RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); RUBY_ASSERT(rb_shape_obj_too_complex_p(obj)); - RUBY_ASSERT(!FL_TEST_RAW(obj, ROBJECT_EMBED)); + RUBY_ASSERT(FL_TEST_RAW(obj, ROBJECT_HEAP)); return (st_table *)ROBJECT(obj)->as.heap.fields; } @@ -361,7 +361,7 @@ ROBJECT_SET_FIELDS_HASH(VALUE obj, const st_table *tbl) { RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); RUBY_ASSERT(rb_shape_obj_too_complex_p(obj)); - RUBY_ASSERT(!FL_TEST_RAW(obj, ROBJECT_EMBED)); + RUBY_ASSERT(FL_TEST_RAW(obj, ROBJECT_HEAP)); ROBJECT(obj)->as.heap.fields = (VALUE *)tbl; } diff --git a/variable.c b/variable.c index b5620af27b..b0c364af59 100644 --- a/variable.c +++ b/variable.c @@ -1480,11 +1480,11 @@ obj_transition_too_complex(VALUE obj, st_table *table) case T_OBJECT: { VALUE *old_fields = NULL; - if (FL_TEST_RAW(obj, ROBJECT_EMBED)) { - FL_UNSET_RAW(obj, ROBJECT_EMBED); + if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { + old_fields = ROBJECT_FIELDS(obj); } else { - old_fields = ROBJECT_FIELDS(obj); + FL_SET_RAW(obj, ROBJECT_HEAP); } RBASIC_SET_SHAPE_ID(obj, shape_id); ROBJECT_SET_FIELDS_HASH(obj, table); @@ -1614,12 +1614,12 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) } if (RB_TYPE_P(obj, T_OBJECT) && - !RB_FL_TEST_RAW(obj, ROBJECT_EMBED) && + FL_TEST_RAW(obj, ROBJECT_HEAP) && rb_obj_embedded_size(new_fields_count) <= rb_gc_obj_slot_size(obj)) { // Re-embed objects when instances become small enough // This is necessary because YJIT assumes that objects with the same shape // have the same embeddedness for efficiency (avoid extra checks) - RB_FL_SET_RAW(obj, ROBJECT_EMBED); + FL_UNSET_RAW(obj, ROBJECT_HEAP); MEMCPY(ROBJECT_FIELDS(obj), fields, VALUE, new_fields_count); xfree(fields); } @@ -1813,16 +1813,16 @@ rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t new_capacity) { RUBY_ASSERT(!rb_shape_obj_too_complex_p(obj)); - if (RBASIC(obj)->flags & ROBJECT_EMBED) { + if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { + REALLOC_N(ROBJECT(obj)->as.heap.fields, VALUE, new_capacity); + } + else { VALUE *ptr = ROBJECT_FIELDS(obj); VALUE *newptr = ALLOC_N(VALUE, new_capacity); MEMCPY(newptr, ptr, VALUE, current_len); - RB_FL_UNSET_RAW(obj, ROBJECT_EMBED); + FL_SET_RAW(obj, ROBJECT_HEAP); ROBJECT(obj)->as.heap.fields = newptr; } - else { - REALLOC_N(ROBJECT(obj)->as.heap.fields, VALUE, new_capacity); - } } static int diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index ac5c6f2721..dfd382a173 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -2916,7 +2916,7 @@ fn gen_get_ivar( guard_object_is_heap(asm, recv, recv_opnd, Counter::getivar_not_heap); // Compile time self is embedded and the ivar index lands within the object - let embed_test_result = unsafe { FL_TEST_RAW(comptime_receiver, VALUE(ROBJECT_EMBED.as_usize())) != VALUE(0) }; + let embed_test_result = comptime_receiver.embedded_p(); let expected_shape = unsafe { rb_obj_shape_id(comptime_receiver) }; let shape_id_offset = unsafe { rb_shape_id_offset() }; diff --git a/yjit/src/cruby.rs b/yjit/src/cruby.rs index f7a08b3b18..38d931b8ae 100644 --- a/yjit/src/cruby.rs +++ b/yjit/src/cruby.rs @@ -450,7 +450,7 @@ impl VALUE { pub fn embedded_p(self) -> bool { unsafe { - FL_TEST_RAW(self, VALUE(ROBJECT_EMBED as usize)) != VALUE(0) + FL_TEST_RAW(self, VALUE(ROBJECT_HEAP as usize)) == VALUE(0) } } diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 7ed758ef4b..ea51574fe7 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -306,7 +306,7 @@ pub const RARRAY_EMBED_LEN_SHIFT: ruby_rarray_consts = 15; pub type ruby_rarray_consts = u32; pub const RMODULE_IS_REFINEMENT: ruby_rmodule_flags = 8192; pub type ruby_rmodule_flags = u32; -pub const ROBJECT_EMBED: ruby_robject_flags = 65536; +pub const ROBJECT_HEAP: ruby_robject_flags = 65536; pub type ruby_robject_flags = u32; pub type rb_block_call_func = ::std::option::Option< unsafe extern "C" fn( diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 3ec2954c73..015dd7f912 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -499,7 +499,7 @@ impl VALUE { pub fn embedded_p(self) -> bool { unsafe { - FL_TEST_RAW(self, VALUE(ROBJECT_EMBED as usize)) != VALUE(0) + FL_TEST_RAW(self, VALUE(ROBJECT_HEAP as usize)) == VALUE(0) } } diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 64e77948b8..12fc6b91fa 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -160,7 +160,7 @@ pub const RARRAY_EMBED_LEN_SHIFT: ruby_rarray_consts = 15; pub type ruby_rarray_consts = u32; pub const RMODULE_IS_REFINEMENT: ruby_rmodule_flags = 8192; pub type ruby_rmodule_flags = u32; -pub const ROBJECT_EMBED: ruby_robject_flags = 65536; +pub const ROBJECT_HEAP: ruby_robject_flags = 65536; pub type ruby_robject_flags = u32; pub const RUBY_ENCODING_INLINE_MAX: ruby_encoding_consts = 127; pub const RUBY_ENCODING_SHIFT: ruby_encoding_consts = 22;