Refactor raw accesses to rb_shape_t.capacity

Author: byroot

internal/variable.h

@@ -70,7 +70,7 @@ VALUE rb_gvar_get(ID);
 VALUE rb_gvar_set(ID, VALUE);
 VALUE rb_gvar_defined(ID);
 void rb_const_warn_if_deprecated(const rb_const_entry_t *, VALUE, ID);
-void rb_ensure_iv_list_size(VALUE obj, uint32_t len, uint32_t newsize);
+void rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t newsize);
 attr_index_t rb_obj_ivar_set(VALUE obj, ID id, VALUE val);
 
 #endif /* INTERNAL_VARIABLE_H */

object.c

@@ -355,9 +355,12 @@ rb_obj_copy_ivar(VALUE dest, VALUE obj)
     VALUE *src_buf = ROBJECT_FIELDS(obj);
     VALUE *dest_buf = ROBJECT_FIELDS(dest);
 
-    RUBY_ASSERT(src_num_ivs <= RSHAPE(dest_shape_id)->capacity);
-    if (RSHAPE(initial_shape_id)->capacity < RSHAPE(dest_shape_id)->capacity) {
-        rb_ensure_iv_list_size(dest, RSHAPE(initial_shape_id)->capacity, RSHAPE(dest_shape_id)->capacity);
+    attr_index_t initial_capa = RSHAPE_CAPACITY(initial_shape_id);
+    attr_index_t dest_capa = RSHAPE_CAPACITY(dest_shape_id);
+
+    RUBY_ASSERT(src_num_ivs <= dest_capa);
+    if (initial_capa < dest_capa) {
+        rb_ensure_iv_list_size(dest, 0, dest_capa);
         dest_buf = ROBJECT_FIELDS(dest);
     }
 

shape.c

@@ -321,7 +321,7 @@ static void
 shape_tree_compact(void *data)
 {
     rb_shape_t *cursor = rb_shape_get_root_shape();
-    rb_shape_t *end = RSHAPE(GET_SHAPE_TREE()->next_shape_id);
+    rb_shape_t *end = RSHAPE(GET_SHAPE_TREE()->next_shape_id - 1);
     while (cursor < end) {
         if (cursor->edges && !SINGLE_CHILD_P(cursor->edges)) {
             cursor->edges = rb_gc_location(cursor->edges);
@@ -1107,6 +1107,8 @@ shape_rebuild(rb_shape_t *initial_shape, rb_shape_t *dest_shape)
     return midway_shape;
 }
 
+// Rebuild `dest_shape_id` starting from `initial_shape_id`, and keep only SHAPE_IVAR transitions.
+// SHAPE_OBJ_ID and frozen status are lost.
 shape_id_t
 rb_shape_rebuild(shape_id_t initial_shape_id, shape_id_t dest_shape_id)
 {
@@ -1135,6 +1137,9 @@ rb_shape_copy_fields(VALUE dest, VALUE *dest_buf, shape_id_t dest_shape_id, VALU
         while (src_shape->parent_id != INVALID_SHAPE_ID) {
             if (src_shape->type == SHAPE_IVAR) {
                 while (dest_shape->edge_name != src_shape->edge_name) {
+                    if (UNLIKELY(dest_shape->parent_id == INVALID_SHAPE_ID)) {
+                        rb_bug("Lost field %s", rb_id2name(src_shape->edge_name));
+                    }
                     dest_shape = RSHAPE(dest_shape->parent_id);
                 }
 

shape.h

@@ -232,7 +232,7 @@ ROBJECT_FIELDS_CAPACITY(VALUE obj)
     // Asking for capacity doesn't make sense when the object is using
     // a hash table for storing instance variables
     RUBY_ASSERT(!rb_shape_obj_too_complex_p(obj));
-    return RSHAPE(RBASIC_SHAPE_ID(obj))->capacity;
+    return RSHAPE_CAPACITY(RBASIC_SHAPE_ID(obj));
 }
 
 static inline st_table *

test/ruby/test_shapes.rb

@@ -92,15 +92,18 @@ def write_iv
   # RubyVM::Shape.of returns new instances of shape objects for
   # each call. This helper method allows us to define equality for
   # shapes
-  def assert_shape_equal(shape1, shape2)
-    assert_equal(shape1.id, shape2.id)
-    assert_equal(shape1.parent_id, shape2.parent_id)
-    assert_equal(shape1.depth, shape2.depth)
-    assert_equal(shape1.type, shape2.type)
-  end
-
-  def refute_shape_equal(shape1, shape2)
-    refute_equal(shape1.id, shape2.id)
+  def assert_shape_equal(e, a)
+    assert_equal(
+      {id: e.id, parent_id: e.parent_id, depth: e.depth, type: e.type},
+      {id: a.id, parent_id: a.parent_id, depth: a.depth, type: a.type},
+    )
+  end
+
+  def refute_shape_equal(e, a)
+    refute_equal(
+      {id: e.id, parent_id: e.parent_id, depth: e.depth, type: e.type},
+      {id: a.id, parent_id: a.parent_id, depth: a.depth, type: a.type},
+    )
   end
 
   def test_iv_order_correct_on_complex_objects

variable.c

@@ -1825,13 +1825,13 @@ generic_fields_lookup_ensure_size(st_data_t *k, st_data_t *v, st_data_t u, int e
     if (!existing || fields_lookup->resize) {
         if (existing) {
             RUBY_ASSERT(RSHAPE(fields_lookup->shape_id)->type == SHAPE_IVAR || RSHAPE(fields_lookup->shape_id)->type == SHAPE_OBJ_ID);
-            RUBY_ASSERT(RSHAPE(RSHAPE(fields_lookup->shape_id)->parent_id)->capacity < RSHAPE(fields_lookup->shape_id)->capacity);
+            RUBY_ASSERT(RSHAPE_CAPACITY(RSHAPE(fields_lookup->shape_id)->parent_id) < RSHAPE_CAPACITY(fields_lookup->shape_id));
         }
         else {
             FL_SET_RAW((VALUE)*k, FL_EXIVAR);
         }
 
-        fields_tbl = gen_fields_tbl_resize(fields_tbl, RSHAPE(fields_lookup->shape_id)->capacity);
+        fields_tbl = gen_fields_tbl_resize(fields_tbl, RSHAPE_CAPACITY(fields_lookup->shape_id));
         *v = (st_data_t)fields_tbl;
     }
 
@@ -1940,14 +1940,14 @@ generic_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
 }
 
 void
-rb_ensure_iv_list_size(VALUE obj, uint32_t current_capacity, uint32_t new_capacity)
+rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t new_capacity)
 {
     RUBY_ASSERT(!rb_shape_obj_too_complex_p(obj));
 
     if (RBASIC(obj)->flags & ROBJECT_EMBED) {
         VALUE *ptr = ROBJECT_FIELDS(obj);
         VALUE *newptr = ALLOC_N(VALUE, new_capacity);
-        MEMCPY(newptr, ptr, VALUE, current_capacity);
+        MEMCPY(newptr, ptr, VALUE, current_len);
         RB_FL_UNSET_RAW(obj, ROBJECT_EMBED);
         ROBJECT(obj)->as.heap.fields = newptr;
     }
@@ -2370,13 +2370,13 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj)
             }
         }
 
-        if (!RSHAPE(dest_shape_id)->capacity) {
+        if (!RSHAPE_LEN(dest_shape_id)) {
             rb_obj_set_shape_id(dest, dest_shape_id);
             FL_UNSET(dest, FL_EXIVAR);
             return;
         }
 
-        new_fields_tbl = gen_fields_tbl_resize(0, RSHAPE(dest_shape_id)->capacity);
+        new_fields_tbl = gen_fields_tbl_resize(0, RSHAPE_CAPACITY(dest_shape_id));
 
         VALUE *src_buf = obj_fields_tbl->as.shape.fields;
         VALUE *dest_buf = new_fields_tbl->as.shape.fields;

vm_insnhelper.c

@@ -1455,11 +1455,10 @@ vm_setivar_default(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_i
         RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);
     }
     else if (dest_shape_id != INVALID_SHAPE_ID) {
-        rb_shape_t *shape = RSHAPE(shape_id);
         rb_shape_t *dest_shape = RSHAPE(dest_shape_id);
 
-        if (shape_id == dest_shape->parent_id && dest_shape->edge_name == id && shape->capacity == dest_shape->capacity) {
-            RUBY_ASSERT(index < dest_shape->capacity);
+        if (shape_id == dest_shape->parent_id && dest_shape->edge_name == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {
+            RUBY_ASSERT(index < RSHAPE_CAPACITY(dest_shape_id));
         }
         else {
             return Qundef;
@@ -1499,17 +1498,16 @@ vm_setivar(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_index_t i
                 VM_ASSERT(!rb_ractor_shareable_p(obj));
             }
             else if (dest_shape_id != INVALID_SHAPE_ID) {
-                rb_shape_t *shape = RSHAPE(shape_id);
                 rb_shape_t *dest_shape = RSHAPE(dest_shape_id);
                 shape_id_t source_shape_id = dest_shape->parent_id;
 
-                if (shape_id == source_shape_id && dest_shape->edge_name == id && shape->capacity == dest_shape->capacity) {
+                if (shape_id == source_shape_id && dest_shape->edge_name == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {
                     RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);
 
                     RBASIC_SET_SHAPE_ID(obj, dest_shape_id);
 
                     RUBY_ASSERT(rb_shape_get_next_iv_shape(source_shape_id, id) == dest_shape_id);
-                    RUBY_ASSERT(index < dest_shape->capacity);
+                    RUBY_ASSERT(index < RSHAPE_CAPACITY(dest_shape_id));
                 }
                 else {
                     break;