8000 Refactor generic fields to use `T_IMEMO/fields` objects. by casperisfine · Pull Request #13626 · ruby/ruby · GitHub
[go: up one dir, main page]
Skip to content

Refactor generic fields to use T_IMEMO/fields objects. #13626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 17, 2025
Merged

Refactor generic fields to use T_IMEMO/fields objects. #13626

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d7554d5
Handle false positives in tool/auto-style.rb
byroot Jun 17, 2025
fee6cae
Rename `imemo_class_fields` -> `imemo_fields`
byroot Jun 16, 2025
63cf53d
Refactor `rb_imemo_fields_new` to not assume T_CLASS
byroot Jun 16, 2025
11c387b
Refactor generic fields to use `T_IMEMO/fields` objects.
byroot Jun 16, 2025
23a70aa
Add missing write barriers in `rb_imemo_fields_clone`.
byroot Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Refactor generic fields to use T_IMEMO/fields objects. 
Followup: #13589

This simplify a lot of things, as we no longer need to manually
manage the memory, we can use the Read-Copy-Update pattern and
avoid numerous race conditions.

Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>
  • Loading branch information
@byroot @etiennebarrie
byroot and etiennebarrie committed Jun 17, 2025
commit 11c387be868956e35720348a86d42b7d034011f8
9 changes: 5 additions & 4 deletions ext/objspace/objspace_dump.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,10 @@ dump_object(VALUE obj, struct dump_config *dc)

dc->cur_obj = obj;
dc->cur_obj_references = 0;
if (BUILTIN_TYPE(obj) == T_NODE || BUILTIN_TYPE(obj) == T_IMEMO) {
if (BUILTIN_TYPE(obj) == T_NODE || (BUILTIN_TYPE(obj) == T_IMEMO && !IMEMO_TYPE_P(obj, imemo_fields))) {
dc->cur_obj_klass = 0;
} else {
}
else {
dc->cur_obj_klass = RBASIC_CLASS(obj);
}

Expand All @@ -414,8 +415,8 @@ dump_object(VALUE obj, struct dump_config *dc)
dump_append(dc, obj_type(obj));
dump_append(dc, "\"");

if (BUILTIN_TYPE(obj) != T_IMEMO) {
size_t shape_id = rb_obj_shape_id(obj);
if (BUILTIN_TYPE(obj) != T_IMEMO || IMEMO_TYPE_P(obj, imemo_fields)) {
size_t shape_id = rb_obj_shape_id(obj) & SHAPE_ID_OFFSET_MASK;
dump_append(dc, ", \"shape_id\":");
dump_append_sizet(dc, shape_id);
}
Expand Down
130 changes: 37 additions & 93 deletions gc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2015,49 +2015,39 @@ object_id_to_ref(void *objspace_ptr, VALUE object_id)
static inline void
obj_free_object_id(VALUE obj)
{
if (RB_BUILTIN_TYPE(obj) == T_IMEMO) {
return;
}

#if RUBY_DEBUG
switch (BUILTIN_TYPE(obj)) {
case T_CLASS:
case T_MODULE:
break;
default:
if (rb_shape_obj_has_id(obj)) {
VALUE id = object_id_get(obj, RBASIC_SHAPE_ID(obj)); // Crash if missing
if (!(FIXNUM_P(id) || RB_TYPE_P(id, T_BIGNUM))) {
rb_p(obj);
rb_bug("Corrupted object_id");
}
}
break;
}
#endif

VALUE obj_id = 0;
if (RB_UNLIKELY(id2ref_tbl)) {
switch (BUILTIN_TYPE(obj)) {
case T_CLASS:
case T_MODULE:
obj_id = RCLASS(obj)->object_id;
break;
default: {
case T_IMEMO:
if (!IMEMO_TYPE_P(obj, imemo_fields)) {
return;
}
// fallthrough
case T_OBJECT:
{
shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
if (rb_shape_has_object_id(shape_id)) {
obj_id = object_id_get(obj, shape_id);
}
break;
}
default:
// For generic_fields, the T_IMEMO/fields is responsible for freeing the id.
return;
}

if (RB_UNLIKELY(obj_id)) {
RUBY_ASSERT(FIXNUM_P(obj_id) || RB_TYPE_P(obj_id, T_BIGNUM));

if (!st_delete(id2ref_tbl, (st_data_t *)&obj_id, NULL)) {
// If we're currently building the table then it's not a bug
if (id2ref_tbl_built) {
// If we're currently building the table then it's not a bug.
// The the object is a T_IMEMO/fields, then it's possible the actual object
// has been garbage collected already.
if (id2ref_tbl_built && !RB_TYPE_P(obj, T_IMEMO)) {
rb_bug("Object ID seen, but not in _id2ref table: object_id=%llu object=%s", NUM2ULL(obj_id), rb_obj_info(obj));
}
}
Expand All @@ -2071,7 +2061,7 @@ rb_gc_obj_free_vm_weak_references(VALUE obj)
obj_free_object_id(obj);

if (rb_obj_exivar_p(obj)) {
rb_free_generic_ivar((VALUE)obj);
rb_free_generic_ivar(obj);
}

switch (BUILTIN_TYPE(obj)) {
Expand Down Expand Up @@ -2316,10 +2306,6 @@ rb_obj_memsize_of(VALUE obj)
return 0;
}

if (rb_obj_exivar_p(obj)) {
size += rb_generic_ivar_memsize(obj);
}

switch (BUILTIN_TYPE(obj)) {
case T_OBJECT:
if (rb_shape_obj_too_complex_p(obj)) {
Expand Down Expand Up @@ -3935,38 +3921,6 @@ vm_weak_table_foreach_update_weak_value(st_data_t *key, st_data_t *value, st_dat
return iter_data->update_callback((VALUE *)value, iter_data->data);
}

static void
free_gen_fields_tbl(VALUE obj, struct gen_fields_tbl *fields_tbl)
{
if (UNLIKELY(rb_shape_obj_too_complex_p(obj))) {
st_free_table(fields_tbl->as.complex.table);
}

xfree(fields_tbl);
}

static int
vm_weak_table_gen_fields_foreach_too_complex_i(st_data_t _key, st_data_t value, st_data_t data, int error)
{
struct global_vm_table_foreach_data *iter_data = (struct global_vm_table_foreach_data *)data;

GC_ASSERT(!iter_data->weak_only);

if (SPECIAL_CONST_P((VALUE)value)) return ST_CONTINUE;

return iter_data->callback((VALUE)value, iter_data->data);
}

static int
vm_weak_table_gen_fields_foreach_too_complex_replace_i(st_data_t *_key, st_data_t *value, st_data_t data, int existing)
{
struct global_vm_table_foreach_data *iter_data = (struct global_vm_table_foreach_data *)data;

GC_ASSERT(!iter_data->weak_only);

return iter_data->update_callback((VALUE *)value, iter_data->data);
}

struct st_table *rb_generic_fields_tbl_get(void);

static int
Expand Down Expand Up @@ -4003,60 +3957,50 @@ vm_weak_table_gen_fields_foreach(st_data_t key, st_data_t value, st_data_t data)

int ret = iter_data->callback((VALUE)key, iter_data->data);

VALUE new_value = (VALUE)value;
VALUE new_key = (VALUE)key;

switch (ret) {
case ST_CONTINUE:
break;

case ST_DELETE:
free_gen_fields_tbl((VALUE)key, (struct gen_fields_tbl *)value);
RBASIC_SET_SHAPE_ID((VALUE)key, ROOT_SHAPE_ID);
return ST_DELETE;

case ST_REPLACE: {
VALUE new_key = (VALUE)key;
ret = iter_data->update_callback(&new_key, iter_data->data);
if (key != new_key) ret = ST_DELETE;
DURING_GC_COULD_MALLOC_REGION_START();
{
st_insert(rb_generic_fields_tbl_get(), (st_data_t)new_key, value);
if (key != new_key) {
ret = ST_DELETE;
}
DURING_GC_COULD_MALLOC_REGION_END();
key = (st_data_t)new_key;
break;
}

default:
return ret;
rb_bug("vm_weak_table_gen_fields_foreach: return value %d not supported", ret);
}

if (!iter_data->weak_only) {
struct gen_fields_tbl *fields_tbl = (struct gen_fields_tbl *)value;
int ivar_ret = iter_data->callback(new_value, iter_data->data);
switch (ivar_ret) {
case ST_CONTINUE:
break;

if (rb_shape_obj_too_complex_p((VALUE)key)) {
st_foreach_with_replace(
fields_tbl->as.complex.table,
vm_weak_table_gen_fields_foreach_too_complex_i,
vm_weak_table_gen_fields_foreach_too_complex_replace_i,
data
);
case ST_REPLACE:
iter_data->update_callback(&new_value, iter_data->data);
break;

default:
rb_bug("vm_weak_table_gen_fields_foreach: return value %d not supported", ivar_ret);
}
else {
uint32_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID((VALUE)key));
for (uint32_t i = 0; i < fields_count; i++) {
if (SPECIAL_CONST_P(fields_tbl->as.shape.fields[i])) continue;
}

int ivar_ret = iter_data->callback(fields_tbl->as.shape.fields[i], iter_data->data);
switch (ivar_ret) {
case ST_CONTINUE:
break;
case ST_REPLACE:
iter_data->update_callback(&fields_tbl->as.shape.fields[i], iter_data->data);
break;
default:
rb_bug("vm_weak_table_gen_fields_foreach: return value %d not supported", ivar_ret);
}
}
if (key != new_key || value != new_value) {
DURING_GC_COULD_MALLOC_REGION_START();
{
st_insert(rb_generic_fields_tbl_get(), (st_data_t)new_key, new_value);
}
DURING_GC_COULD_MALLOC_REGION_END();
}

return ret;
Expand Down
30 changes: 30 additions & 0 deletions imemo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,23 @@ rb_imemo_fields_new_complex(VALUE klass, size_t capa)
return imemo_fields_new_complex(klass, capa);
}

static int
imemo_fields_trigger_wb_i(st_data_t key, st_data_t value, st_data_t arg)
{
VALUE field_obj = (VALUE)arg;
RB_OBJ_WRITTEN(field_obj, Qundef, (VALUE)value);
return ST_CONTINUE;
}

VALUE
rb_imemo_fields_new_complex_tbl(VALUE klass, st_table *tbl)
{
VALUE fields = imemo_fields_new(klass, sizeof(struct rb_fields));
IMEMO_OBJ_FIELDS(fields)->as.complex.table = tbl;
st_foreach(tbl, imemo_fields_trigger_wb_i, (st_data_t)fields);
return fields;
}

VALUE
rb_imemo_fields_clone(VALUE fields_obj)
{
Expand All @@ -168,6 +185,19 @@ rb_imemo_fields_clone(VALUE fields_obj)
return clone;
}

void
rb_imemo_fields_clear(VALUE fields_obj)
{
// When replacing an imemo/fields by another one, we must clear
// its shape so that gc.c:obj_free_object_id won't be called.
if (rb_shape_obj_too_complex_p(fields_obj)) {
RBASIC_SET_SHAPE_ID(fields_obj, ROOT_TOO_COMPLEX_SHAPE_ID);
}
else {
RBASIC_SET_SHAPE_ID(fields_obj, ROOT_SHAPE_ID);
}
}

/* =========================================================================
* memsize
* ========================================================================= */
Expand Down
2 changes: 2 additions & 0 deletions internal/imemo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,9 @@ struct rb_fields {

VALUE rb_imemo_fields_new(VALUE klass, size_t capa);
VALUE rb_imemo_fields_new_complex(VALUE klass, size_t capa);
VALUE rb_imemo_fields_new_complex_tbl(VALUE klass, st_table *tbl);
VALUE rb_imemo_fields_clone(VALUE fields_obj);
void rb_imemo_fields_clear(VALUE fields_obj);

static inline VALUE *
rb_imemo_fields_ptr(VALUE obj_fields)
Expand Down
4 changes: 1 addition & 3 deletions internal/variable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
/* variable.c */
void rb_gc_mark_global_tbl(void);
void rb_gc_update_global_tbl(void);
size_t rb_generic_ivar_memsize(VALUE);
VALUE rb_search_class_path(VALUE);
VALUE rb_attr_delete(VALUE, ID);
void rb_autoload_str(VALUE mod, ID id, VALUE file);
Expand Down Expand Up @@ -47,8 +46,7 @@ void rb_gvar_namespace_ready(const char *name);
*/
VALUE rb_mod_set_temporary_name(VALUE, VALUE);

struct gen_fields_tbl;
int rb_gen_fields_tbl_get(VALUE obj, ID id, struct gen_fields_tbl **fields_tbl);
int rb_gen_fields_tbl_get(VALUE obj, ID id, VALUE *fields_obj);
void rb_obj_copy_ivs_to_hash_table(VALUE obj, st_table *table);
void rb_obj_init_too_complex(VALUE obj, st_table *table);
void rb_evict_ivars_to_hash(VALUE obj);
Expand Down
9 changes: 5 additions & 4 deletions ractor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1657,8 +1657,8 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data)
} while (0)

if (UNLIKELY(rb_obj_exivar_p(obj))) {
struct gen_fields_tbl *fields_tbl;
rb_ivar_generic_fields_tbl_lookup(obj, &fields_tbl);
VALUE fields_obj;
rb_ivar_generic_fields_tbl_lookup(obj, &fields_obj);

if (UNLIKELY(rb_shape_obj_too_complex_p(obj))) {
struct obj_traverse_replace_callback_data d = {
Expand All @@ -1667,7 +1667,7 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data)
.src = obj,
};
rb_st_foreach_with_replace(
fields_tbl->as.complex.table,
rb_imemo_fields_complex_tbl(fields_obj),
obj_iv_hash_traverse_replace_foreach_i,
obj_iv_hash_traverse_replace_i,
(st_data_t)&d
Expand All @@ -1676,8 +1676,9 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data)
}
else {
uint32_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID(obj));
VALUE *fields = rb_imemo_fields_ptr(fields_obj);
for (uint32_t i = 0; i < fields_count; i++) {
CHECK_AND_REPLACE(fields_tbl->as.shape.fields[i]);
CHECK_AND_REPLACE(fields[i]);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/ruby/test_encoding.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_singleton
encodings.each do |e|
assert_raise(TypeError) { e.dup }
assert_raise(TypeError) { e.clone }
assert_equal(e.object_id, Marshal.load(Marshal.dump(e)).object_id)
assert_same(e, Marshal.load(Marshal.dump(e)))
end
end

Expand Down
Loading
0