1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Allow T_CLASS and generic types to be too_complex

Browse Source 
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: https://github.com/ruby/ruby/pull/6931
This commit is contained in:
Jean Boussier 2025-05-11 18:47:22 +02:00
parent 54c2edc05d
commit f2e5f6dbb6
Notes: git 2025-05-11 17:36:11 +00:00 
Merged: https://github.com/ruby/ruby/pull/13301
3 changed files with 53 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
gc.c
View File

@ -1872,26 +1872,7 @@ object_id(VALUE obj)
// we'd at least need to generate the object_id using atomics. // we'd at least need to generate the object_id using atomics.
lock_lev = rb_gc_vm_lock(); lock_lev = rb_gc_vm_lock();
if (rb_shape_too_complex_p(shape)) { if (rb_shape_has_object_id(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)) {
rb_shape_t *object_id_shape = rb_shape_object_id_shape(obj); rb_shape_t *object_id_shape = rb_shape_object_id_shape(obj);
id = rb_obj_field_get(obj, object_id_shape); id = rb_obj_field_get(obj, object_id_shape);
} }

47
shape.c
View File

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

35
test/ruby/test_object_id.rb
View File

@ -137,10 +137,14 @@ class TestObjectIdTooComplex < TestObjectId
assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
end end
8.times do |i| 8.times do |i|
TooComplex.new.instance_variable_set("@a#{i}", 1) TooComplex.new.instance_variable_set("@TestObjectIdTooComplex#{i}", 1)
end end
@obj = TooComplex.new @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
end end
@ -152,11 +156,21 @@ class TestObjectIdTooComplexClass < TestObjectId
if defined?(RubyVM::Shape::SHAPE_MAX_VARIATIONS) if defined?(RubyVM::Shape::SHAPE_MAX_VARIATIONS)
assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
end end
8.times do |i|
TooComplex.new.instance_variable_set("@a#{i}", 1)
end
@obj = TooComplex.new @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
end end
@ -169,9 +183,14 @@ class TestObjectIdTooComplexGeneric < TestObjectId
assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
end end
8.times do |i| 8.times do |i|
TooComplex.new.instance_variable_set("@a#{i}", 1) TooComplex.new.instance_variable_set("@TestObjectIdTooComplexGeneric#{i}", 1)
end end
@obj = TooComplex.new @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
end end