Skip to content

Commit 9cbef37

Browse files
committed
Refactor raw accesses to rb_shape_t.capacity
1 parent da2453c commit 9cbef37

File tree

9 files changed

+38
-29
lines changed

9 files changed

+38
-29
lines changed

internal/variable.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ VALUE rb_gvar_get(ID);
7070
VALUE rb_gvar_set(ID, VALUE);
7171
VALUE rb_gvar_defined(ID);
7272
void rb_const_warn_if_deprecated(const rb_const_entry_t *, VALUE, ID);
73-
void rb_ensure_iv_list_size(VALUE obj, uint32_t len, uint32_t newsize);
73+
void rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t newsize);
7474
attr_index_t rb_obj_ivar_set(VALUE obj, ID id, VALUE val);
7575

7676
#endif /* INTERNAL_VARIABLE_H */

object.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,12 @@ rb_obj_copy_ivar(VALUE dest, VALUE obj)
355355
VALUE *src_buf = ROBJECT_FIELDS(obj);
356356
VALUE *dest_buf = ROBJECT_FIELDS(dest);
357357

358-
RUBY_ASSERT(src_num_ivs <= RSHAPE(dest_shape_id)->capacity);
359-
if (RSHAPE(initial_shape_id)->capacity < RSHAPE(dest_shape_id)->capacity) {
360-
rb_ensure_iv_list_size(dest, RSHAPE(initial_shape_id)->capacity, RSHAPE(dest_shape_id)->capacity);
358+
attr_index_t initial_capa = RSHAPE_CAPACITY(initial_shape_id);
359+
attr_index_t dest_capa = RSHAPE_CAPACITY(dest_shape_id);
360+
361+
RUBY_ASSERT(src_num_ivs <= dest_capa);
362+
if (initial_capa < dest_capa) {
363+
rb_ensure_iv_list_size(dest, 0, dest_capa);
361364
dest_buf = ROBJECT_FIELDS(dest);
362365
}
363366

shape.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ static void
321321
shape_tree_compact(void *data)
322322
{
323323
rb_shape_t *cursor = rb_shape_get_root_shape();
324-
rb_shape_t *end = RSHAPE(GET_SHAPE_TREE()->next_shape_id);
324+
rb_shape_t *end = RSHAPE(GET_SHAPE_TREE()->next_shape_id - 1);
325325
while (cursor < end) {
326326
if (cursor->edges && !SINGLE_CHILD_P(cursor->edges)) {
327327
cursor->edges = rb_gc_location(cursor->edges);
@@ -1107,6 +1107,8 @@ shape_rebuild(rb_shape_t *initial_shape, rb_shape_t *dest_shape)
11071107
return midway_shape;
11081108
}
11091109

1110+
// Rebuild `dest_shape_id` starting from `initial_shape_id`, and keep only SHAPE_IVAR transitions.
1111+
// SHAPE_OBJ_ID and frozen status are lost.
11101112
shape_id_t
11111113
rb_shape_rebuild(shape_id_t initial_shape_id, shape_id_t dest_shape_id)
11121114
{
@@ -1135,6 +1137,9 @@ rb_shape_copy_fields(VALUE dest, VALUE *dest_buf, shape_id_t dest_shape_id, VALU
11351137
while (src_shape->parent_id != INVALID_SHAPE_ID) {
11361138
if (src_shape->type == SHAPE_IVAR) {
11371139
while (dest_shape->edge_name != src_shape->edge_name) {
1140+
if (UNLIKELY(dest_shape->parent_id == INVALID_SHAPE_ID)) {
1141+
rb_bug("Lost field %s", rb_id2name(src_shape->edge_name));
1142+
}
11381143
dest_shape = RSHAPE(dest_shape->parent_id);
11391144
}
11401145

shape.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ ROBJECT_FIELDS_CAPACITY(VALUE obj)
232232
// Asking for capacity doesn't make sense when the object is using
233233
// a hash table for storing instance variables
234234
RUBY_ASSERT(!rb_shape_obj_too_complex_p(obj));
235-
return RSHAPE(RBASIC_SHAPE_ID(obj))->capacity;
235+
return RSHAPE_CAPACITY(RBASIC_SHAPE_ID(obj));
236236
}
237237

238238
static inline st_table *

test/ruby/test_shapes.rb

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,15 +92,18 @@ def write_iv
9292
# RubyVM::Shape.of returns new instances of shape objects for
9393
# each call. This helper method allows us to define equality for
9494
# shapes
95-
def assert_shape_equal(shape1, shape2)
96-
assert_equal(shape1.id, shape2.id)
97-
assert_equal(shape1.parent_id, shape2.parent_id)
98-
assert_equal(shape1.depth, shape2.depth)
99-
assert_equal(shape1.type, shape2.type)
100-
end
101-
102-
def refute_shape_equal(shape1, shape2)
103-
refute_equal(shape1.id, shape2.id)
95+
def assert_shape_equal(e, a)
96+
assert_equal(
97+
{id: e.id, parent_id: e.parent_id, depth: e.depth, type: e.type},
98+
{id: a.id, parent_id: a.parent_id, depth: a.depth, type: a.type},
99+
)
100+
end
101+
102+
def refute_shape_equal(e, a)
103+
refute_equal(
104+
{id: e.id, parent_id: e.parent_id, depth: e.depth, type: e.type},
105+
{id: a.id, parent_id: a.parent_id, depth: a.depth, type: a.type},
106+
)
104107
end
105108

106109
def test_iv_order_correct_on_complex_objects

variable.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1825,13 +1825,13 @@ generic_fields_lookup_ensure_size(st_data_t *k, st_data_t *v, st_data_t u, int e
18251825
if (!existing || fields_lookup->resize) {
18261826
if (existing) {
18271827
RUBY_ASSERT(RSHAPE(fields_lookup->shape_id)->type == SHAPE_IVAR || RSHAPE(fields_lookup->shape_id)->type == SHAPE_OBJ_ID);
1828-
RUBY_ASSERT(RSHAPE(RSHAPE(fields_lookup->shape_id)->parent_id)->capacity < RSHAPE(fields_lookup->shape_id)->capacity);
1828+
RUBY_ASSERT(RSHAPE_CAPACITY(RSHAPE(fields_lookup->shape_id)->parent_id) < RSHAPE_CAPACITY(fields_lookup->shape_id));
18291829
}
18301830
else {
18311831
FL_SET_RAW((VALUE)*k, FL_EXIVAR);
18321832
}
18331833

1834-
fields_tbl = gen_fields_tbl_resize(fields_tbl, RSHAPE(fields_lookup->shape_id)->capacity);
1834+
fields_tbl = gen_fields_tbl_resize(fields_tbl, RSHAPE_CAPACITY(fields_lookup->shape_id));
18351835
*v = (st_data_t)fields_tbl;
18361836
}
18371837

@@ -1940,14 +1940,14 @@ generic_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
19401940
}
19411941

19421942
void
1943-
rb_ensure_iv_list_size(VALUE obj, uint32_t current_capacity, uint32_t new_capacity)
1943+
rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t new_capacity)
19441944
{
19451945
RUBY_ASSERT(!rb_shape_obj_too_complex_p(obj));
19461946

19471947
if (RBASIC(obj)->flags & ROBJECT_EMBED) {
19481948
VALUE *ptr = ROBJECT_FIELDS(obj);
19491949
VALUE *newptr = ALLOC_N(VALUE, new_capacity);
1950-
MEMCPY(newptr, ptr, VALUE, current_capacity);
1950+
MEMCPY(newptr, ptr, VALUE, current_len);
19511951
RB_FL_UNSET_RAW(obj, ROBJECT_EMBED);
19521952
ROBJECT(obj)->as.heap.fields = newptr;
19531953
}
@@ -2370,13 +2370,13 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj)
23702370
}
23712371
}
23722372

2373-
if (!RSHAPE(dest_shape_id)->capacity) {
2373+
if (!RSHAPE_LEN(dest_shape_id)) {
23742374
rb_obj_set_shape_id(dest, dest_shape_id);
23752375
FL_UNSET(dest, FL_EXIVAR);
23762376
return;
23772377
}
23782378

2379-
new_fields_tbl = gen_fields_tbl_resize(0, RSHAPE(dest_shape_id)->capacity);
2379+
new_fields_tbl = gen_fields_tbl_resize(0, RSHAPE_CAPACITY(dest_shape_id));
23802380

23812381
VALUE *src_buf = obj_fields_tbl->as.shape.fields;
23822382
VALUE *dest_buf = new_fields_tbl->as.shape.fields;

vm_insnhelper.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1455,11 +1455,10 @@ vm_setivar_default(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_i
14551455
RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);
14561456
}
14571457
else if (dest_shape_id != INVALID_SHAPE_ID) {
1458-
rb_shape_t *shape = RSHAPE(shape_id);
14591458
rb_shape_t *dest_shape = RSHAPE(dest_shape_id);
14601459

1461-
if (shape_id == dest_shape->parent_id && dest_shape->edge_name == id && shape->capacity == dest_shape->capacity) {
1462-
RUBY_ASSERT(index < dest_shape->capacity);
1460+
if (shape_id == dest_shape->parent_id && dest_shape->edge_name == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {
1461+
RUBY_ASSERT(index < RSHAPE_CAPACITY(dest_shape_id));
14631462
}
14641463
else {
14651464
return Qundef;
@@ -1499,17 +1498,16 @@ vm_setivar(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_index_t i
14991498
VM_ASSERT(!rb_ractor_shareable_p(obj));
15001499
}
15011500
else if (dest_shape_id != INVALID_SHAPE_ID) {
1502-
rb_shape_t *shape = RSHAPE(shape_id);
15031501
rb_shape_t *dest_shape = RSHAPE(dest_shape_id);
15041502
shape_id_t source_shape_id = dest_shape->parent_id;
15051503

1506-
if (shape_id == source_shape_id && dest_shape->edge_name == id && shape->capacity == dest_shape->capacity) {
1504+
if (shape_id == source_shape_id && dest_shape->edge_name == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {
15071505
RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);
15081506

15091507
RBASIC_SET_SHAPE_ID(obj, dest_shape_id);
15101508

15111509
RUBY_ASSERT(rb_shape_get_next_iv_shape(source_shape_id, id) == dest_shape_id);
1512-
RUBY_ASSERT(index < dest_shape->capacity);
1510+
RUBY_ASSERT(index < RSHAPE_CAPACITY(dest_shape_id));
15131511
}
15141512
else {
15151513
break;

yjit/src/cruby_bindings.inc.rs

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

zjit/src/cruby_bindings.inc.rs

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)