Allow T_CLASS and generic types to be too_complex

The intial complex shape implementation never allowed objects
other than T_OBJECT to become too complex, unless we run out of
shapes.

I don't see any reason to prevent that.

Ref: ruby#6931

Author: byroot

gc.c

@@ -1872,26 +1872,7 @@ object_id(VALUE obj)
     // we'd at least need to generate the object_id using atomics.
     lock_lev = rb_gc_vm_lock();
 
-    if (rb_shape_too_complex_p(shape)) {
-        st_table *table = ROBJECT_FIELDS_HASH(obj);
-        if (rb_shape_has_object_id(shape)) {
-            st_lookup(table, (st_data_t)ruby_internal_object_id, (st_data_t *)&id);
-            RUBY_ASSERT(id, "object_id missing");
-
-            rb_gc_vm_unlock(lock_lev);
-            return id;
-        }
-
-        id = ULL2NUM(next_object_id);
-        next_object_id += OBJ_ID_INCREMENT;
-        rb_shape_t *object_id_shape = rb_shape_object_id_shape(obj);
-        st_insert(table, (st_data_t)ruby_internal_object_id, (st_data_t)id);
-        rb_shape_set_shape(obj, object_id_shape);
-        if (RB_UNLIKELY(id_to_obj_tbl)) {
-            st_insert(id_to_obj_tbl, (st_data_t)id, (st_data_t)obj);
-        }
-    }
-    else if (rb_shape_has_object_id(shape)) {
+    if (rb_shape_has_object_id(shape)) {
         rb_shape_t *object_id_shape = rb_shape_object_id_shape(obj);
         id = rb_obj_field_get(obj, object_id_shape);
     }

shape.c

@@ -799,35 +799,38 @@ shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings)
     }
 #endif
 
-    bool allow_new_shape = true;
-
-    if (BUILTIN_TYPE(obj) == T_OBJECT) {
-        VALUE klass = rb_obj_class(obj);
-        allow_new_shape = RCLASS_VARIATION_COUNT(klass) < SHAPE_MAX_VARIATIONS;
+    VALUE klass;
+    switch (BUILTIN_TYPE(obj)) {
+      case T_CLASS:
+      case T_MODULE:
+        klass = rb_singleton_class(obj);
+        break;
+      default:
+        klass = rb_obj_class(obj);
+        break;
     }
 
+    bool allow_new_shape = RCLASS_VARIATION_COUNT(klass) < SHAPE_MAX_VARIATIONS;
     bool variation_created = false;
     rb_shape_t *new_shape = get_next_shape_internal(shape, id, SHAPE_IVAR, &variation_created, allow_new_shape);
 
     // Check if we should update max_iv_count on the object's class
-    if (BUILTIN_TYPE(obj) == T_OBJECT) {
-        VALUE klass = rb_obj_class(obj);
-        if (new_shape->next_field_index > RCLASS_MAX_IV_COUNT(klass)) {
-            RCLASS_SET_MAX_IV_COUNT(klass, new_shape->next_field_index);
-        }
-
-        if (variation_created) {
-            RCLASS_VARIATION_COUNT(klass)++;
-            if (emit_warnings && rb_warning_category_enabled_p(RB_WARN_CATEGORY_PERFORMANCE)) {
-                if (RCLASS_VARIATION_COUNT(klass) >= SHAPE_MAX_VARIATIONS) {
-                    rb_category_warn(
-                        RB_WARN_CATEGORY_PERFORMANCE,
-                        "The class %"PRIsVALUE" reached %d shape variations, instance variables accesses will be slower and memory usage increased.\n"
-                        "It is recommended to define instance variables in a consistent order, for instance by eagerly defining them all in the #initialize method.",
-                        rb_class_path(klass),
-                        SHAPE_MAX_VARIATIONS
-                    );
-                }
+    if (obj != klass && new_shape->next_field_index > RCLASS_MAX_IV_COUNT(klass)) {
+        RCLASS_SET_MAX_IV_COUNT(klass, new_shape->next_field_index);
+    }
+
+    if (variation_created) {
+        RCLASS_VARIATION_COUNT(klass)++;
+
+        if (emit_warnings && rb_warning_category_enabled_p(RB_WARN_CATEGORY_PERFORMANCE)) {
+            if (RCLASS_VARIATION_COUNT(klass) >= SHAPE_MAX_VARIATIONS) {
+                rb_category_warn(
+                    RB_WARN_CATEGORY_PERFORMANCE,
+                    "The class %"PRIsVALUE" reached %d shape variations, instance variables accesses will be slower and memory usage increased.\n"
+                    "It is recommended to define instance variables in a consistent order, for instance by eagerly defining them all in the #initialize method.",
+                    rb_class_path(klass),
+                    SHAPE_MAX_VARIATIONS
+                );
             }
         }
     }

test/ruby/test_object_id.rb

@@ -137,10 +137,14 @@ def setup
       assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
     end
     8.times do |i|
-      TooComplex.new.instance_variable_set("@a#{i}", 1)
+      TooComplex.new.instance_variable_set("@TestObjectIdTooComplex#{i}", 1)
     end
     @obj = TooComplex.new
-    @obj.instance_variable_set(:@test, 1)
+    @obj.instance_variable_set("@a#{rand(10_000)}", 1)
+
+    if defined?(RubyVM::Shape)
+      assert_predicate(RubyVM::Shape.of(@obj), :too_complex?)
+    end
   end
 end
 
@@ -152,11 +156,21 @@ def setup
     if defined?(RubyVM::Shape::SHAPE_MAX_VARIATIONS)
       assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
     end
+
+    @obj = TooComplex.new
+
+    @obj.instance_variable_set("@___#{rand(100_000)}", 1)
+
     8.times do |i|
-      TooComplex.new.instance_variable_set("@a#{i}", 1)
+      @obj.instance_variable_set("@TestObjectIdTooComplexClass#{i}", 1)
+      @obj.remove_instance_variable("@TestObjectIdTooComplexClass#{i}")
+    end
+
+    @obj.instance_variable_set("@___#{rand(100_000)}", 1)
+
+    if defined?(RubyVM::Shape)
+      assert_predicate(RubyVM::Shape.of(@obj), :too_complex?)
     end
-    @obj = TooComplex.new
-    @obj.instance_variable_set(:@test, 1)
   end
 end
 
@@ -169,9 +183,14 @@ def setup
       assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
     end
     8.times do |i|
-      TooComplex.new.instance_variable_set("@a#{i}", 1)
+      TooComplex.new.instance_variable_set("@TestObjectIdTooComplexGeneric#{i}", 1)
     end
     @obj = TooComplex.new
-    @obj.instance_variable_set(:@test, 1)
+    @obj.instance_variable_set("@a#{rand(10_000)}", 1)
+    @obj.instance_variable_set("@a#{rand(10_000)}", 1)
+
+    if defined?(RubyVM::Shape)
+      assert_predicate(RubyVM::Shape.of(@obj), :too_complex?)
+    end
   end
 end