summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAaron Patterson <[email protected]>2022-12-05 16:48:47 -0800
committerAaron Patterson <[email protected]>2022-12-07 09:57:11 -0800
commitedc7af48acd12666a2945f30901d16b62a39f474 ()
tree7497e93afe9d84f970228cb515a5dc9092db1fbb
parentf725bf358a38b2d5dccb016a962f560baaee55c2 (diff)
Stop transitioning to UNDEF when undefining an instance variable
Cases like this: ```ruby obj = Object.new loop do obj.instance_variable_set(:@foo, 1) obj.remove_instance_variable(:@foo) end ``` can cause us to use many more shapes than we want (and even run out). This commit changes the code such that when an instance variable is removed, we'll walk up the shape tree, find the shape, then rebuild any child nodes that happened to be below the "targetted for removal" IV. This also requires moving any instance variables so that indexes derived from the shape tree will work correctly. Co-Authored-By: Jemma Issroff <[email protected]> Co-authored-by: John Hawthorn <[email protected]>
Notes: Merged: https://.com/ruby/ruby/pull/6866
-rw-r--r--common.mk1
-rw-r--r--marshal.c22
-rw-r--r--mjit_c.rb4
-rw-r--r--shape.c104
-rw-r--r--shape.h4
-rw-r--r--spec/bundler/bundler/fetcher/compact_index_spec.rb2
-rw-r--r--test/ruby/test_shapes.rb34
-rwxr-xr-xtool/mjit/bindgen.rb1
-rw-r--r--variable.c66
-rw-r--r--vm_insnhelper.c2
10 files changed, 152 insertions, 88 deletions
@@ -14219,6 +14219,7 @@ shape.$(OBJEXT): {$(VPATH)}st.h
shape.$(OBJEXT): {$(VPATH)}subst.h
shape.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h
shape.$(OBJEXT): {$(VPATH)}thread_native.h
shape.$(OBJEXT): {$(VPATH)}vm_core.h
shape.$(OBJEXT): {$(VPATH)}vm_debug.h
shape.$(OBJEXT): {$(VPATH)}vm_opts.h
@@ -721,13 +721,23 @@ w_ivar_each(VALUE obj, st_index_t num, struct dump_call_arg *arg)
struct w_ivar_arg ivarg = {arg, num};
if (!num) return;
rb_ivar_foreach(obj, w_obj_each, (st_data_t)&ivarg);
- if (ivarg.num_ivar) {
- rb_raise(rb_eRuntimeError, "instance variable removed from %"PRIsVALUE" instance",
- CLASS_OF(arg->obj));
- }
if (shape_id != rb_shape_get_shape_id(arg->obj)) {
- rb_raise(rb_eRuntimeError, "instance variable added to %"PRIsVALUE" instance",
- CLASS_OF(arg->obj));
}
}
@@ -194,10 +194,6 @@ module RubyVM::MJIT
Primitive.cexpr! %q{ UINT2NUM(SHAPE_IVAR) }
end
- def C.SHAPE_IVAR_UNDEF
- Primitive.cexpr! %q{ UINT2NUM(SHAPE_IVAR_UNDEF) }
- end
-
def C.SHAPE_ROOT
Primitive.cexpr! %q{ UINT2NUM(SHAPE_ROOT) }
end
@@ -5,6 +5,7 @@
#include "internal/class.h"
#include "internal/symbol.h"
#include "internal/variable.h"
#include <stdbool.h>
#ifndef SHAPE_DEBUG
@@ -96,6 +97,19 @@ rb_shape_get_shape_id(VALUE obj)
#endif
}
rb_shape_t*
rb_shape_get_shape(VALUE obj)
{
@@ -131,7 +145,6 @@ get_next_shape_internal(rb_shape_t * shape, ID id, enum shape_type shape_type)
new_shape->next_iv_index = shape->next_iv_index + 1;
break;
case SHAPE_CAPACITY_CHANGE:
- case SHAPE_IVAR_UNDEF:
case SHAPE_FROZEN:
case SHAPE_T_OBJECT:
new_shape->next_iv_index = shape->next_iv_index;
@@ -157,12 +170,88 @@ rb_shape_frozen_shape_p(rb_shape_t* shape)
return SHAPE_FROZEN == (enum shape_type)shape->type;
}
-void
-rb_shape_transition_shape_remove_ivar(VALUE obj, ID id, rb_shape_t *shape)
{
- rb_shape_t * next_shape = get_next_shape_internal(shape, id, SHAPE_IVAR_UNDEF);
- rb_shape_set_shape(obj, next_shape);
}
void
@@ -238,7 +327,6 @@ rb_shape_get_iv_index(rb_shape_t * shape, ID id, attr_index_t *value)
*value = shape->next_iv_index - 1;
return true;
case SHAPE_CAPACITY_CHANGE:
- case SHAPE_IVAR_UNDEF:
case SHAPE_ROOT:
case SHAPE_INITIAL_CAPACITY:
case SHAPE_T_OBJECT:
@@ -329,9 +417,6 @@ rb_shape_rebuild_shape(rb_shape_t * initial_shape, rb_shape_t * dest_shape)
midway_shape = rb_shape_get_next_iv_shape(midway_shape, dest_shape->edge_name);
break;
- case SHAPE_IVAR_UNDEF:
- midway_shape = get_next_shape_internal(midway_shape, dest_shape->edge_name, SHAPE_IVAR_UNDEF);
- break;
case SHAPE_ROOT:
case SHAPE_FROZEN:
case SHAPE_CAPACITY_CHANGE:
@@ -638,7 +723,6 @@ Init_shape(void)
rb_define_const(rb_cShape, "SHAPE_ROOT", INT2NUM(SHAPE_ROOT));
rb_define_const(rb_cShape, "SHAPE_IVAR", INT2NUM(SHAPE_IVAR));
rb_define_const(rb_cShape, "SHAPE_T_OBJECT", INT2NUM(SHAPE_T_OBJECT));
- rb_define_const(rb_cShape, "SHAPE_IVAR_UNDEF", INT2NUM(SHAPE_IVAR_UNDEF));
rb_define_const(rb_cShape, "SHAPE_FROZEN", INT2NUM(SHAPE_FROZEN));
rb_define_const(rb_cShape, "SHAPE_ID_NUM_BITS", INT2NUM(SHAPE_ID_NUM_BITS));
rb_define_const(rb_cShape, "SHAPE_FLAG_SHIFT", INT2NUM(SHAPE_FLAG_SHIFT));
@@ -51,7 +51,6 @@ enum shape_type {
SHAPE_IVAR,
SHAPE_FROZEN,
SHAPE_CAPACITY_CHANGE,
- SHAPE_IVAR_UNDEF,
SHAPE_INITIAL_CAPACITY,
SHAPE_T_OBJECT,
};
@@ -125,6 +124,7 @@ bool rb_shape_root_shape_p(rb_shape_t* shape);
rb_shape_t * rb_shape_get_root_shape(void);
uint8_t rb_shape_id_num_bits(void);
int32_t rb_shape_id_offset(void);
rb_shape_t* rb_shape_get_shape_by_id_without_assertion(shape_id_t shape_id);
rb_shape_t * rb_shape_get_parent(rb_shape_t * shape);
@@ -136,7 +136,7 @@ shape_id_t rb_shape_get_shape_id(VALUE obj);
rb_shape_t* rb_shape_get_shape(VALUE obj);
int rb_shape_frozen_shape_p(rb_shape_t* shape);
void rb_shape_transition_shape_frozen(VALUE obj);
-void rb_shape_transition_shape_remove_ivar(VALUE obj, ID id, rb_shape_t *shape);
rb_shape_t * rb_shape_transition_shape_capa(rb_shape_t * shape, uint32_t new_capacity);
rb_shape_t * rb_shape_get_next_iv_shape(rb_shape_t * shape, ID id);
rb_shape_t* rb_shape_get_next(rb_shape_t* shape, VALUE obj, ID id);
@@ -52,7 +52,9 @@ RSpec.describe Bundler::Fetcher::CompactIndex do
context "when OpenSSL is FIPS-enabled" do
def remove_cached_md5_availability
return unless Bundler::SharedHelpers.instance_variable_defined?(:@md5_available)
Bundler::SharedHelpers.remove_instance_variable(:@md5_available)
end
@@ -28,7 +28,7 @@ class TestShapes < Test::Unit::TestCase
@foo = 1
end
- def remove
remove_instance_variable(:@foo)
end
@@ -64,26 +64,36 @@ class TestShapes < Test::Unit::TestCase
def test_iv_index
example = RemoveAndAdd.new
- shape = RubyVM::Shape.of(example)
- assert_equal 0, shape.next_iv_index
example.add_foo # makes a transition
- new_shape = RubyVM::Shape.of(example)
assert_equal([:@foo], example.instance_variables)
- assert_equal(shape.id, new_shape.parent.id)
- assert_equal(1, new_shape.next_iv_index)
- example.remove # makes a transition
- remove_shape = RubyVM::Shape.of(example)
assert_equal([], example.instance_variables)
- assert_equal(new_shape.id, remove_shape.parent.id)
- assert_equal(1, remove_shape.next_iv_index)
example.add_bar # makes a transition
bar_shape = RubyVM::Shape.of(example)
assert_equal([:@bar], example.instance_variables)
- assert_equal(remove_shape.id, bar_shape.parent.id)
- assert_equal(2, bar_shape.next_iv_index)
end
class TestObject; end
@@ -359,7 +359,6 @@ generator = BindingGenerator.new(
SHAPE_FROZEN
SHAPE_INITIAL_CAPACITY
SHAPE_IVAR
- SHAPE_IVAR_UNDEF
SHAPE_ROOT
],
ULONG: %w[
@@ -1219,7 +1219,7 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef)
rb_check_frozen(obj);
VALUE val = undef;
- attr_index_t index;
switch (BUILTIN_TYPE(obj)) {
case T_CLASS:
@@ -1228,36 +1228,18 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef)
RB_VM_LOCK_ENTER();
{
- rb_shape_t * shape = rb_shape_get_shape(obj);
- if (rb_shape_get_iv_index(shape, id, &index)) {
- rb_shape_transition_shape_remove_ivar(obj, id, shape);
- val = RCLASS_IVPTR(obj)[index];
- RCLASS_IVPTR(obj)[index] = Qundef;
- }
}
RB_VM_LOCK_LEAVE();
break;
case T_OBJECT: {
- rb_shape_t * shape = rb_shape_get_shape(obj);
- if (rb_shape_get_iv_index(shape, id, &index)) {
- rb_shape_transition_shape_remove_ivar(obj, id, shape);
- val = ROBJECT_IVPTR(obj)[index];
- ROBJECT_IVPTR(obj)[index] = Qundef;
- }
break;
}
default: {
- rb_shape_t * shape = rb_shape_get_shape(obj);
-
- if (rb_shape_get_iv_index(shape, id, &index)) {
- rb_shape_transition_shape_remove_ivar(obj, id, shape);
- struct gen_ivtbl *ivtbl;
- rb_gen_ivtbl_get(obj, id, &ivtbl);
- val = ivtbl->ivptr[index];
- ivtbl->ivptr[index] = Qundef;
- }
break;
}
@@ -1613,7 +1595,6 @@ iterate_over_shapes_with_callback(rb_shape_t *shape, rb_ivar_foreach_callback_fu
case SHAPE_INITIAL_CAPACITY:
case SHAPE_CAPACITY_CHANGE:
case SHAPE_FROZEN:
- case SHAPE_IVAR_UNDEF:
case SHAPE_T_OBJECT:
iterate_over_shapes_with_callback(rb_shape_get_parent(shape), callback, itr_data);
return;
@@ -1892,58 +1873,39 @@ check_id_type(VALUE obj, VALUE *pname,
VALUE
rb_obj_remove_instance_variable(VALUE obj, VALUE name)
{
- VALUE val = Qnil;
const ID id = id_for_var(obj, name, an, instance);
// Frozen check comes here because it's expected that we raise a
// NameError (from the id_for_var check) before we raise a FrozenError
rb_check_frozen(obj);
- attr_index_t index;
-
if (!id) {
goto not_defined;
}
switch (BUILTIN_TYPE(obj)) {
case T_CLASS:
case T_MODULE:
IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id);
- rb_shape_t * shape = rb_shape_get_shape(obj);
- if (rb_shape_get_iv_index(shape, id, &index)) {
- rb_shape_transition_shape_remove_ivar(obj, id, shape);
- val = RCLASS_IVPTR(obj)[index];
- RCLASS_IVPTR(obj)[index] = Qundef;
- return val;
- }
break;
case T_OBJECT: {
- rb_shape_t * shape = rb_shape_get_shape(obj);
- if (rb_shape_get_iv_index(shape, id, &index)) {
- rb_shape_transition_shape_remove_ivar(obj, id, shape);
- val = ROBJECT_IVPTR(obj)[index];
- ROBJECT_IVPTR(obj)[index] = Qundef;
- return val;
- }
-
break;
}
default: {
- rb_shape_t * shape = rb_shape_get_shape(obj);
-
- if (rb_shape_get_iv_index(shape, id, &index)) {
- rb_shape_transition_shape_remove_ivar(obj, id, shape);
- struct gen_ivtbl *ivtbl;
- rb_gen_ivtbl_get(obj, id, &ivtbl);
- val = ivtbl->ivptr[index];
- ivtbl->ivptr[index] = Qundef;
- return val;
- }
-
break;
}
}
not_defined:
rb_name_err_raise("instance variable %1$s not defined",
obj, name);
@@ -1416,7 +1416,7 @@ vm_setivar(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_index_t i
rb_shape_t *dest_shape = rb_shape_get_shape_by_id(dest_shape_id);
shape_id_t source_shape_id = dest_shape->parent_id;
- RUBY_ASSERT(dest_shape->type == SHAPE_IVAR || dest_shape->type == SHAPE_IVAR_UNDEF);
if (shape_id == source_shape_id && dest_shape->edge_name == id) {
RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-24 04:37:05 +0000