Make get_next_shape_internal idempotent
Since the check for MAX_SHAPE_ID was done before even checking if the transition we're looking for even exists, as soon as the max shape is reached, get_next_shape_internal would always return `TOO_COMPLEX` regardless of whether the transition we're looking for already exist or not. In addition to entirely de-optimize all newly created objects, it also made an assertion fail in `vm_setivar`: ``` vm_setivar:rb_shape_get_next_iv_shape(rb_shape_get_shape_by_id(source_shape_id), id) == dest_shape ```
This commit is contained in:
parent
d654d580f3
commit
4aee6931c3
54
shape.c
54
shape.c
@ -461,7 +461,6 @@ get_next_shape_internal(rb_shape_t * shape, ID id, enum shape_type shape_type, b
|
|||||||
|
|
||||||
*variation_created = false;
|
*variation_created = false;
|
||||||
|
|
||||||
if (GET_SHAPE_TREE()->next_shape_id <= MAX_SHAPE_ID) {
|
|
||||||
RB_VM_LOCK_ENTER();
|
RB_VM_LOCK_ENTER();
|
||||||
{
|
{
|
||||||
// If the current shape has children
|
// If the current shape has children
|
||||||
@ -474,13 +473,6 @@ get_next_shape_internal(rb_shape_t * shape, ID id, enum shape_type shape_type, b
|
|||||||
if (child->edge_name == id) {
|
if (child->edge_name == id) {
|
||||||
res = child;
|
res = child;
|
||||||
}
|
}
|
||||||
else {
|
|
||||||
// Otherwise we're going to have to create a new shape
|
|
||||||
// and insert it as a child node, so create an id
|
|
||||||
// table and insert the existing child
|
|
||||||
shape->edges = rb_id_table_create(2);
|
|
||||||
rb_id_table_insert(shape->edges, child->edge_name, (VALUE)child);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
// If it has more than one child, do a hash lookup to find it.
|
// If it has more than one child, do a hash lookup to find it.
|
||||||
@ -489,36 +481,40 @@ get_next_shape_internal(rb_shape_t * shape, ID id, enum shape_type shape_type, b
|
|||||||
res = (rb_shape_t *)lookup_result;
|
res = (rb_shape_t *)lookup_result;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// If the shape we were looking for above was found,
|
|
||||||
// then `res` will be set to the child. If it isn't set, then
|
|
||||||
// we know we need a new child shape, and that we must insert
|
|
||||||
// it in to the table.
|
|
||||||
if (!res) {
|
|
||||||
if (new_variations_allowed) {
|
|
||||||
*variation_created = true;
|
|
||||||
rb_shape_t * new_shape = rb_shape_alloc_new_child(id, shape, shape_type);
|
|
||||||
rb_id_table_insert(shape->edges, id, (VALUE)new_shape);
|
|
||||||
res = new_shape;
|
|
||||||
}
|
}
|
||||||
else {
|
|
||||||
|
// If we didn't find the shape we're looking for we create it.
|
||||||
|
if (!res) {
|
||||||
|
// If we're not allowed to create a new variation, of if we're out of shapes
|
||||||
|
// we return TOO_COMPLEX_SHAPE.
|
||||||
|
if (!new_variations_allowed || GET_SHAPE_TREE()->next_shape_id > MAX_SHAPE_ID) {
|
||||||
res = rb_shape_get_shape_by_id(OBJ_TOO_COMPLEX_SHAPE_ID);
|
res = rb_shape_get_shape_by_id(OBJ_TOO_COMPLEX_SHAPE_ID);
|
||||||
}
|
}
|
||||||
}
|
else {
|
||||||
|
rb_shape_t * new_shape = rb_shape_alloc_new_child(id, shape, shape_type);
|
||||||
|
|
||||||
|
if (!shape->edges) {
|
||||||
|
// If the shape had no edge yet, we can directly set the new child
|
||||||
|
shape->edges = TAG_SINGLE_CHILD(new_shape);
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
// If the shape didn't have any outgoing edges, then create
|
// If the edge was single child we need to allocate a table.
|
||||||
// the new outgoing edge and tag the pointer.
|
if (SINGLE_CHILD_P(shape->edges)) {
|
||||||
rb_shape_t * new_shape = rb_shape_alloc_new_child(id, shape, shape_type);
|
rb_shape_t * old_child = SINGLE_CHILD(shape->edges);
|
||||||
shape->edges = TAG_SINGLE_CHILD(new_shape);
|
shape->edges = rb_id_table_create(2);
|
||||||
|
rb_id_table_insert(shape->edges, old_child->edge_name, (VALUE)old_child);
|
||||||
|
}
|
||||||
|
|
||||||
|
rb_id_table_insert(shape->edges, new_shape->edge_name, (VALUE)new_shape);
|
||||||
|
*variation_created = true;
|
||||||
|
}
|
||||||
|
|
||||||
res = new_shape;
|
res = new_shape;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
RB_VM_LOCK_LEAVE();
|
RB_VM_LOCK_LEAVE();
|
||||||
}
|
|
||||||
else {
|
|
||||||
res = rb_shape_get_shape_by_id(OBJ_TOO_COMPLEX_SHAPE_ID);
|
|
||||||
}
|
|
||||||
return res;
|
return res;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -207,6 +207,25 @@ class TestShapes < Test::Unit::TestCase
|
|||||||
end;
|
end;
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_run_out_of_shape
|
||||||
|
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
|
||||||
|
begin;
|
||||||
|
class A
|
||||||
|
def initialize
|
||||||
|
@a = 1
|
||||||
|
end
|
||||||
|
end
|
||||||
|
# Try to run out of shapes
|
||||||
|
o = Object.new
|
||||||
|
i = 0
|
||||||
|
while RubyVM::Shape.shapes_available > 0
|
||||||
|
o.instance_variable_set(:"@i#{i}", 1)
|
||||||
|
i += 1
|
||||||
|
A.new
|
||||||
|
end
|
||||||
|
end;
|
||||||
|
end
|
||||||
|
|
||||||
def test_use_all_shapes_module
|
def test_use_all_shapes_module
|
||||||
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
|
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
|
||||||
begin;
|
begin;
|
||||||
|
Loading…
x
Reference in New Issue
Block a user