Skip to content

imemo_fields: store owner object in RBasic.klass #14147

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

Closed
wants to merge 2 commits into from
+98 −53
Closed

imemo_fields: store owner object in RBasic.klass #14147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4f7435
gc.c: Filter garbage in build_id2ref_i
byroot Aug 8, 2025
1986bca
imemo_fields: store owner object in RBasic.klass
byroot Aug 8, 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
67 changes: 47 additions & 20 deletions gc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1755,7 +1755,6 @@ rb_gc_pointer_to_heap_p(VALUE obj)
#define LAST_OBJECT_ID() (object_id_counter * OBJ_ID_INCREMENT)
static VALUE id2ref_value = 0;
static st_table *id2ref_tbl = NULL;
static bool id2ref_tbl_built = false;

#if SIZEOF_SIZE_T == SIZEOF_LONG_LONG
static size_t object_id_counter = 1;
Expand Down Expand Up @@ -1922,7 +1921,7 @@ object_id(VALUE obj)
// in fields.
return class_object_id(obj);
case T_IMEMO:
rb_bug("T_IMEMO can't have an object_id");
RUBY_ASSERT(IMEMO_TYPE_P(obj, imemo_fields));
break;
default:
break;
Expand All @@ -1938,26 +1937,56 @@ object_id(VALUE obj)
return object_id0(obj);
}

struct build_id2ref_args {
st_table *id2ref_tbl;
rb_objspace_t *objspace;
};

static void
build_id2ref_i(VALUE obj, void *data)
{
st_table *id2ref_tbl = (st_table *)data;
struct build_id2ref_args *args = (struct build_id2ref_args *)data;
st_table *id2ref_tbl = args->id2ref_tbl;
rb_objspace_t *objspace = args->objspace;

// If the object is garbage we must not insert it.
// But also we must clear its object id, otherwise when it will be finally freed
// obj_free_object_id will crash.
switch (BUILTIN_TYPE(obj)) {
case T_CLASS:
case T_MODULE:
if (RCLASS(obj)->object_id) {
st_insert(id2ref_tbl, RCLASS(obj)->object_id, obj);
if (rb_gc_impl_garbage_object_p(objspace, obj)) {
RCLASS(obj)->object_id = 0;
}
else {
st_insert(id2ref_tbl, RCLASS(obj)->object_id, obj);
}
}
break;
case T_IMEMO:
case T_NONE:
if (IMEMO_TYPE_P(obj, imemo_fields) && rb_shape_obj_has_id(obj)) {
if (rb_gc_impl_garbage_object_p(objspace, obj)) {
RBASIC_SET_SHAPE_ID(obj, ROOT_SHAPE_ID);
}
else {
st_insert(id2ref_tbl, rb_obj_id(obj), rb_imemo_fields_owner(obj));
}
}
break;
default:
case T_OBJECT:
if (rb_shape_obj_has_id(obj)) {
st_insert(id2ref_tbl, rb_obj_id(obj), obj);
if (rb_gc_impl_garbage_object_p(objspace, obj)) {
RBASIC_SET_SHAPE_ID(obj, ROOT_SHAPE_ID);
}
else {
st_insert(id2ref_tbl, rb_obj_id(obj), obj);
}
}
break;
default:
// For generic_fields, the T_IMEMO/fields is responsible for populating the entry.
break;
}
}

Expand All @@ -1981,10 +2010,13 @@ object_id_to_ref(void *objspace_ptr, VALUE object_id)
// objects we just added to the table.
bool gc_disabled = RTEST(rb_gc_disable_no_rest());
{
rb_gc_impl_each_object(objspace, build_id2ref_i, (void *)id2ref_tbl);
struct build_id2ref_args args = {
.id2ref_tbl = id2ref_tbl,
.objspace = objspace,
};
rb_gc_impl_each_object(objspace, build_id2ref_i, (void *)&args);
}
if (!gc_disabled) rb_gc_enable();
id2ref_tbl_built = true;
}

VALUE obj;
Expand Down Expand Up @@ -2021,12 +2053,12 @@ obj_free_object_id(VALUE obj)
// 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);
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;
}
break;
}
default:
// For generic_fields, the T_IMEMO/fields is responsible for freeing the id.
return;
Expand All @@ -2036,12 +2068,7 @@ obj_free_object_id(VALUE obj)
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.
// 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));
}
rb_bug("Object ID seen, but not in _id2ref table: object_id=%llu object=%s", NUM2ULL(obj_id), rb_obj_info(obj));
}
}
}
Expand Down
26 changes: 13 additions & 13 deletions imemo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,40 +109,40 @@ rb_imemo_tmpbuf_parser_heap(void *buf, rb_imemo_tmpbuf_t *old_heap, size_t cnt)
}

static VALUE
imemo_fields_new(VALUE klass, size_t capa)
imemo_fields_new(VALUE owner, size_t capa)
{
size_t embedded_size = offsetof(struct rb_fields, as.embed) + capa * sizeof(VALUE);
if (rb_gc_size_allocatable_p(embedded_size)) {
VALUE fields = rb_imemo_new(imemo_fields, klass, embedded_size);
VALUE fields = rb_imemo_new(imemo_fields, owner, embedded_size);
RUBY_ASSERT(IMEMO_TYPE_P(fields, imemo_fields));
return fields;
}
else {
VALUE fields = rb_imemo_new(imemo_fields, klass, sizeof(struct rb_fields));
VALUE fields = rb_imemo_new(imemo_fields, owner, sizeof(struct rb_fields));
FL_SET_RAW(fields, OBJ_FIELD_EXTERNAL);
IMEMO_OBJ_FIELDS(fields)->as.external.ptr = ALLOC_N(VALUE, capa);
return fields;
}
}

VALUE
rb_imemo_fields_new(VALUE klass, size_t capa)
rb_imemo_fields_new(VALUE owner, size_t capa)
{
return imemo_fields_new(klass, capa);
return imemo_fields_new(owner, capa);
}

static VALUE
imemo_fields_new_complex(VALUE klass, size_t capa)
imemo_fields_new_complex(VALUE owner, size_t capa)
{
VALUE fields = imemo_fields_new(klass, sizeof(struct rb_fields));
VALUE fields = imemo_fields_new(owner, sizeof(struct rb_fields));
IMEMO_OBJ_FIELDS(fields)->as.complex.table = st_init_numtable_with_size(capa);
return fields;
}

VALUE
rb_imemo_fields_new_complex(VALUE klass, size_t capa)
rb_imemo_fields_new_complex(VALUE owner, size_t capa)
{
return imemo_fields_new_complex(klass, capa);
return imemo_fields_new_complex(owner, capa);
}

static int
Expand All @@ -161,9 +161,9 @@ imemo_fields_complex_wb_i(st_data_t key, st_data_t value, st_data_t arg)
}

VALUE
rb_imemo_fields_new_complex_tbl(VALUE klass, st_table *tbl)
rb_imemo_fields_new_complex_tbl(VALUE owner, st_table *tbl)
{
VALUE fields = imemo_fields_new(klass, sizeof(struct rb_fields));
VALUE fields = imemo_fields_new(owner, 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;
Expand All @@ -176,15 +176,15 @@ rb_imemo_fields_clone(VALUE fields_obj)
VALUE clone;

if (rb_shape_too_complex_p(shape_id)) {
clone = rb_imemo_fields_new_complex(CLASS_OF(fields_obj), 0);
clone = rb_imemo_fields_new_complex(rb_imemo_fields_owner(fields_obj), 0);
RBASIC_SET_SHAPE_ID(clone, shape_id);
st_table *src_table = rb_imemo_fields_complex_tbl(fields_obj);
st_table *dest_table = rb_imemo_fields_complex_tbl(clone);
st_replace(dest_table, src_table);
st_foreach(dest_table, imemo_fields_complex_wb_i, (st_data_t)clone);
}
else {
clone = imemo_fields_new(CLASS_OF(fields_obj), RSHAPE_CAPACITY(shape_id));
clone = imemo_fields_new(rb_imemo_fields_owner(fields_obj), RSHAPE_CAPACITY(shape_id));
RBASIC_SET_SHAPE_ID(clone, shape_id);
VALUE *fields = rb_imemo_fields_ptr(clone);
attr_index_t fields_count = RSHAPE_LEN(shape_id);
Expand Down
2 changes: 1 addition & 1 deletion internal/class.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ RCLASS_WRITABLE_ENSURE_FIELDS_OBJ(VALUE obj)
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
rb_classext_t *ext = RCLASS_EXT_WRITABLE(obj);
if (!ext->fields_obj) {
RB_OBJ_WRITE(obj, &ext->fields_obj, rb_imemo_fields_new(rb_singleton_class(obj), 1));
RB_OBJ_WRITE(obj, &ext->fields_obj, rb_imemo_fields_new(obj, 1));
}
return ext->fields_obj;
}
Expand Down
12 changes: 9 additions & 3 deletions internal/imemo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,18 @@ struct rb_fields {
#define OBJ_FIELD_EXTERNAL IMEMO_FL_USER0
#define IMEMO_OBJ_FIELDS(fields) ((struct rb_fields *)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_new(VALUE owner, size_t capa);
VALUE rb_imemo_fields_new_complex(VALUE owner, size_t capa);
VALUE rb_imemo_fields_new_complex_tbl(VALUE owner, 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_owner(VALUE fields_obj)
{
return CLASS_OF(fields_obj);
}

static inline VALUE *
rb_imemo_fields_ptr(VALUE obj_fields)
{
Expand Down
13 changes: 11 additions & 2 deletions shape.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,8 +877,17 @@ shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings)
#endif

VALUE klass;
if (IMEMO_TYPE_P(obj, imemo_fields)) { // HACK
klass = CLASS_OF(obj);
if (IMEMO_TYPE_P(obj, imemo_fields)) {
VALUE owner = rb_imemo_fields_owner(obj);
switch (BUILTIN_TYPE(owner)) {
case T_CLASS:
case T_MODULE:
klass = rb_singleton_class(owner);
break;
default:
klass = rb_obj_class(owner);
break;
}
}
else {
klass = rb_obj_class(obj);
Expand Down
7 changes: 5 additions & 2 deletions test/ruby/test_shapes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,14 @@ class Hi; end
def test_too_many_ivs_on_class
obj = Class.new

(MANY_IVS + 1).times do
obj.instance_variable_set(:@test_too_many_ivs_on_class, 1)
refute_predicate RubyVM::Shape.of(obj), :too_complex?

MANY_IVS.times do
obj.instance_variable_set(:"@a#{_1}", 1)
end

assert_false RubyVM::Shape.of(obj).too_complex?
refute_predicate RubyVM::Shape.of(obj), :too_complex?
end

def test_removing_when_too_many_ivs_on_class
Expand Down
24 changes: 12 additions & 12 deletions variable.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1666,10 +1666,10 @@ imemo_fields_complex_from_obj_i(ID key, VALUE val, st_data_t arg)
}

static VALUE
imemo_fields_complex_from_obj(VALUE klass, VALUE source_fields_obj, shape_id_t shape_id)
imemo_fields_complex_from_obj(VALUE owner, VALUE source_fields_obj, shape_id_t shape_id)
{
attr_index_t len = source_fields_obj ? RSHAPE_LEN(RBASIC_SHAPE_ID(source_fields_obj)) : 0;
VALUE fields_obj = rb_imemo_fields_new_complex(klass, len + 1);
VALUE fields_obj = rb_imemo_fields_new_complex(owner, len + 1);

rb_field_foreach(source_fields_obj, imemo_fields_complex_from_obj_i, (st_data_t)fields_obj, false);
RBASIC_SET_SHAPE_ID(fields_obj, shape_id);
Expand All @@ -1678,9 +1678,9 @@ imemo_fields_complex_from_obj(VALUE klass, VALUE source_fields_obj, shape_id_t s
}

static VALUE
imemo_fields_copy_capa(VALUE klass, VALUE source_fields_obj, attr_index_t new_size)
imemo_fields_copy_capa(VALUE owner, VALUE source_fields_obj, attr_index_t new_size)
{
VALUE fields_obj = rb_imemo_fields_new(klass, new_size);
VALUE fields_obj = rb_imemo_fields_new(owner, new_size);
if (source_fields_obj) {
attr_index_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID(source_fields_obj));
VALUE *fields = rb_imemo_fields_ptr(fields_obj);
Expand Down Expand Up @@ -1832,7 +1832,7 @@ general_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val, void *data,
}

static VALUE
imemo_fields_set(VALUE klass, VALUE fields_obj, shape_id_t target_shape_id, ID field_name, VALUE val, bool concurrent)
imemo_fields_set(VALUE owner, VALUE fields_obj, shape_id_t target_shape_id, ID field_name, VALUE val, bool concurrent)
{
const VALUE original_fields_obj = fields_obj;
shape_id_t current_shape_id = fields_obj ? RBASIC_SHAPE_ID(fields_obj) : ROOT_SHAPE_ID;
Expand All @@ -1847,7 +1847,7 @@ imemo_fields_set(VALUE klass, VALUE fields_obj, shape_id_t target_shape_id, ID f
}
}
else {
fields_obj = imemo_fields_complex_from_obj(klass, original_fields_obj, target_shape_id);
fields_obj = imemo_fields_complex_from_obj(owner, original_fields_obj, target_shape_id);
current_shape_id = target_shape_id;
}

Expand All @@ -1861,7 +1861,7 @@ imemo_fields_set(VALUE klass, VALUE fields_obj, shape_id_t target_shape_id, ID f
else {
attr_index_t index = RSHAPE_INDEX(target_shape_id);
if (concurrent || index >= RSHAPE_CAPACITY(current_shape_id)) {
fields_obj = imemo_fields_copy_capa(klass, original_fields_obj, RSHAPE_CAPACITY(target_shape_id));
fields_obj = imemo_fields_copy_capa(owner, original_fields_obj, RSHAPE_CAPACITY(target_shape_id));
}

VALUE *table = rb_imemo_fields_ptr(fields_obj);
Expand All @@ -1884,7 +1884,7 @@ generic_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE va
}

const VALUE original_fields_obj = rb_obj_fields(obj, field_name);
VALUE fields_obj = imemo_fields_set(rb_obj_class(obj), original_fields_obj, target_shape_id, field_name, val, false);
VALUE fields_obj = imemo_fields_set(obj, original_fields_obj, target_shape_id, field_name, val, false);

rb_obj_set_fields(obj, fields_obj, field_name, original_fields_obj);
}
Expand Down Expand Up @@ -2319,7 +2319,7 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj)
return;
}

new_fields_obj = rb_imemo_fields_new(rb_obj_class(dest), RSHAPE_CAPACITY(dest_shape_id));
new_fields_obj = rb_imemo_fields_new(dest, RSHAPE_CAPACITY(dest_shape_id));
VALUE *src_buf = rb_imemo_fields_ptr(fields_obj);
VALUE *dest_buf = rb_imemo_fields_ptr(new_fields_obj);
rb_shape_copy_fields(new_fields_obj, dest_buf, dest_shape_id, src_buf, src_shape_id);
Expand Down Expand Up @@ -4640,7 +4640,7 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc
{
bool existing = true;
const VALUE original_fields_obj = fields_obj;
fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_fields_new(rb_singleton_class(klass), 1);
fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_fields_new(klass, 1);

shape_id_t current_shape_id = RBASIC_SHAPE_ID(fields_obj);
shape_id_t next_shape_id = current_shape_id;
Expand All @@ -4660,7 +4660,7 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc

next_shape_id = rb_shape_transition_add_ivar(fields_obj, id);
if (UNLIKELY(rb_shape_too_complex_p(next_shape_id))) {
fields_obj = imemo_fields_complex_from_obj(rb_singleton_class(klass), fields_obj, next_shape_id);
fields_obj = imemo_fields_complex_from_obj(klass, fields_obj, next_shape_id);
goto too_complex;
}

Expand All @@ -4670,7 +4670,7 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc
if (next_capacity > current_capacity) {
// We allocate a new fields_obj even when concurrency isn't a concern
// so that we're embedded as long as possible.
fields_obj = imemo_fields_copy_capa(rb_singleton_class(klass), fields_obj, next_capacity);
fields_obj = imemo_fields_copy_capa(klass, fields_obj, next_capacity);
}

RUBY_ASSERT(RSHAPE(next_shape_id)->type == SHAPE_IVAR);
Expand Down
Loading