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

Limit maximum number of IVs on a shape on T_OBJECTS

Browse Source 
Create SHAPE_MAX_NUM_IVS (currently 50) and limit all shapes of
T_OBJECTS to that number of IVs. When a shape with a T_OBJECT has more than 50 IVs, fall back to the
obj_too_complex shape which uses hash lookup for ivs.

Note that a previous version of this commit
78fcc9847a9db6d42c8c263154ec05903a370b6b was reverted in
88f2b94065be3fcd6769a3f132cfee8ecfb663b8 because it did not account for
non-T_OBJECTS
This commit is contained in:
Jemma Issroff 2023-01-26 11:42:17 -05:00 committed by Aaron Patterson
parent cab8c11611
commit 28da990984
Notes: git 2023-02-06 16:41:18 +00:00 
Merged: https://github.com/ruby/ruby/pull/7188
3 changed files with 33 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
shape.c
View File

@ -114,7 +114,7 @@ rb_shape_get_shape(VALUE obj)
}
static rb_shape_t*
get_next_shape_internal(rb_shape_t * shape, ID id, enum shape_type shape_type, bool * variation_created, bool new_shapes_allowed)
get_next_shape_internal(rb_shape_t * shape, ID id, enum shape_type shape_type, bool * variation_created, bool new_shapes_allowed, bool new_shape_necessary)
{
rb_shape_t *res = NULL;
@ -123,7 +123,7 @@ get_next_shape_internal(rb_shape_t * shape, ID id, enum shape_type shape_type, b
*variation_created = false;
if (new_shapes_allowed) {
if (new_shape_necessary || (new_shapes_allowed && (shape->next_iv_index < SHAPE_MAX_NUM_IVS))) {
RB_VM_LOCK_ENTER();
{
bool had_edges = !!shape->edges;
@ -238,7 +238,7 @@ remove_shape_recursive(VALUE obj, ID id, rb_shape_t * shape, VALUE * removed)
// has the same attributes as this shape.
if (new_parent) {
bool dont_care;
rb_shape_t * new_child = get_next_shape_internal(new_parent, shape->edge_name, shape->type, &dont_care, true);
rb_shape_t * new_child = get_next_shape_internal(new_parent, shape->edge_name, shape->type, &dont_care, true, false);
new_child->capacity = shape->capacity;
if (new_child->type == SHAPE_IVAR) {
move_iv(obj, id, shape->next_iv_index - 1, new_child->next_iv_index - 1);
@ -283,7 +283,7 @@ rb_shape_transition_shape_frozen(VALUE obj)
}
bool dont_care;
next_shape = get_next_shape_internal(shape, (ID)id_frozen, SHAPE_FROZEN, &dont_care, true);
next_shape = get_next_shape_internal(shape, (ID)id_frozen, SHAPE_FROZEN, &dont_care, true, false);
RUBY_ASSERT(next_shape);
rb_shape_set_shape(obj, next_shape);
@ -298,7 +298,7 @@ rb_shape_get_next_iv_shape(rb_shape_t* shape, ID id)
{
RUBY_ASSERT(!is_instance_id(id) || RTEST(rb_sym2str(ID2SYM(id))));
bool dont_care;
return get_next_shape_internal(shape, id, SHAPE_IVAR, &dont_care, true);
return get_next_shape_internal(shape, id, SHAPE_IVAR, &dont_care, true, false);
}
rb_shape_t *
@ -314,7 +314,9 @@ rb_shape_get_next(rb_shape_t* shape, VALUE obj, ID id)
}
bool variation_created = false;
rb_shape_t * new_shape = get_next_shape_internal(shape, id, SHAPE_IVAR, &variation_created, allow_new_shape);
// For non T_OBJECTS, force a new shape
bool new_shape_necessary = BUILTIN_TYPE(obj) != T_OBJECT;
rb_shape_t * new_shape = get_next_shape_internal(shape, id, SHAPE_IVAR, &variation_created, allow_new_shape, new_shape_necessary);
if (!new_shape) {
RUBY_ASSERT(BUILTIN_TYPE(obj) == T_OBJECT);
@ -341,7 +343,7 @@ rb_shape_transition_shape_capa(rb_shape_t* shape, uint32_t new_capacity)
{
ID edge_name = rb_make_temporary_id(new_capacity);
bool dont_care;
rb_shape_t * new_shape = get_next_shape_internal(shape, edge_name, SHAPE_CAPACITY_CHANGE, &dont_care, true);
rb_shape_t * new_shape = get_next_shape_internal(shape, edge_name, SHAPE_CAPACITY_CHANGE, &dont_care, true, false);
new_shape->capacity = new_capacity;
return new_shape;
}
@ -751,7 +753,7 @@ Init_default_shapes(void)
rb_shape_t * shape = rb_shape_get_shape_by_id(i);
bool dont_care;
rb_shape_t * t_object_shape =
get_next_shape_internal(shape, id_t_object, SHAPE_T_OBJECT, &dont_care, true);
get_next_shape_internal(shape, id_t_object, SHAPE_T_OBJECT, &dont_care, true, false);
t_object_shape->edges = rb_id_table_create(0);
RUBY_ASSERT(rb_shape_id(t_object_shape) == (shape_id_t)(i + SIZE_POOL_COUNT));
}
@ -761,7 +763,7 @@ Init_default_shapes(void)
#if RUBY_DEBUG
rb_shape_t * special_const_shape =
#endif
get_next_shape_internal(root, (ID)id_frozen, SHAPE_FROZEN, &dont_care, true);
get_next_shape_internal(root, (ID)id_frozen, SHAPE_FROZEN, &dont_care, true, false);
RUBY_ASSERT(rb_shape_id(special_const_shape) == SPECIAL_CONST_SHAPE_ID);
RUBY_ASSERT(SPECIAL_CONST_SHAPE_ID == (GET_VM()->next_shape_id - 1));
RUBY_ASSERT(rb_shape_frozen_shape_p(special_const_shape));
@ -800,6 +802,7 @@ Init_shape(void)
rb_define_const(rb_cShape, "SPECIAL_CONST_SHAPE_ID", INT2NUM(SPECIAL_CONST_SHAPE_ID));
rb_define_const(rb_cShape, "OBJ_TOO_COMPLEX_SHAPE_ID", INT2NUM(OBJ_TOO_COMPLEX_SHAPE_ID));
rb_define_const(rb_cShape, "SHAPE_MAX_VARIATIONS", INT2NUM(SHAPE_MAX_VARIATIONS));
rb_define_const(rb_cShape, "SHAPE_MAX_NUM_IVS", INT2NUM(SHAPE_MAX_NUM_IVS));
rb_define_singleton_method(rb_cShape, "transition_tree", shape_transition_tree, 0);
rb_define_singleton_method(rb_cShape, "find_by_id", rb_shape_find_by_id, 1);

1
shape.h
View File

@ -31,6 +31,7 @@ typedef uint16_t shape_id_t;
# define SHAPE_BITMAP_SIZE 16384
# define SHAPE_MAX_VARIATIONS 8
# define SHAPE_MAX_NUM_IVS 50
# define MAX_SHAPE_ID (SHAPE_MASK - 1)
# define INVALID_SHAPE_ID SHAPE_MASK

20
test/ruby/test_shapes.rb
View File

@ -88,6 +88,26 @@ class TestShapes < Test::Unit::TestCase
assert_predicate RubyVM::Shape.of(tc), :too_complex?
end
def test_too_many_ivs_on_obj
obj = Object.new
(RubyVM::Shape::SHAPE_MAX_NUM_IVS + 1).times do
obj.instance_variable_set(:"@a#{_1}", 1)
end
assert_predicate RubyVM::Shape.of(obj), :too_complex?
end
def test_too_many_ivs_on_class
obj = Class.new
(RubyVM::Shape::SHAPE_MAX_NUM_IVS + 1).times do
obj.instance_variable_set(:"@a#{_1}", 1)
end
assert_false RubyVM::Shape.of(obj).too_complex?
end
def test_too_complex_ractor
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
begin;