Skip to content

Commit e9b4bc1

Browse files
committed
Get rid of TOO_COMPLEX shape type
Instead it's now a `shape_id` flag. This allows to check if an object is complex without having to chase the `rb_shape_t` pointer.
1 parent 9714e04 commit e9b4bc1

8 files changed

Lines changed: 46 additions & 64 deletions

File tree

ext/objspace/objspace_dump.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -821,9 +821,6 @@ shape_id_i(shape_id_t shape_id, void *data)
821821
case SHAPE_T_OBJECT:
822822
dump_append(dc, ", \"shape_type\":\"T_OBJECT\"");
823823
break;
824-
case SHAPE_OBJ_TOO_COMPLEX:
825-
dump_append(dc, ", \"shape_type\":\"OBJ_TOO_COMPLEX\"");
826-
break;
827824
case SHAPE_OBJ_ID:
828825
dump_append(dc, ", \"shape_type\":\"OBJ_ID\"");
829826
break;

shape.c

Lines changed: 36 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -379,12 +379,6 @@ shape_frozen_p(shape_id_t shape_id)
379379
}
380380
#endif
381381

382-
static inline bool
383-
shape_too_complex_p(rb_shape_t *shape)
384-
{
385-
return shape->flags & SHAPE_FL_TOO_COMPLEX;
386-
}
387-
388382
void
389383
rb_shape_each_shape_id(each_shape_callback callback, void *data)
390384
{
@@ -529,7 +523,6 @@ rb_shape_alloc_new_child(ID id, rb_shape_t *shape, enum shape_type shape_type)
529523
redblack_cache_ancestors(new_shape);
530524
}
531525
break;
532-
case SHAPE_OBJ_TOO_COMPLEX:
533526
case SHAPE_ROOT:
534527
case SHAPE_T_OBJECT:
535528
rb_bug("Unreachable");
@@ -619,9 +612,6 @@ get_next_shape_internal_atomic(rb_shape_t *shape, ID id, enum shape_type shape_t
619612
static rb_shape_t *
620613
get_next_shape_internal(rb_shape_t *shape, ID id, enum shape_type shape_type, bool *variation_created, bool new_variations_allowed)
621614
{
622-
// There should never be outgoing edges from "too complex", except for SHAPE_OBJ_ID
623-
RUBY_ASSERT(!shape_too_complex_p(shape) || shape_type == SHAPE_OBJ_ID);
624-
625615
if (rb_multi_ractor_p()) {
626616
return get_next_shape_internal_atomic(shape, id, shape_type, variation_created, new_variations_allowed);
627617
}
@@ -691,6 +681,7 @@ remove_shape_recursive(rb_shape_t *shape, ID id, rb_shape_t **removed_shape)
691681
if (shape->parent_id == INVALID_SHAPE_ID) {
692682
// We've hit the top of the shape tree and couldn't find the
693683
// IV we wanted to remove, so return NULL
684+
*removed_shape = NULL;
694685
return NULL;
695686
}
696687
else {
@@ -706,23 +697,14 @@ remove_shape_recursive(rb_shape_t *shape, ID id, rb_shape_t **removed_shape)
706697
// We found a new parent. Create a child of the new parent that
707698
// has the same attributes as this shape.
708699
if (new_parent) {
709-
if (UNLIKELY(shape_too_complex_p(new_parent))) {
710-
return new_parent;
711-
}
712-
713700
bool dont_care;
714701
rb_shape_t *new_child = get_next_shape_internal(new_parent, shape->edge_name, shape->type, &dont_care, true);
715-
if (UNLIKELY(!new_child)) {
716-
return new_child;
717-
}
718-
719-
RUBY_ASSERT(new_child->capacity <= shape->capacity);
720-
702+
RUBY_ASSERT(!new_child || new_child->capacity <= shape->capacity);
721703
return new_child;
722704
}
723705
else {
724706
// We went all the way to the top of the shape tree and couldn't
725-
// find an IV to remove, so return NULL
707+
// find an IV to remove so return NULL.
726708
return NULL;
727709
}
728710
}
@@ -732,19 +714,27 @@ remove_shape_recursive(rb_shape_t *shape, ID id, rb_shape_t **removed_shape)
732714
shape_id_t
733715
rb_shape_transition_remove_ivar(VALUE obj, ID id, shape_id_t *removed_shape_id)
734716
{
735-
shape_id_t shape_id = rb_obj_shape_id(obj);
736-
rb_shape_t *shape = RSHAPE(shape_id);
717+
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
737718

738-
RUBY_ASSERT(!shape_too_complex_p(shape));
739-
RUBY_ASSERT(!shape_frozen_p(shape_id));
719+
RUBY_ASSERT(!rb_shape_too_complex_p(original_shape_id));
720+
RUBY_ASSERT(!shape_frozen_p(original_shape_id));
740721

741722
rb_shape_t *removed_shape = NULL;
742-
rb_shape_t *new_shape = remove_shape_recursive(shape, id, &removed_shape);
743-
if (new_shape) {
723+
rb_shape_t *new_shape = remove_shape_recursive(RSHAPE(original_shape_id), id, &removed_shape);
724+
725+
if (removed_shape) {
744726
*removed_shape_id = raw_shape_id(removed_shape);
745-
return raw_shape_id(new_shape);
746727
}
747-
return shape_id;
728+
729+
if (new_shape) {
730+
return shape_id(new_shape, original_shape_id);
731+
}
732+
else if (removed_shape) {
733+
// We found the shape to remove, but couldn't create a new variation.
734+
// We must transition to TOO_COMPLEX.
735+
return ROOT_TOO_COMPLEX_SHAPE_ID | (original_shape_id & SHAPE_ID_FLAGS_MASK);
736+
}
737+
return original_shape_id;
748738
}
749739

750740
shape_id_t
@@ -760,7 +750,7 @@ shape_id_t
760750
rb_shape_transition_complex(VALUE obj)
761751
{
762752
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
763-
return shape_id(ROOT_SHAPE_ID | SHAPE_ID_FL_TOO_COMPLEX, original_shape_id);
753+
return ROOT_TOO_COMPLEX_SHAPE_ID | (original_shape_id & SHAPE_ID_FLAGS_MASK);
764754
}
765755

766756
static inline bool
@@ -832,7 +822,6 @@ shape_get_iv_index(rb_shape_t *shape, ID id, attr_index_t *value)
832822
case SHAPE_ROOT:
833823
case SHAPE_T_OBJECT:
834824
return false;
835-
case SHAPE_OBJ_TOO_COMPLEX:
836825
case SHAPE_OBJ_ID:
837826
rb_bug("Ivar should not exist on transition");
838827
}
@@ -848,9 +837,6 @@ static inline rb_shape_t *
848837
shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings)
849838
{
850839
RUBY_ASSERT(!is_instance_id(id) || RTEST(rb_sym2str(ID2SYM(id))));
851-
if (UNLIKELY(shape_too_complex_p(shape))) {
852-
return shape;
853-
}
854840

855841
#if RUBY_DEBUG
856842
attr_index_t index;
@@ -874,6 +860,11 @@ shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings)
874860
bool variation_created = false;
875861
rb_shape_t *new_shape = get_next_shape_internal(shape, id, SHAPE_IVAR, &variation_created, allow_new_shape);
876862

863+
if (!new_shape) {
864+
// We could create a new variation, transitioning to TOO_COMPLEX.
865+
return NULL;
866+
}
867+
877868
// Check if we should update max_iv_count on the object's class
878869
if (obj != klass && new_shape->next_field_index > RCLASS_MAX_IV_COUNT(klass)) {
879870
RCLASS_SET_MAX_IV_COUNT(klass, new_shape->next_field_index);
@@ -999,11 +990,11 @@ shape_cache_get_iv_index(rb_shape_t *shape, ID id, attr_index_t *value)
999990
bool
1000991
rb_shape_get_iv_index(shape_id_t shape_id, ID id, attr_index_t *value)
1001992
{
1002-
rb_shape_t *shape = RSHAPE(shape_id);
1003-
1004993
// It doesn't make sense to ask for the index of an IV that's stored
1005994
// on an object that is "too complex" as it uses a hash for storing IVs
1006-
RUBY_ASSERT(!shape_too_complex_p(shape));
995+
RUBY_ASSERT(!rb_shape_too_complex_p(shape_id));
996+
997+
rb_shape_t *shape = RSHAPE(shape_id);
1007998

1008999
if (!shape_cache_get_iv_index(shape, id, value)) {
10091000
// If it wasn't in the ancestor cache, then don't do a linear search
@@ -1066,9 +1057,6 @@ shape_traverse_from_new_root(rb_shape_t *initial_shape, rb_shape_t *dest_shape)
10661057
case SHAPE_ROOT:
10671058
case SHAPE_T_OBJECT:
10681059
break;
1069-
case SHAPE_OBJ_TOO_COMPLEX:
1070-
rb_bug("Unreachable");
1071-
break;
10721060
}
10731061

10741062
return next_shape;
@@ -1085,17 +1073,17 @@ rb_shape_traverse_from_new_root(shape_id_t initial_shape_id, shape_id_t dest_sha
10851073
// Rebuild a similar shape with the same ivars but starting from
10861074
// a different SHAPE_T_OBJECT, and don't cary over non-canonical transitions
10871075
// such as SHAPE_OBJ_ID.
1088-
rb_shape_t *
1089-
rb_shape_rebuild_shape(rb_shape_t *initial_shape, rb_shape_t *dest_shape)
1076+
static rb_shape_t *
1077+
shape_rebuild(rb_shape_t *initial_shape, rb_shape_t *dest_shape)
10901078
{
10911079
rb_shape_t *midway_shape;
10921080

10931081
RUBY_ASSERT(initial_shape->type == SHAPE_T_OBJECT || initial_shape->type == SHAPE_ROOT);
10941082

10951083
if (dest_shape->type != initial_shape->type) {
1096-
midway_shape = rb_shape_rebuild_shape(initial_shape, RSHAPE(dest_shape->parent_id));
1097-
if (UNLIKELY(raw_shape_id(midway_shape) == ROOT_TOO_COMPLEX_SHAPE_ID)) {
1098-
return midway_shape;
1084+
midway_shape = shape_rebuild(initial_shape, RSHAPE(dest_shape->parent_id));
1085+
if (UNLIKELY(!midway_shape)) {
1086+
return NULL;
10991087
}
11001088
}
11011089
else {
@@ -1110,9 +1098,6 @@ rb_shape_rebuild_shape(rb_shape_t *initial_shape, rb_shape_t *dest_shape)
11101098
case SHAPE_ROOT:
11111099
case SHAPE_T_OBJECT:
11121100
break;
1113-
case SHAPE_OBJ_TOO_COMPLEX:
1114-
rb_bug("Unreachable");
1115-
break;
11161101
}
11171102

11181103
return midway_shape;
@@ -1124,7 +1109,7 @@ rb_shape_rebuild(shape_id_t initial_shape_id, shape_id_t dest_shape_id)
11241109
RUBY_ASSERT(!rb_shape_too_complex_p(initial_shape_id));
11251110
RUBY_ASSERT(!rb_shape_too_complex_p(dest_shape_id));
11261111

1127-
return raw_shape_id(rb_shape_rebuild_shape(RSHAPE(initial_shape_id), RSHAPE(dest_shape_id)));
1112+
return raw_shape_id(shape_rebuild(RSHAPE(initial_shape_id), RSHAPE(dest_shape_id)));
11281113
}
11291114

11301115
void
@@ -1171,7 +1156,7 @@ rb_shape_copy_complex_ivars(VALUE dest, VALUE obj, shape_id_t src_shape_id, st_t
11711156
RUBY_FUNC_EXPORTED bool
11721157
rb_shape_obj_too_complex_p(VALUE obj)
11731158
{
1174-
return shape_too_complex_p(obj_shape(obj));
1159+
return !RB_SPECIAL_CONST_P(obj) && rb_shape_too_complex_p(RBASIC_SHAPE_ID(obj));
11751160
}
11761161

11771162
size_t
@@ -1210,8 +1195,7 @@ static VALUE
12101195
shape_too_complex(VALUE self)
12111196
{
12121197
shape_id_t shape_id = NUM2INT(rb_struct_getmember(self, rb_intern("id")));
1213-
rb_shape_t *shape = RSHAPE(shape_id);
1214-
return RBOOL(shape_too_complex_p(shape));
1198+
return RBOOL(rb_shape_too_complex_p(shape_id));
12151199
}
12161200

12171201
static VALUE

shape.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ typedef uint32_t redblack_id_t;
3131
#define ROOT_SHAPE_ID 0x0
3232
#define ROOT_TOO_COMPLEX_SHAPE_ID (ROOT_SHAPE_ID | SHAPE_ID_FL_TOO_COMPLEX)
3333
#define SPECIAL_CONST_SHAPE_ID (ROOT_SHAPE_ID | SHAPE_ID_FL_FROZEN)
34-
#define FIRST_T_OBJECT_SHAPE_ID 0x2
34+
#define FIRST_T_OBJECT_SHAPE_ID 0x1
3535

3636
extern ID ruby_internal_object_id;
3737

@@ -63,7 +63,6 @@ enum shape_type {
6363
SHAPE_IVAR,
6464
SHAPE_OBJ_ID,
6565
SHAPE_T_OBJECT,
66-
SHAPE_OBJ_TOO_COMPLEX,
6766
};
6867

6968
enum shape_flags {

variable.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2240,10 +2240,6 @@ iterate_over_shapes_with_callback(rb_shape_t *shape, rb_ivar_foreach_callback_fu
22402240
}
22412241
}
22422242
return false;
2243-
case SHAPE_OBJ_TOO_COMPLEX:
2244-
default:
2245-
rb_bug("Unreachable");
2246-
UNREACHABLE_RETURN(false);
22472243
}
22482244
}
22492245

yjit.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,12 @@ rb_yjit_set_exception_return(rb_control_frame_t *cfp, void *leave_exit, void *le
832832
}
833833
}
834834

835+
bool
836+
rb_yjit_shape_too_complex_p(shape_id_t shape_id)
837+
{
838+
return rb_shape_too_complex_p(shape_id);
839+
}
840+
835841
// Primitives used by yjit.rb
836842
VALUE rb_yjit_stats_enabled_p(rb_execution_context_t *ec, VALUE self);
837843
VALUE rb_yjit_print_stats_p(rb_execution_context_t *ec, VALUE self);

yjit/bindgen/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ fn main() {
100100
.allowlist_function("rb_shape_get_iv_index")
101101
.allowlist_function("rb_shape_transition_add_ivar_no_warnings")
102102
.allowlist_function("rb_shape_obj_too_complex_p")
103-
.allowlist_function("rb_shape_too_complex_p")
103+
.allowlist_function("rb_yjit_shape_too_complex_p")
104104
.allowlist_var("SHAPE_ID_NUM_BITS")
105105

106106
// From ruby/internal/intern/object.h

yjit/src/codegen.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3124,7 +3124,7 @@ fn gen_set_ivar(
31243124

31253125
// If the VM ran out of shapes, or this class generated too many leaf,
31263126
// it may be de-optimized into OBJ_TOO_COMPLEX_SHAPE (hash-table).
3127-
new_shape_too_complex = unsafe { rb_shape_too_complex_p(next_shape_id) };
3127+
new_shape_too_complex = unsafe { rb_yjit_shape_too_complex_p(next_shape_id) };
31283128
if new_shape_too_complex {
31293129
Some((next_shape_id, None, 0_usize))
31303130
} else {

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.

0 commit comments

Comments
 (0)