From 360be94d0492f766b08cc39e33f5e248f49a89b7 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 6 Aug 2025 19:47:08 +0200 Subject: [PATCH] RTypedData: keep direct reference to IMEMO/fields Similar to f3206cc79bec2fd852e81ec56de59f0a67ab32b7 but for TypedData. It's quite common for TypedData objects to have a mix of reference in their struct and some ivars. Since we do happen to have 8B free in the RtypedData struct, we could use it to keep a direct reference to the IMEMO/fields saving having to synchronize the VM and lookup the `gen_fields_tbl` on every ivar access. For old school Data classes however, we don't have free space, but this API is soft-deprecated and no longer very common. --- gc.c | 27 +++++++++++++++++-------- include/ruby/internal/abi.h | 2 +- include/ruby/internal/core/rdata.h | 12 +++++------ include/ruby/internal/core/rtypeddata.h | 3 +++ variable.c | 27 ++++++++++++++++++++++--- 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/gc.c b/gc.c index 7663e82f41..5cfef6ff2c 100644 --- a/gc.c +++ b/gc.c @@ -1047,7 +1047,7 @@ rb_data_object_wrap(VALUE klass, void *datap, RUBY_DATA_FUNC dmark, RUBY_DATA_FU { RUBY_ASSERT_ALWAYS(dfree != (RUBY_DATA_FUNC)1); if (klass) rb_data_object_check(klass); - return newobj_of(GET_RACTOR(), klass, T_DATA, (VALUE)dmark, (VALUE)datap, (VALUE)dfree, !dmark, sizeof(struct RTypedData)); + return newobj_of(GET_RACTOR(), klass, T_DATA, (VALUE)dmark, (VALUE)dfree, (VALUE)datap, !dmark, sizeof(struct RTypedData)); } VALUE @@ -1064,7 +1064,7 @@ typed_data_alloc(VALUE klass, VALUE typed_flag, void *datap, const rb_data_type_ RBIMPL_NONNULL_ARG(type); if (klass) rb_data_object_check(klass); bool wb_protected = (type->flags & RUBY_FL_WB_PROTECTED) || !type->function.dmark; - return newobj_of(GET_RACTOR(), klass, T_DATA, ((VALUE)type) | IS_TYPED_DATA | typed_flag, (VALUE)datap, 0, wb_protected, size); + return newobj_of(GET_RACTOR(), klass, T_DATA, 0, ((VALUE)type) | IS_TYPED_DATA | typed_flag, (VALUE)datap, wb_protected, size); } VALUE @@ -3173,10 +3173,15 @@ rb_gc_mark_children(void *objspace, VALUE obj) break; case T_DATA: { - void *const ptr = RTYPEDDATA_P(obj) ? RTYPEDDATA_GET_DATA(obj) : DATA_PTR(obj); + bool typed_data = RTYPEDDATA_P(obj); + void *const ptr = typed_data ? RTYPEDDATA_GET_DATA(obj) : DATA_PTR(obj); + + if (typed_data) { + gc_mark_internal(RTYPEDDATA(obj)->fields_obj); + } if (ptr) { - if (RTYPEDDATA_P(obj) && gc_declarative_marking_p(RTYPEDDATA_TYPE(obj))) { + if (typed_data && gc_declarative_marking_p(RTYPEDDATA_TYPE(obj))) { size_t *offset_list = TYPED_DATA_REFS_OFFSET_LIST(obj); for (size_t offset = *offset_list; offset != RUBY_REF_END; offset = *offset_list++) { @@ -3184,7 +3189,7 @@ rb_gc_mark_children(void *objspace, VALUE obj) } } else { - RUBY_DATA_FUNC mark_func = RTYPEDDATA_P(obj) ? + RUBY_DATA_FUNC mark_func = typed_data ? RTYPEDDATA_TYPE(obj)->function.dmark : RDATA(obj)->dmark; if (mark_func) (*mark_func)(ptr); @@ -4121,9 +4126,15 @@ rb_gc_update_object_references(void *objspace, VALUE obj) case T_DATA: /* Call the compaction callback, if it exists */ { - void *const ptr = RTYPEDDATA_P(obj) ? RTYPEDDATA_GET_DATA(obj) : DATA_PTR(obj); + bool typed_data = RTYPEDDATA_P(obj); + void *const ptr = typed_data ? RTYPEDDATA_GET_DATA(obj) : DATA_PTR(obj); + + if (typed_data) { + UPDATE_IF_MOVED(objspace, RTYPEDDATA(obj)->fields_obj); + } + if (ptr) { - if (RTYPEDDATA_P(obj) && gc_declarative_marking_p(RTYPEDDATA_TYPE(obj))) { + if (typed_data && gc_declarative_marking_p(RTYPEDDATA_TYPE(obj))) { size_t *offset_list = TYPED_DATA_REFS_OFFSET_LIST(obj); for (size_t offset = *offset_list; offset != RUBY_REF_END; offset = *offset_list++) { @@ -4131,7 +4142,7 @@ rb_gc_update_object_references(void *objspace, VALUE obj) *ref = gc_location_internal(objspace, *ref); } } - else if (RTYPEDDATA_P(obj)) { + else if (typed_data) { RUBY_DATA_FUNC compact_func = RTYPEDDATA_TYPE(obj)->function.dcompact; if (compact_func) (*compact_func)(ptr); } diff --git a/include/ruby/internal/abi.h b/include/ruby/internal/abi.h index 0c99d93bf9..7ceb8c40b7 100644 --- a/include/ruby/internal/abi.h +++ b/include/ruby/internal/abi.h @@ -24,7 +24,7 @@ * In released versions of Ruby, this number is not defined since teeny * versions of Ruby should guarantee ABI compatibility. */ -#define RUBY_ABI_VERSION 2 +#define RUBY_ABI_VERSION 3 /* Windows does not support weak symbols so ruby_abi_version will not exist * in the shared library. */ diff --git a/include/ruby/internal/core/rdata.h b/include/ruby/internal/core/rdata.h index cab412af72..bebb2a8822 100644 --- a/include/ruby/internal/core/rdata.h +++ b/include/ruby/internal/core/rdata.h @@ -133,12 +133,6 @@ struct RData { */ RUBY_DATA_FUNC dmark; - /** Pointer to the actual C level struct that you want to wrap. - * This is in between dmark and dfree to allow DATA_PTR to continue - * to work for both RData and non-embedded RTypedData. - */ - void *data; - /** * This function is called when the object is no longer used. You need to * do whatever necessary to avoid memory leaks. @@ -147,6 +141,12 @@ struct RData { * impossible at that moment (that is why GC runs). */ RUBY_DATA_FUNC dfree; + + /** Pointer to the actual C level struct that you want to wrap. + * This is in between dmark and dfree to allow DATA_PTR to continue + * to work for both RData and non-embedded RTypedData. + */ + void *data; }; RBIMPL_SYMBOL_EXPORT_BEGIN() diff --git a/include/ruby/internal/core/rtypeddata.h b/include/ruby/internal/core/rtypeddata.h index edf482267a..539a2f86d2 100644 --- a/include/ruby/internal/core/rtypeddata.h +++ b/include/ruby/internal/core/rtypeddata.h @@ -355,6 +355,9 @@ struct RTypedData { /** The part that all ruby objects have in common. */ struct RBasic basic; + /** Direct reference to the slots that holds instance variables, if any **/ + VALUE fields_obj; + /** * This is a `const rb_data_type_t *const` value, with the low bits set: * diff --git a/variable.c b/variable.c index e0a85b8f48..7e5875519d 100644 --- a/variable.c +++ b/variable.c @@ -1231,13 +1231,20 @@ rb_obj_fields(VALUE obj, ID field_name) VALUE fields_obj = 0; if (rb_shape_obj_has_fields(obj)) { switch (BUILTIN_TYPE(obj)) { + case T_DATA: + if (LIKELY(RTYPEDDATA_P(obj))) { + fields_obj = RTYPEDDATA(obj)->fields_obj; + break; + } + goto generic_fields; case T_STRUCT: if (LIKELY(!FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS))) { fields_obj = RSTRUCT_FIELDS_OBJ(obj); break; } - // fall through + goto generic_fields; default: + generic_fields: RB_VM_LOCKING() { if (!st_lookup(generic_fields_tbl_, (st_data_t)obj, (st_data_t *)&fields_obj)) { rb_bug("Object is missing entry in generic_fields_tbl"); @@ -1254,13 +1261,20 @@ rb_free_generic_ivar(VALUE obj) if (rb_obj_exivar_p(obj)) { st_data_t key = (st_data_t)obj, value; switch (BUILTIN_TYPE(obj)) { + case T_DATA: + if (LIKELY(RTYPEDDATA_P(obj))) { + RB_OBJ_WRITE(obj, &RTYPEDDATA(obj)->fields_obj, 0); + break; + } + goto generic_fields; case T_STRUCT: if (LIKELY(!FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS))) { RSTRUCT_SET_FIELDS_OBJ(obj, 0); break; } - // fall through + goto generic_fields; default: + generic_fields: RB_VM_LOCKING() { st_delete(generic_fields_tbl_no_ractor_check(), &key, &value); } @@ -1279,13 +1293,20 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie if (fields_obj != original_fields_obj) { switch (BUILTIN_TYPE(obj)) { + case T_DATA: + if (LIKELY(RTYPEDDATA_P(obj))) { + RB_OBJ_WRITE(obj, &RTYPEDDATA(obj)->fields_obj, fields_obj); + break; + } + goto generic_fields; case T_STRUCT: if (LIKELY(!FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS))) { RSTRUCT_SET_FIELDS_OBJ(obj, fields_obj); break; } - // fall through + goto generic_fields; default: + generic_fields: RB_VM_LOCKING() { st_insert(generic_fields_tbl_, (st_data_t)obj, (st_data_t)fields_obj); }