Skip to content

Commit

Permalink
Rename iv_count on shapes to next_iv_index
Browse files Browse the repository at this point in the history 
`iv_count` is a misleading name because when IVs are unset, the new
shape doesn't decrement this value. `next_iv_count` is an accurate, and
more descriptive name.
  • Loading branch information
@jemmaissroff @tenderlove
jemmaissroff authored and tenderlove committed Oct 21, 2022
1 parent 13bd617 commit a11952d
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 25 deletions.
2 changes: 1 addition & 1 deletion gc.c
View file
Expand Up @@ -3434,7 +3434,7 @@ obj_free(rb_objspace_t *objspace, VALUE obj)
VALUE klass = RBASIC_CLASS(obj);

// Increment max_iv_count if applicable, used to determine size pool allocation
uint32_t num_of_ivs = shape->iv_count;
uint32_t num_of_ivs = shape->next_iv_index;
if (RCLASS_EXT(klass)->max_iv_count < num_of_ivs) {
RCLASS_EXT(klass)->max_iv_count = num_of_ivs;
}
Expand Down
2 changes: 1 addition & 1 deletion mjit_c.rb
View file
Expand Up @@ -606,7 +606,7 @@ def C.rb_shape
"rb_shape", Primitive.cexpr!("SIZEOF(struct rb_shape)"),
edges: [CType::Pointer.new { self.rb_id_table }, Primitive.cexpr!("OFFSETOF((*((struct rb_shape *)NULL)), edges)")],
edge_name: [self.ID, Primitive.cexpr!("OFFSETOF((*((struct rb_shape *)NULL)), edge_name)")],
iv_count: [self.attr_index_t, Primitive.cexpr!("OFFSETOF((*((struct rb_shape *)NULL)), iv_count)")],
next_iv_index: [self.attr_index_t, Primitive.cexpr!("OFFSETOF((*((struct rb_shape *)NULL)), next_iv_index)")],
type: [CType::Immediate.parse("uint8_t"), Primitive.cexpr!("OFFSETOF((*((struct rb_shape *)NULL)), type)")],
parent_id: [self.shape_id_t, Primitive.cexpr!("OFFSETOF((*((struct rb_shape *)NULL)), parent_id)")],
)
Expand Down
22 changes: 11 additions & 11 deletions shape.c
View file
Expand Up @@ -142,19 +142,19 @@ get_next_shape_internal(rb_shape_t* shape, ID id, VALUE obj, enum shape_type sha

switch (shape_type) {
case SHAPE_IVAR:
new_shape->iv_count = rb_shape_get_shape_by_id(new_shape->parent_id)->iv_count + 1;
new_shape->next_iv_index = rb_shape_get_shape_by_id(new_shape->parent_id)->next_iv_index + 1;

// Check if we should update max_iv_count on the object's class
// Check if we should update next_iv_index on the object's class
if (BUILTIN_TYPE(obj) == T_OBJECT) {
VALUE klass = rb_obj_class(obj);
if (new_shape->iv_count > RCLASS_EXT(klass)->max_iv_count) {
RCLASS_EXT(klass)->max_iv_count = new_shape->iv_count;
if (new_shape->next_iv_index > RCLASS_EXT(klass)->max_iv_count) {
RCLASS_EXT(klass)->max_iv_count = new_shape->next_iv_index;
}
}
break;
case SHAPE_IVAR_UNDEF:
case SHAPE_FROZEN:
new_shape->iv_count = rb_shape_get_shape_by_id(new_shape->parent_id)->iv_count;
new_shape->next_iv_index = rb_shape_get_shape_by_id(new_shape->parent_id)->next_iv_index;
break;
case SHAPE_ROOT:
rb_bug("Unreachable");
Expand Down Expand Up @@ -244,8 +244,8 @@ rb_shape_get_iv_index(rb_shape_t * shape, ID id, attr_index_t *value)

switch (shape_type) {
case SHAPE_IVAR:
RUBY_ASSERT(shape->iv_count > 0);
*value = shape->iv_count - 1;
RUBY_ASSERT(shape->next_iv_index > 0);
*value = shape->next_iv_index - 1;
return true;
case SHAPE_IVAR_UNDEF:
case SHAPE_ROOT:
Expand Down Expand Up @@ -280,7 +280,7 @@ rb_shape_alloc_with_parent_id(ID edge_name, shape_id_t parent_id)
rb_shape_t * shape = shape_alloc();

shape->edge_name = edge_name;
shape->iv_count = 0;
shape->next_iv_index = 0;
shape->parent_id = parent_id;

return shape;
Expand Down Expand Up @@ -404,12 +404,12 @@ rb_shape_edge_name(VALUE self)
}

static VALUE
rb_shape_iv_count(VALUE self)
rb_shape_next_iv_index(VALUE self)
{
rb_shape_t* shape;
TypedData_Get_Struct(self, rb_shape_t, &shape_data_type, shape);

return INT2NUM(shape->iv_count);
return INT2NUM(shape->next_iv_index);
}

static VALUE
Expand Down Expand Up @@ -526,7 +526,7 @@ Init_shape(void)
rb_define_method(rb_cShape, "parent", rb_shape_parent, 0);
rb_define_method(rb_cShape, "edges", rb_shape_edges, 0);
rb_define_method(rb_cShape, "edge_name", rb_shape_edge_name, 0);
rb_define_method(rb_cShape, "iv_count", rb_shape_iv_count, 0);
rb_define_method(rb_cShape, "next_iv_index", rb_shape_next_iv_index, 0);
rb_define_method(rb_cShape, "depth", rb_shape_export_depth, 0);
rb_define_method(rb_cShape, "id", rb_wrapped_shape_id, 0);
rb_define_method(rb_cShape, "type", rb_shape_type, 0);
Expand Down
4 changes: 2 additions & 2 deletions shape.h
View file
Expand Up @@ -45,7 +45,7 @@ typedef uint16_t shape_id_t;
struct rb_shape {
struct rb_id_table * edges; // id_table from ID (ivar) to next shape
ID edge_name; // ID (ivar) for transition from parent to rb_shape
attr_index_t iv_count;
attr_index_t next_iv_index;
uint8_t type;
shape_id_t parent_id;
};
Expand Down Expand Up @@ -129,7 +129,7 @@ static inline uint32_t
ROBJECT_IV_COUNT(VALUE obj)
{
RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT);
uint32_t ivc = rb_shape_get_shape_by_id(ROBJECT_SHAPE_ID(obj))->iv_count;
uint32_t ivc = rb_shape_get_shape_by_id(ROBJECT_SHAPE_ID(obj))->next_iv_index;
RUBY_ASSERT(ivc <= ROBJECT_NUMIV(obj));
return ivc;
}
Expand Down
8 changes: 4 additions & 4 deletions test/ruby/test_shapes.rb
View file
Expand Up @@ -65,25 +65,25 @@ def test_shape_order
def test_iv_index
example = RemoveAndAdd.new
shape = RubyVM::Shape.of(example)
assert_equal 0, shape.iv_count
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.iv_count)
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.iv_count)
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.iv_count)
assert_equal(2, bar_shape.next_iv_index)
end

def test_new_obj_has_root_shape
Expand Down
10 changes: 5 additions & 5 deletions variable.c
View file
Expand Up @@ -1018,7 +1018,7 @@ generic_ivar_update(st_data_t *k, st_data_t *v, st_data_t u, int existing)
}
}
FL_SET((VALUE)*k, FL_EXIVAR);
ivtbl = gen_ivtbl_resize(ivtbl, ivup->shape->iv_count);
ivtbl = gen_ivtbl_resize(ivtbl, ivup->shape->next_iv_index);
// Reinsert in to the hash table because ivtbl might be a newly resized chunk of memory
*v = (st_data_t)ivtbl;
ivup->ivtbl = ivtbl;
Expand Down Expand Up @@ -1435,7 +1435,7 @@ rb_ensure_generic_iv_list_size(VALUE obj, uint32_t newsize)
void
rb_init_iv_list(VALUE obj)
{
uint32_t newsize = (uint32_t)(rb_shape_get_shape(obj)->iv_count * 2.0);
uint32_t newsize = (uint32_t)(rb_shape_get_shape(obj)->next_iv_index * 2.0);
uint32_t len = ROBJECT_NUMIV(obj);
rb_ensure_iv_list_size(obj, len, newsize < len ? len : newsize);
}
Expand All @@ -1450,7 +1450,7 @@ obj_ivar_set(VALUE obj, ID id, VALUE val)

if (!rb_shape_get_iv_index(shape, id, &index)) {
shape = rb_shape_get_next(shape, obj, id);
index = shape->iv_count - 1;
index = shape->next_iv_index - 1;
}

uint32_t len = ROBJECT_NUMIV(obj);
Expand Down Expand Up @@ -1615,7 +1615,7 @@ iterate_over_shapes_with_callback(rb_shape_t *shape, VALUE* iv_list, rb_ivar_for
return;
case SHAPE_IVAR:
iterate_over_shapes_with_callback(rb_shape_get_shape_by_id(shape->parent_id), iv_list, callback, arg);
VALUE val = iv_list[shape->iv_count - 1];
VALUE val = iv_list[shape->next_iv_index - 1];
if (val != Qundef) {
callback(shape->edge_name, val, arg);
}
Expand Down Expand Up @@ -1753,7 +1753,7 @@ rb_ivar_count(VALUE obj)

switch (BUILTIN_TYPE(obj)) {
case T_OBJECT:
if (rb_shape_get_shape(obj)->iv_count > 0) {
if (rb_shape_get_shape(obj)->next_iv_index > 0) {
st_index_t i, count, num = ROBJECT_IV_COUNT(obj);
const VALUE *const ivptr = ROBJECT_IVPTR(obj);
for (i = count = 0; i < num; ++i) {
Expand Down
2 changes: 1 addition & 1 deletion yjit/src/cruby_bindings.inc.rs
View file
Expand Up @@ -419,7 +419,7 @@ pub type shape_id_t = u32;
pub struct rb_shape {
pub edges: *mut rb_id_table,
pub edge_name: ID,
pub iv_count: attr_index_t,
pub next_iv_index: attr_index_t,
pub type_: u8,
pub parent_id: shape_id_t,
}
Expand Down

0 comments on commit a11952d

Please sign in to comment.