diff --git a/gc.c b/gc.c index 11efdfa544..fb1cc39f3d 100644 --- a/gc.c +++ b/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); } diff --git a/shape.c b/shape.c index fc83988e9b..6e1cb94567 100644 --- a/shape.c +++ b/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 (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 - ); - } + 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 + ); } } } diff --git a/test/ruby/test_object_id.rb b/test/ruby/test_object_id.rb index 3543802990..2277bba634 100644 --- a/test/ruby/test_object_id.rb +++ b/test/ruby/test_object_id.rb @@ -137,10 +137,14 @@ class TestObjectIdTooComplex < TestObjectId 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 @@ class TestObjectIdTooComplexClass < TestObjectId if defined?(RubyVM::Shape::SHAPE_MAX_VARIATIONS) assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS end - 8.times do |i| - TooComplex.new.instance_variable_set("@a#{i}", 1) - end + @obj = TooComplex.new - @obj.instance_variable_set(:@test, 1) + + @obj.instance_variable_set("@___#{rand(100_000)}", 1) + + 8.times do |i| + @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 end end @@ -169,9 +183,14 @@ class TestObjectIdTooComplexGeneric < TestObjectId 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