imemo_fields: store owner object in RBasic.klass

It is much more convenient than storing the klass, especially
when dealing with `object_id` as it allows to update the id2ref
table without having to dereference the owner, which may be
garbage at that point.

Author: byroot

gc.c

@@ -1921,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;
@@ -1965,15 +1965,16 @@ build_id2ref_i(VALUE obj, void *data)
         }
         break;
       case T_IMEMO:
-        if (IMEMO_TYPE_P(obj, imemo_fields) && rb_shape_obj_has_id(obj) && rb_gc_impl_garbage_object_p(objspace, obj)) {
-            rb_imemo_fields_clear(obj);
+        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;
-      case T_NONE:
-      case T_ZOMBIE:
-        break;
-      default:
+      case T_OBJECT:
         if (rb_shape_obj_has_id(obj)) {
             if (rb_gc_impl_garbage_object_p(objspace, obj)) {
                 RBASIC_SET_SHAPE_ID(obj, ROOT_SHAPE_ID);
@@ -1983,6 +1984,9 @@ build_id2ref_i(VALUE obj, void *data)
             }
         }
         break;
+      default:
+        // For generic_fields, the T_IMEMO/fields is responsible for populating the entry.
+        break;
     }
 }
 

imemo.c

@@ -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
@@ -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;
@@ -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);

internal/class.h

@@ -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;
 }

internal/imemo.h

@@ -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)
 {

shape.c

@@ -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);

test/ruby/test_shapes.rb

@@ -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

variable.c

@@ -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);
@@ -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);
@@ -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;
@@ -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;
         }
 
@@ -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);
@@ -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);
 }
@@ -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);
@@ -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;
@@ -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;
         }
 
@@ -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);