8000 Get rid of `rb_shape_t.flags` · ruby/ruby@772fc1f · GitHub
[go: up one dir, main page]
Skip to content

Commit 772fc1f

Browse files
byrootbyroot
committed
Get rid of rb_shape_t.flags
Now all flags are only in the `shape_id_t`, and can all be checked without needing to dereference a pointer.
1 parent 111986f commit 772fc1f

File tree

7 files changed

+191
-76
lines changed

7 files changed

+191
-76
lines changed

‎gc.c

Lines changed: 58 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -666,9 +666,6 @@ typedef struct gc_function_map {
666666
void (*undefine_finalizer)(void *objspace_ptr, VALUE obj);
667667
void (*copy_finalizer)(void *objspace_ptr, VALUE dest, VALUE obj);
668668
void (*shutdown_call_finalizer)(void *objspace_ptr);
669-
// Object ID
670-
VALUE (*object_id)(void *objspace_ptr, VALUE obj);
671-
VALUE (*object_id_to_ref)(void *objspace_ptr, VALUE object_id);
672669
// Forking
673670
void (*before_fork)(void *objspace_ptr);
674671
void (*after_fork)(void *objspace_ptr, rb_pid_t pid);
@@ -1892,16 +1889,35 @@ class_object_id(VALUE klass)
18921889
return id;
18931890
}
18941891

1892+
static inline VALUE
1893+
object_id_get(VALUE obj, shape_id_t shape_id)
1894+
{
1895+
VALUE id;
1896+
if (rb_shape_too_complex_p(shape_id)) {
1897+
id = rb_obj_field_get(obj, ROOT_TOO_COMPLEX_WITH_OBJ_ID);
1898+
}
1899+
else {
1900+
id = rb_obj_field_get(obj, rb_shape_object_id(shape_id));
1901+
}
1902+
1903+
#if RUBY_DEBUG
1904+
if (!(FIXNUM_P(id) || RB_TYPE_P(id, T_BIGNUM))) {
1905+
rb_p(obj);
1906+
rb_bug("Object's shape includes object_id, but it's missing %s", rb_obj_info(obj));
1907+
}
1908+
#endif
1909+
1910+
return id;
1911+
}
1912+
18951913
static VALUE
18961914
object_id0(VALUE obj)
18971915
{
18981916
VALUE id = Qfalse;
1917+
shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
18991918

1900-
if (rb_shape_has_object_id(RBASIC_SHAPE_ID(obj))) {
1901-
shape_id_t object_id_shape_id = rb_shape_transition_object_id(obj);
1902-
id = rb_obj_field_get(obj, object_id_shape_id);
1903-
RUBY_ASSERT(id, "object_id missing");
1904-
return id;
1919+
if (rb_shape_has_object_id(shape_id)) {
1920+
return object_id_get(obj, shape_id);
19051921
}
19061922

19071923
// rb_shape_object_id_shape may lock if the current shape has
@@ -1910,6 +1926,10 @@ object_id0(VALUE obj)
19101926

19111927
id = generate_next_object_id();
19121928
rb_obj_field_set(obj, object_id_shape_id, id);
1929+
1930+
RUBY_ASSERT(RBASIC_SHAPE_ID(obj) == object_id_shape_id);
1931+
RUBY_ASSERT(rb_shape_obj_has_id(obj));
1932+
19131933
if (RB_UNLIKELY(id2ref_tbl)) {
19141934
st_insert(id2ref_tbl, (st_data_t)id, (st_data_t)obj);
19151935
}
@@ -2016,30 +2036,47 @@ obj_free_object_id(VALUE obj)
20162036
return;
20172037
}
20182038

2039+
#if RUBY_DEBUG
2040+
switch (BUILTIN_TYPE(obj)) {
2041+
case T_CLASS:
2042+
case T_MODULE:
2043+
break;
2044+
default:
2045+
if (rb_shape_obj_has_id(obj)) {
2046+
VALUE id = object_id_get(obj, RBASIC_SHAPE_ID(obj)); // Crash if missing
2047+
if (!(FIXNUM_P(id) || RB_TYPE_P(id, T_BIGNUM))) {
2048+
rb_p(obj);
2049+
rb_bug("Corrupted object_id");
2050+
}
2051+
}
2052+
break;
2053+
}
2054+
#endif
2055+
20192056
VALUE obj_id = 0;
20202057
if (RB_UNLIKELY(id2ref_tbl)) {
20212058
switch (BUILTIN_TYPE(obj)) {
20222059
case T_CLASS:
20232060
case T_MODULE:
2024-
if (RCLASS(obj)->object_id) {
2025-
obj_id = RCLASS(obj)->object_id;
2026-
}
2061+
obj_id = RCLASS(obj)->object_id;
20272062
break;
2028-
default:
2029-
if (rb_shape_obj_has_id(obj)) {
2030-
obj_id = object_id(obj);
2063+
default: {
2064+
shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
2065+
if (rb_shape_has_object_id(shape_id)) {
2066+
obj_id = object_id_get(obj, shape_id);
20312067
}
20322068
break;
2069+
}
20332070
}
2034-
}
20352071

2036-
if (RB_UNLIKELY(obj_id)) {
2037-
RUBY_ASSERT(FIXNUM_P(obj_id) || RB_TYPE_P(obj, T_BIGNUM));
2072+
if (RB_UNLIKELY(obj_id)) {
2073+
RUBY_ASSERT(FIXNUM_P(obj_id) || RB_TYPE_P(obj_id, T_BIGNUM));
20382074

2039-
if (!st_delete(id2ref_tbl, (st_data_t *)&obj_id, NULL)) {
2040-
// If we're currently building the table then it's not a bug
2041-
if (id2ref_tbl_built) {
2042-
rb_bug("Object ID seen, but not in _id2ref table: object_id=%llu object=%s", NUM2ULL(obj_id), rb_obj_info(obj));
2075+
if (!st_delete(id2ref_tbl, (st_data_t *)&obj_id, NULL)) {
2076+
// If we're currently building the table then it's not a bug
2077+
if (id2ref_tbl_built) {
2078+
rb_bug("Object ID seen, but not in _id2ref table: object_id=%llu object=%s", NUM2ULL(obj_id), rb_obj_info(obj));
2079+
}
20432080
}
20442081
}
20452082
}

‎shape.c

Lines changed: 83 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -424,12 +424,6 @@ rb_shape_depth(shape_id_t shape_id)
424424
return depth;
425425
}
426426

427-
static inline rb_shape_t *
428-
obj_shape(VALUE obj)
429-
{
430-
return RSHAPE(rb_obj_shape_id(obj));
431-
}
432-
433427
static rb_shape_t *
434428
shape_alloc(void)
435429
{
@@ -461,7 +455,6 @@ rb_shape_alloc(ID edge_name, rb_shape_t *parent, enum shape_type type)
461455
{
462456
rb_shape_t *shape = rb_shape_alloc_with_parent_id(edge_name, raw_shape_id(parent));
463457
shape->type = (uint8_t)type;
464-
shape->flags = parent->flags;
465458
shape->heap_index = parent->heap_index;
466459
shape->capacity = parent->capacity;
467460
shape->edges = 0;
@@ -510,8 +503,6 @@ rb_shape_alloc_new_child(ID id, rb_shape_t *shape, enum shape_type shape_type)
510503

511504
switch (shape_type) {
512505
case SHAPE_OBJ_ID:
513-
new_shape->flags |= SHAPE_FL_HAS_OBJECT_ID;
514-
// fallthrough
515506
case SHAPE_IVAR:
516507
if (UNLIKELY(shape->next_field_index >= shape->capacity)) {
517508
RUBY_ASSERT(shape->next_field_index == shape->capacity);
@@ -711,6 +702,16 @@ remove_shape_recursive(rb_shape_t *shape, ID id, rb_shape_t **removed_shape)
711702
}
712703
}
713704

705+
static inline shape_id_t
706+
transition_frozen(shape_id_t shape_id)
707+
{
708+
if (rb_shape_has_object_id(shape_id)) {
709+
return ROOT_TOO_COMPLEX_WITH_OBJ_ID | (shape_id & SHAPE_ID_FLAGS_MASK);
710+
}
711+
return ROOT_TOO_COMPLEX_SHAPE_ID | (shape_id & SHAPE_ID_FLAGS_MASK);
712+
}
713+
714+
714715
shape_id_t
715716
rb_shape_transition_remove_ivar(VALUE obj, ID id, shape_id_t *removed_shape_id)
716717
{
@@ -732,7 +733,7 @@ rb_shape_transition_remove_ivar(VALUE obj, ID id, shape_id_t *removed_shape_id)
732733
else if (removed_shape) {
733734
// We found the shape to remove, but couldn't create a new variation.
734735
// We must transition to TOO_COMPLEX.
735-
return ROOT_TOO_COMPLEX_SHAPE_ID | (original_shape_id & SHAPE_ID_FLAGS_MASK);
736+
return transition_frozen(original_shape_id);
736737
}
737738
return original_shape_id;
738739
}
@@ -749,41 +750,42 @@ rb_shape_transition_frozen(VALUE obj)
749750
shape_id_t
750751
rb_shape_transition_complex(VALUE obj)
751752
{
752-
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
753-
return ROOT_TOO_COMPLEX_SHAPE_ID | (original_shape_id & SHAPE_ID_FLAGS_MASK);
753+
return transition_frozen(RBASIC_SHAPE_ID(obj));
754754
}
755755

756-
static inline bool
757-
shape_has_object_id(rb_shape_t *shape)
756+
shape_id_t
757+
rb_shape_transition_object_id(VALUE obj)
758758
{
759-
return shape->flags & SHAPE_FL_HAS_OBJECT_ID;
760-
}
759+
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
761760

762-
bool
763-
rb_shape_has_object_id(shape_id_t shape_id)
764-
{
765-
return shape_has_object_id(RSHAPE(shape_id));
761+
RUBY_ASSERT(!rb_shape_has_object_id(original_shape_id));
762+
763+
rb_shape_t *shape = NULL;
764+
if (!rb_shape_too_complex_p(original_shape_id)) {
765+
bool dont_care;
766+
shape = get_next_shape_internal(RSHAPE(original_shape_id), ruby_internal_object_id, SHAPE_OBJ_ID, &dont_care, true);
767+
}
768+
769+
if (!shape) {
770+
shape = RSHAPE(ROOT_TOO_COMPLEX_WITH_OBJ_ID);
771+
}
772+
return shape_id(shape, original_shape_id) | SHAPE_ID_FL_HAS_OBJECT_ID;
766773
}
767774

768775
shape_id_t
769-
rb_shape_transition_object_id(VALUE obj)
776+
rb_shape_object_id(shape_id_t original_shape_id)
770777
{
771-
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
772-
773-
rb_shape_t* shape = RSHAPE(original_shape_id);
774-
RUBY_ASSERT(shape);
778+
RUBY_ASSERT(rb_shape_has_object_id(original_shape_id));
775779

776-
if (shape->flags & SHAPE_FL_HAS_OBJECT_ID) {
777-
while (shape->type != SHAPE_OBJ_ID) {
778-
shape = RSHAPE(shape->parent_id);
780+
rb_shape_t *shape = RSHAPE(original_shape_id);
781+
while (shape->type != SHAPE_OBJ_ID) {
782+
if (UNLIKELY(shape->parent_id == INVALID_SHAPE_ID)) {
783+
rb_bug("Missing object_id in shape tree");
779784
}
785+
shape = RSHAPE(shape->parent_id);
780786
}
781-
else {
782-
bool dont_care;
783-
shape = get_next_shape_internal(shape, ruby_internal_object_id, SHAPE_OBJ_ID, &dont_care, true);
784-
}
785-
RUBY_ASSERT(shape);
786-
return shape_id(shape, original_shape_id);
787+
788+
return shape_id(shape, original_shape_id) | SHAPE_ID_FL_HAS_OBJECT_ID;
787789
}
788790

789791
/*
@@ -892,17 +894,19 @@ shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings)
892894
shape_id_t
893895
rb_shape_transition_add_ivar(VALUE obj, ID id)
894896
{
895-
RUBY_ASSERT(!shape_frozen_p(RBASIC_SHAPE_ID(obj)));
897+
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
898+
RUBY_ASSERT(!shape_frozen_p(original_shape_id));
896899

897-
return raw_shape_id(shape_get_next(obj_shape(obj), obj, id, true));
900+
return shape_id(shape_get_next(RSHAPE(original_shape_id), obj, id, true), original_shape_id);
898901
}
899902

900903
shape_id_t
901904
rb_shape_transition_add_ivar_no_warnings(VALUE obj, ID id)
902905
{
903-
RUBY_ASSERT(!shape_frozen_p(RBASIC_SHAPE_ID(obj)));
906+
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
907+
RUBY_ASSERT(!shape_frozen_p(original_shape_id));
904908

905-
return raw_shape_id(shape_get_next(obj_shape(obj), obj, id, false));
909+
return shape_id(shape_get_next(RSHAPE(original_shape_id), obj, id, false), original_shape_id);
906910
}
907911

908912
// Same as rb_shape_get_iv_index, but uses a provided valid shape id and index
@@ -1180,7 +1184,40 @@ rb_shape_memsize(shape_id_t shape_id)
11801184
return memsize;
11811185
}
11821186

1187+
#if RUBY_DEBUG
1188+
bool
1189+
rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id)
1190+
{
1191+
rb_shape_t *shape = RSHAPE(shape_id);
1192+
1193+
bool has_object_id = false;
1194+
while (shape->parent_id != INVALID_SHAPE_ID) {
1195+
if (shape->type == SHAPE_OBJ_ID) {
1196+
has_object_id = true;
1197+
break;
1198+
}
1199+
shape = RSHAPE(shape->parent_id);
1200+
}
1201+
1202+
if (rb_shape_has_object_id(shape_id)) {
1203+
if (!has_object_id) {
1204+
rb_p(obj);
1205+
rb_bug("shape_id claim having obj_id but doesn't shape_id=%u, obj=%s", shape_id, rb_obj_info(obj));
1206+
}
1207+
}
1208+
else {
1209+
if (has_object_id) {
1210+
rb_p(obj);
1211+
rb_bug("shape_id claim not having obj_id but it does shape_id=%u, obj=%s", shape_id, rb_obj_info(obj));
1212+
}
1213+
}
1214+
1215+
return true;
1216+
}
1217+
#endif
1218+
11831219
#if SHAPE_DEBUG
1220+
11841221
/*
11851222
* Exposing Shape to Ruby via RubyVM.debug_shape
11861223
*/
@@ -1203,8 +1240,7 @@ static VALUE
12031240
shape_has_object_id_p(VALUE self)
12041241
{
12051242
shape_id_t shape_id = NUM2INT(rb_struct_getmember(self, rb_intern("id")));
1206-
rb_shape_t *shape = RSHAPE(shape_id);
1207-
return RBOOL(shape_has_object_id(shape));
1243+
return RBOOL(rb_shape_has_object_id(shape_id));
12081244
}
12091245

12101246
static VALUE
@@ -1441,6 +1477,13 @@ Init_default_shapes(void)
14411477
GET_SHAPE_TREE()->root_shape = root;
14421478
RUBY_ASSERT(raw_shape_id(GET_SHAPE_TREE()->root_shape) == ROOT_SHAPE_ID);
14431479

1480+
rb_shape_t *root_with_obj_id = rb_shape_alloc_with_parent_id(0, ROOT_SHAPE_ID);
1481+
root_with_obj_id->type = SHAPE_OBJ_ID;
1482+
root_with_obj_id->edge_name = ruby_internal_object_id;
1483+
root_with_obj_id->next_field_index++;
1484+
root_with_obj_id->heap_index = 0;
1485+
RUBY_ASSERT(raw_shape_id(root_with_obj_id) == ROOT_SHAPE_WITH_OBJ_ID);
1486+
14441487
// Make shapes for T_OBJECT
14451488
size_t *sizes = rb_gc_heap_sizes();
14461489
for (int i = 0; sizes[i] > 0; i++) {
@@ -1489,7 +1532,6 @@ Init_shape(void)
14891532
rb_define_const(rb_cShape, "SHAPE_ID_NUM_BITS", INT2NUM(SHAPE_ID_NUM_BITS));
14901533
rb_define_const(rb_cShape, "SHAPE_FLAG_SHIFT", INT2NUM(SHAPE_FLAG_SHIFT));
14911534
rb_define_const(rb_cShape, "SPECIAL_CONST_SHAPE_ID", INT2NUM(SPECIAL_CONST_SHAPE_ID));
1492-
rb_define_const(rb_cShape, "ROOT_TOO_COMPLEX_SHAPE_ID", INT2NUM(ROOT_TOO_COMPLEX_SHAPE_ID));
14931535
rb_define_const(rb_cShape, "FIRST_T_OBJECT_SHAPE_ID", INT2NUM(FIRST_T_OBJECT_SHAPE_ID));
14941536
rb_define_const(rb_cShape, "SHAPE_MAX_VARIATIONS", INT2NUM(SHAPE_MAX_VARIATIONS));
14951537
rb_define_const(rb_cShape, "SIZEOF_RB_SHAPE_T", INT2NUM(sizeof(rb_shape_t)));

0 commit comments

Comments
 (0)
0