From c5158efb3b074dda0dba840a05e2bd1ca99ff70a Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 3 Jun 2024 11:29:43 +0200 Subject: [PATCH 1/2] Reimplement RData on top of RTypedData The goal is to eliminate `RTypedData.typed_flag` which waste `8B`, allowing `TypedData` to fit in a potential `32B` slot. This however makes `RData` 48B long, but given it's a deprecated construct it seems acceptable to degrade it to improve its replacement. --- gc.c | 129 +++++++++++++++--------- include/ruby/internal/core/rdata.h | 30 +++--- include/ruby/internal/core/rtypeddata.h | 80 ++++++++------- include/ruby/internal/gc.h | 1 - 4 files changed, 142 insertions(+), 98 deletions(-) diff --git a/gc.c b/gc.c index 5ee318c5afa2c6..e4bc90eb15535e 100644 --- a/gc.c +++ b/gc.c @@ -661,7 +661,7 @@ typedef struct RVALUE { struct RArray array; struct RRegexp regexp; struct RHash hash; - struct RData data; + struct RDataHeader data; struct RTypedData typeddata; struct RStruct rstruct; struct RBignum bignum; @@ -2974,35 +2974,19 @@ rb_wb_protected_newobj_of(rb_execution_context_t *ec, VALUE klass, VALUE flags, static inline void rb_data_object_check(VALUE klass) { - if (klass != rb_cObject && (rb_get_alloc_func(klass) == rb_class_allocate_instance)) { - rb_undef_alloc_func(klass); - rb_warn("undefining the allocator of T_DATA class %"PRIsVALUE, klass); - } -} - -VALUE -rb_data_object_wrap(VALUE klass, void *datap, RUBY_DATA_FUNC dmark, RUBY_DATA_FUNC dfree) -{ - 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)dfree, (VALUE)datap, !dmark, sizeof(struct RTypedData)); -} - -VALUE -rb_data_object_zalloc(VALUE klass, size_t size, RUBY_DATA_FUNC dmark, RUBY_DATA_FUNC dfree) -{ - VALUE obj = rb_data_object_wrap(klass, 0, dmark, dfree); - DATA_PTR(obj) = xcalloc(1, size); - return obj; + if (klass != rb_cObject && (rb_get_alloc_func(klass) == rb_class_allocate_instance)) { + rb_undef_alloc_func(klass); + rb_warn("undefining the allocator of T_DATA class %"PRIsVALUE, klass); + } } static VALUE -typed_data_alloc(VALUE klass, VALUE typed_flag, void *datap, const rb_data_type_t *type, size_t size) +typed_data_alloc(VALUE klass, const rb_data_type_t *type, void *datap, size_t size) { 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, 1 | typed_flag, (VALUE)datap, wb_protected, size); + return newobj_of(GET_RACTOR(), klass, T_DATA, (VALUE)type, (VALUE)datap, 0, wb_protected, size); } VALUE @@ -3012,7 +2996,7 @@ rb_data_typed_object_wrap(VALUE klass, void *datap, const rb_data_type_t *type) rb_raise(rb_eTypeError, "Cannot wrap an embeddable TypedData"); } - return typed_data_alloc(klass, 0, datap, type, sizeof(struct RTypedData)); + return typed_data_alloc(klass, type, datap, sizeof(struct RTypedData)); } VALUE @@ -3025,13 +3009,14 @@ rb_data_typed_object_zalloc(VALUE klass, size_t size, const rb_data_type_t *type size_t embed_size = offsetof(struct RTypedData, data) + size; if (rb_gc_size_allocatable_p(embed_size)) { - VALUE obj = typed_data_alloc(klass, TYPED_DATA_EMBEDDED, 0, type, embed_size); + VALUE obj = typed_data_alloc(klass, type, NULL, embed_size); memset((char *)obj + offsetof(struct RTypedData, data), 0, size); + FL_SET_RAW(obj, TYPED_DATA_FL_EMBEDDED); return obj; } } - VALUE obj = typed_data_alloc(klass, 0, NULL, type, sizeof(struct RTypedData)); + VALUE obj = typed_data_alloc(klass, type, NULL, sizeof(struct RTypedData)); DATA_PTR(obj) = xcalloc(1, size); return obj; } @@ -3044,7 +3029,7 @@ rb_objspace_data_type_memsize(VALUE obj) const rb_data_type_t *type = RTYPEDDATA_TYPE(obj); const void *ptr = RTYPEDDATA_GET_DATA(obj); - if (RTYPEDDATA_TYPE(obj)->flags & RUBY_TYPED_EMBEDDABLE && !RTYPEDDATA_EMBEDDED_P(obj)) { + if (RTYPEDDATA_TYPE(obj)->flags & RUBY_TYPED_EMBEDDABLE && !rbimpl_rtypeddata_embedded_p(obj)) { #ifdef HAVE_MALLOC_USABLE_SIZE size += malloc_usable_size((void *)ptr); #endif @@ -3069,6 +3054,56 @@ rb_objspace_data_type_name(VALUE obj) } } +static void +mark_deprecated_rdata_object(void *ptr) +{ + struct RData *rdata = (struct RData *)ptr; + if (rdata->dmark) { + rdata->dmark(rdata); + } +} + +static size_t +memsize_deprecated_rdata_object(const void *ptr) +{ + return sizeof(struct RData); +} + +#define DEPRECATED_DATA_FREE RBIMPL_DATA_FUNC(-3) + +const rb_data_type_t deprecated_rdata_type = { + .wrap_struct_name = "RDATA(deprecated)", + .function = { + .dmark = mark_deprecated_rdata_object, + .dfree = DEPRECATED_DATA_FREE, + .dsize = memsize_deprecated_rdata_object, + }, +}; + +VALUE +rb_data_object_wrap(VALUE klass, void *datap, RUBY_DATA_FUNC dmark, RUBY_DATA_FUNC dfree) +{ + RUBY_ASSERT_ALWAYS(dfree != (RUBY_DATA_FUNC)1); + if (klass) rb_data_object_check(klass); + + VALUE obj = rb_data_typed_object_zalloc(klass, sizeof(struct RData), &deprecated_rdata_type); + + struct RData *rdata = (struct RData *)obj; + rdata->dmark = dmark; + rdata->dfree = dfree; + rdata->data = datap; + return obj; +} + +VALUE +rb_data_object_zalloc(VALUE klass, size_t size, RUBY_DATA_FUNC dmark, RUBY_DATA_FUNC dfree) +{ + VALUE obj = rb_data_object_wrap(klass, 0, dmark, dfree); + struct RData *rdata = (struct RData *)obj; + rdata->data = xcalloc(1, size); + return obj; +} + static int ptr_in_page_body_p(const void *ptr, const void *memb) { @@ -3191,31 +3226,29 @@ obj_free_object_id(rb_objspace_t *objspace, VALUE obj) } static bool -rb_data_free(rb_objspace_t *objspace, VALUE obj) +rb_typeddata_free(rb_objspace_t *objspace, VALUE obj) { - void *data = RTYPEDDATA_P(obj) ? RTYPEDDATA_GET_DATA(obj) : DATA_PTR(obj); + void *data = RTYPEDDATA_GET_DATA(obj); if (data) { int free_immediately = false; - void (*dfree)(void *); - if (RTYPEDDATA_P(obj)) { - free_immediately = (RANY(obj)->as.typeddata.type->flags & RUBY_TYPED_FREE_IMMEDIATELY) != 0; - dfree = RANY(obj)->as.typeddata.type->function.dfree; - } - else { - dfree = RANY(obj)->as.data.dfree; + free_immediately = (RANY(obj)->as.typeddata.type->flags & RUBY_TYPED_FREE_IMMEDIATELY) != 0; + + RUBY_DATA_FUNC dfree = RANY(obj)->as.typeddata.type->function.dfree; + if (UNLIKELY(dfree == DEPRECATED_DATA_FREE)) { + dfree = RDATA(obj)->dfree; } if (dfree) { if (dfree == RUBY_DEFAULT_FREE) { - if (!RTYPEDDATA_EMBEDDED_P(obj)) { + if (!rbimpl_rtypeddata_embedded_p(obj)) { xfree(data); RB_DEBUG_COUNTER_INC(obj_data_xfree); } } else if (free_immediately) { (*dfree)(data); - if (RTYPEDDATA_TYPE(obj)->flags & RUBY_TYPED_EMBEDDABLE && !RTYPEDDATA_EMBEDDED_P(obj)) { + if (RTYPEDDATA_TYPE(obj)->flags & RUBY_TYPED_EMBEDDABLE && !rbimpl_rtypeddata_embedded_p(obj)) { xfree(data); } @@ -3373,7 +3406,7 @@ obj_free(rb_objspace_t *objspace, VALUE obj) } break; case T_DATA: - if (!rb_data_free(objspace, obj)) return false; + if (!rb_typeddata_free(objspace, obj)) return false; break; case T_MATCH: { @@ -4330,7 +4363,7 @@ rb_objspace_call_finalizer_i(VALUE obj, void *data) switch (BUILTIN_TYPE(obj)) { case T_DATA: - if (!rb_free_at_exit && (!DATA_PTR(obj) || !RANY(obj)->as.data.dfree)) break; + if (!rb_free_at_exit && (!DATA_PTR(obj) || !RDATA(obj)->dfree)) break; if (rb_obj_is_thread(obj)) break; if (rb_obj_is_mutex(obj)) break; if (rb_obj_is_fiber(obj)) break; @@ -6954,10 +6987,10 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj) case T_DATA: { - void *const ptr = RTYPEDDATA_P(obj) ? RTYPEDDATA_GET_DATA(obj) : DATA_PTR(obj); + void *const ptr = RTYPEDDATA_GET_DATA(obj); if (ptr) { - if (RTYPEDDATA_P(obj) && gc_declarative_marking_p(any->as.typeddata.type)) { + if (gc_declarative_marking_p(any->as.typeddata.type)) { size_t *offset_list = (size_t *)RANY(obj)->as.typeddata.type->function.dmark; for (size_t offset = *offset_list; offset != RUBY_REF_END; offset = *offset_list++) { @@ -6965,9 +6998,7 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj) } } else { - RUBY_DATA_FUNC mark_func = RTYPEDDATA_P(obj) ? - any->as.typeddata.type->function.dmark : - any->as.data.dmark; + RUBY_DATA_FUNC mark_func = any->as.typeddata.type->function.dmark; if (mark_func) (*mark_func)(ptr); } } @@ -10170,9 +10201,9 @@ gc_update_object_references(rb_objspace_t *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); + void *const ptr = RTYPEDDATA_GET_DATA(obj); if (ptr) { - if (RTYPEDDATA_P(obj) && gc_declarative_marking_p(any->as.typeddata.type)) { + if (gc_declarative_marking_p(any->as.typeddata.type)) { size_t *offset_list = (size_t *)RANY(obj)->as.typeddata.type->function.dmark; for (size_t offset = *offset_list; offset != RUBY_REF_END; offset = *offset_list++) { @@ -10181,7 +10212,7 @@ gc_update_object_references(rb_objspace_t *objspace, VALUE obj) *ref = rb_gc_location(*ref); } } - else if (RTYPEDDATA_P(obj)) { + else { RUBY_DATA_FUNC compact_func = any->as.typeddata.type->function.dcompact; if (compact_func) (*compact_func)(ptr); } @@ -13306,7 +13337,7 @@ rb_raw_obj_info_buitin_type(char *const buff, const size_t buff_size, const VALU rb_raw_iseq_info(BUFF_ARGS, iseq); } else if (rb_ractor_p(obj)) { - rb_ractor_t *r = (void *)DATA_PTR(obj); + rb_ractor_t *r = (void *)RTYPEDDATA_GET_DATA(obj); if (r) { APPEND_F("r:%d", r->pub.id); } diff --git a/include/ruby/internal/core/rdata.h b/include/ruby/internal/core/rdata.h index e4c146a716a8bb..76336db60cb760 100644 --- a/include/ruby/internal/core/rdata.h +++ b/include/ruby/internal/core/rdata.h @@ -30,6 +30,7 @@ #include "ruby/internal/attr/warning.h" #include "ruby/internal/cast.h" #include "ruby/internal/core/rbasic.h" +#include "ruby/internal/core/rtypeddata.h" #include "ruby/internal/dllexport.h" #include "ruby/internal/fl_type.h" #include "ruby/internal/value.h" @@ -77,6 +78,7 @@ */ #define RUBY_DEFAULT_FREE RBIMPL_DATA_FUNC(-1) + /** * This is a value you can set to ::RData::dfree. Setting this means the data * is managed by someone else, like, statically allocated. Of course you are @@ -93,16 +95,6 @@ */ #define RUBY_UNTYPED_DATA_FUNC(f) f RBIMPL_ATTRSET_UNTYPED_DATA_FUNC() -/* -#define RUBY_DATA_FUNC(func) ((void (*)(void*))(func)) -*/ - -/** - * This is the type of callbacks registered to ::RData. The argument is the - * `data` field. - */ -typedef void (*RUBY_DATA_FUNC)(void*); - /** * @deprecated * @@ -117,11 +109,26 @@ typedef void (*RUBY_DATA_FUNC)(void*); * too many warnings in the core. Maybe we want to retry later... Just add * deprecated document for now. */ +struct RDataHeader { + /** Basic part, including flags and class. */ + struct RBasic basic; + + const rb_data_type_t *const type; + + /** Pointer to the actual C level struct that you want to wrap. */ + void *data; +}; + struct RData { /** Basic part, including flags and class. */ struct RBasic basic; + const rb_data_type_t *const type; + + /** Pointer to the actual C level struct that you want to wrap. */ + void *data; + /** * This function is called when the object is experiencing GC marks. If it * contains references to other Ruby objects, you need to mark them also. @@ -141,9 +148,6 @@ 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. */ - void *data; }; RBIMPL_SYMBOL_EXPORT_BEGIN() diff --git a/include/ruby/internal/core/rtypeddata.h b/include/ruby/internal/core/rtypeddata.h index 6c19576c2007e6..81da0cf1c85d1b 100644 --- a/include/ruby/internal/core/rtypeddata.h +++ b/include/ruby/internal/core/rtypeddata.h @@ -33,7 +33,6 @@ #include "ruby/internal/attr/pure.h" #include "ruby/internal/cast.h" #include "ruby/internal/core/rbasic.h" -#include "ruby/internal/core/rdata.h" #include "ruby/internal/dllexport.h" #include "ruby/internal/error.h" #include "ruby/internal/fl_type.h" @@ -101,6 +100,16 @@ */ #define RTYPEDDATA_DATA(v) (RTYPEDDATA(v)->data) +/* +#define RUBY_DATA_FUNC(func) ((void (*)(void*))(func)) +*/ + +/** + * This is the type of callbacks registered to ::RData. The argument is the + * `data` field. + */ +typedef void (*RUBY_DATA_FUNC)(void*); + /** @old{rb_check_typeddata} */ #define Check_TypedStruct(v, t) \ rb_check_typeddata(RBIMPL_CAST((VALUE)(v)), (t)) @@ -112,10 +121,9 @@ #define RUBY_TYPED_FROZEN_SHAREABLE RUBY_TYPED_FROZEN_SHAREABLE #define RUBY_TYPED_WB_PROTECTED RUBY_TYPED_WB_PROTECTED #define RUBY_TYPED_PROMOTED1 RUBY_TYPED_PROMOTED1 +#define TYPED_DATA_FL_EMBEDDED RUBY_FL_USER0 /** @endcond */ -#define TYPED_DATA_EMBEDDED 2 - /** * @private * @@ -364,7 +372,7 @@ struct RTypedData { * * @internal */ - const VALUE typed_flag; + // const VALUE typed_flag; /** Pointer to the actual C level struct that you want to wrap. */ void *data; @@ -515,35 +523,7 @@ RBIMPL_SYMBOL_EXPORT_END() #define TypedData_Get_Struct(obj,type,data_type,sval) \ ((sval) = RBIMPL_CAST((type *)rb_check_typeddata((obj), (data_type)))) -static inline bool -RTYPEDDATA_EMBEDDED_P(VALUE obj) -{ -#if RUBY_DEBUG - if (RB_UNLIKELY(!RB_TYPE_P(obj, RUBY_T_DATA))) { - Check_Type(obj, RUBY_T_DATA); - RBIMPL_UNREACHABLE_RETURN(false); - } -#endif - - return RTYPEDDATA(obj)->typed_flag & TYPED_DATA_EMBEDDED; -} - -static inline void * -RTYPEDDATA_GET_DATA(VALUE obj) -{ -#if RUBY_DEBUG - if (RB_UNLIKELY(!RB_TYPE_P(obj, RUBY_T_DATA))) { - Check_Type(obj, RUBY_T_DATA); - RBIMPL_UNREACHABLE_RETURN(false); - } -#endif - - /* We reuse the data pointer in embedded TypedData. We can't use offsetof - * since RTypedData a non-POD type in C++. */ - const size_t embedded_typed_data_size = sizeof(struct RTypedData) - sizeof(void *); - - return RTYPEDDATA_EMBEDDED_P(obj) ? (char *)obj + embedded_typed_data_size : RTYPEDDATA(obj)->data; -} +extern const rb_data_type_t deprecated_rdata_type; RBIMPL_ATTR_PURE() RBIMPL_ATTR_ARTIFICIAL() @@ -561,8 +541,7 @@ RBIMPL_ATTR_ARTIFICIAL() static inline bool rbimpl_rtypeddata_p(VALUE obj) { - VALUE typed_flag = RTYPEDDATA(obj)->typed_flag; - return typed_flag != 0 && typed_flag <= 3; + return RTYPEDDATA(obj)->type != &deprecated_rdata_type; } RBIMPL_ATTR_PURE_UNLESS_DEBUG() @@ -588,6 +567,37 @@ RTYPEDDATA_P(VALUE obj) return rbimpl_rtypeddata_p(obj); } +static inline bool +rbimpl_rtypeddata_embedded_p(VALUE obj) +{ +#if RUBY_DEBUG + if (RB_UNLIKELY(!RB_TYPE_P(obj, RUBY_T_DATA))) { + Check_Type(obj, RUBY_T_DATA); + RBIMPL_UNREACHABLE_RETURN(false); + } +#endif + + return RTYPEDDATA_P(obj) && RTYPEDDATA(obj)->type->flags & RUBY_TYPED_EMBEDDABLE && FL_TEST_RAW(obj, TYPED_DATA_FL_EMBEDDED); +} + + +static inline void * +RTYPEDDATA_GET_DATA(VALUE obj) +{ +#if RUBY_DEBUG + if (RB_UNLIKELY(!RB_TYPE_P(obj, RUBY_T_DATA))) { + Check_Type(obj, RUBY_T_DATA); + RBIMPL_UNREACHABLE_RETURN(false); + } +#endif + + /* We reuse the data pointer in embedded TypedData. We can't use offsetof + * since RTypedData a non-POD type in C++. */ + const size_t embedded_typed_data_size = sizeof(struct RTypedData) - sizeof(void *); + + return rbimpl_rtypeddata_embedded_p(obj) ? (char *)obj + embedded_typed_data_size : RTYPEDDATA(obj)->data; +} + RBIMPL_ATTR_PURE_UNLESS_DEBUG() RBIMPL_ATTR_ARTIFICIAL() /* :TODO: can this function be __attribute__((returns_nonnull)) or not? */ diff --git a/include/ruby/internal/gc.h b/include/ruby/internal/gc.h index 462f416af21cfd..e05a51553fab97 100644 --- a/include/ruby/internal/gc.h +++ b/include/ruby/internal/gc.h @@ -825,5 +825,4 @@ rb_obj_write( RBIMPL_ATTR_DEPRECATED(("Will be removed soon")) static inline void rb_gc_force_recycle(VALUE obj){} - #endif /* RBIMPL_GC_H */ From 7a2ce88fe6f2cb49745a8cd9128d782057132b13 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 10 Jun 2024 10:51:51 +0200 Subject: [PATCH 2/2] rb_objspace_call_finalizer_i --- gc.c | 6 +++--- include/ruby/internal/core/rtypeddata.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gc.c b/gc.c index e4bc90eb15535e..ad379d8db7f470 100644 --- a/gc.c +++ b/gc.c @@ -3071,7 +3071,7 @@ memsize_deprecated_rdata_object(const void *ptr) #define DEPRECATED_DATA_FREE RBIMPL_DATA_FUNC(-3) -const rb_data_type_t deprecated_rdata_type = { +const rb_data_type_t ruby_deprecated_rdata_type = { .wrap_struct_name = "RDATA(deprecated)", .function = { .dmark = mark_deprecated_rdata_object, @@ -3086,7 +3086,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); - VALUE obj = rb_data_typed_object_zalloc(klass, sizeof(struct RData), &deprecated_rdata_type); + VALUE obj = rb_data_typed_object_zalloc(klass, sizeof(struct RData), &ruby_deprecated_rdata_type); struct RData *rdata = (struct RData *)obj; rdata->dmark = dmark; @@ -4363,7 +4363,7 @@ rb_objspace_call_finalizer_i(VALUE obj, void *data) switch (BUILTIN_TYPE(obj)) { case T_DATA: - if (!rb_free_at_exit && (!DATA_PTR(obj) || !RDATA(obj)->dfree)) break; + if (!rb_free_at_exit && (!RTYPEDDATA_GET_DATA(obj) || !RANY(obj)->as.typeddata.type->function.dfree)) break; if (rb_obj_is_thread(obj)) break; if (rb_obj_is_mutex(obj)) break; if (rb_obj_is_fiber(obj)) break; diff --git a/include/ruby/internal/core/rtypeddata.h b/include/ruby/internal/core/rtypeddata.h index 81da0cf1c85d1b..1d8b1183bb1d20 100644 --- a/include/ruby/internal/core/rtypeddata.h +++ b/include/ruby/internal/core/rtypeddata.h @@ -523,7 +523,6 @@ RBIMPL_SYMBOL_EXPORT_END() #define TypedData_Get_Struct(obj,type,data_type,sval) \ ((sval) = RBIMPL_CAST((type *)rb_check_typeddata((obj), (data_type)))) -extern const rb_data_type_t deprecated_rdata_type; RBIMPL_ATTR_PURE() RBIMPL_ATTR_ARTIFICIAL() @@ -541,7 +540,8 @@ RBIMPL_ATTR_ARTIFICIAL() static inline bool rbimpl_rtypeddata_p(VALUE obj) { - return RTYPEDDATA(obj)->type != &deprecated_rdata_type; + extern const rb_data_type_t ruby_deprecated_rdata_type; + return RTYPEDDATA(obj)->type != &ruby_deprecated_rdata_type; } RBIMPL_ATTR_PURE_UNLESS_DEBUG()