1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Get rid of rb_shape_t.flags

Browse Source 
Now all flags are only in the `shape_id_t`, and can all be checked
without needing to dereference a pointer.
This commit is contained in:
Jean Boussier 2025-06-04 11:11:24 +02:00
parent 111986f8b0
commit 772fc1f187
Notes: git 2025-06-05 05:44:57 +00:00 
Merged: https://github.com/ruby/ruby/pull/13515
7 changed files with 193 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
gc.c
View File

@ -666,9 +666,6 @@ typedef struct gc_function_map {
void (*undefine_finalizer)(void *objspace_ptr, VALUE obj);
void (*copy_finalizer)(void *objspace_ptr, VALUE dest, VALUE obj);
void (*shutdown_call_finalizer)(void *objspace_ptr);
// Object ID
VALUE (*object_id)(void *objspace_ptr, VALUE obj);
VALUE (*object_id_to_ref)(void *objspace_ptr, VALUE object_id);
// Forking
void (*before_fork)(void *objspace_ptr);
void (*after_fork)(void *objspace_ptr, rb_pid_t pid);
@ -1892,16 +1889,35 @@ class_object_id(VALUE klass)
return id;
}
static inline VALUE
object_id_get(VALUE obj, shape_id_t shape_id)
{
VALUE id;
if (rb_shape_too_complex_p(shape_id)) {
id = rb_obj_field_get(obj, ROOT_TOO_COMPLEX_WITH_OBJ_ID);
}
else {
id = rb_obj_field_get(obj, rb_shape_object_id(shape_id));
}
#if RUBY_DEBUG
if (!(FIXNUM_P(id) || RB_TYPE_P(id, T_BIGNUM))) {
rb_p(obj);
rb_bug("Object's shape includes object_id, but it's missing %s", rb_obj_info(obj));
}
#endif
return id;
}
static VALUE
object_id0(VALUE obj)
{
VALUE id = Qfalse;
shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
if (rb_shape_has_object_id(RBASIC_SHAPE_ID(obj))) {
shape_id_t object_id_shape_id = rb_shape_transition_object_id(obj);
id = rb_obj_field_get(obj, object_id_shape_id);
RUBY_ASSERT(id, "object_id missing");
return id;
if (rb_shape_has_object_id(shape_id)) {
return object_id_get(obj, shape_id);
}
// rb_shape_object_id_shape may lock if the current shape has
@ -1910,6 +1926,10 @@ object_id0(VALUE obj)
id = generate_next_object_id();
rb_obj_field_set(obj, object_id_shape_id, id);
RUBY_ASSERT(RBASIC_SHAPE_ID(obj) == object_id_shape_id);
RUBY_ASSERT(rb_shape_obj_has_id(obj));
if (RB_UNLIKELY(id2ref_tbl)) {
st_insert(id2ref_tbl, (st_data_t)id, (st_data_t)obj);
}
@ -2016,30 +2036,47 @@ obj_free_object_id(VALUE obj)
return;
}
#if RUBY_DEBUG
switch (BUILTIN_TYPE(obj)) {
case T_CLASS:
case T_MODULE:
break;
default:
if (rb_shape_obj_has_id(obj)) {
VALUE id = object_id_get(obj, RBASIC_SHAPE_ID(obj)); // Crash if missing
if (!(FIXNUM_P(id) || RB_TYPE_P(id, T_BIGNUM))) {
rb_p(obj);
rb_bug("Corrupted object_id");
}
}
break;
}
#endif
VALUE obj_id = 0;
if (RB_UNLIKELY(id2ref_tbl)) {
switch (BUILTIN_TYPE(obj)) {
case T_CLASS:
case T_MODULE:
if (RCLASS(obj)->object_id) {
obj_id = RCLASS(obj)->object_id;
}
break;
default:
if (rb_shape_obj_has_id(obj)) {
obj_id = object_id(obj);
obj_id = RCLASS(obj)->object_id;
break;
default: {
shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
if (rb_shape_has_object_id(shape_id)) {
obj_id = object_id_get(obj, shape_id);
}
break;
}
}
}
if (RB_UNLIKELY(obj_id)) {
RUBY_ASSERT(FIXNUM_P(obj_id) || RB_TYPE_P(obj, T_BIGNUM));
if (RB_UNLIKELY(obj_id)) {
RUBY_ASSERT(FIXNUM_P(obj_id) || RB_TYPE_P(obj_id, T_BIGNUM));
if (!st_delete(id2ref_tbl, (st_data_t *)&obj_id, NULL)) {
// If we're currently building the table then it's not a bug
if (id2ref_tbl_built) {
rb_bug("Object ID seen, but not in _id2ref table: object_id=%llu object=%s", NUM2ULL(obj_id), rb_obj_info(obj));
if (!st_delete(id2ref_tbl, (st_data_t *)&obj_id, NULL)) {
// If we're currently building the table then it's not a bug
if (id2ref_tbl_built) {
rb_bug("Object ID seen, but not in _id2ref table: object_id=%llu object=%s", NUM2ULL(obj_id), rb_obj_info(obj));
}
}
}
}

126
shape.c
View File

@ -424,12 +424,6 @@ rb_shape_depth(shape_id_t shape_id)
return depth;
}
static inline rb_shape_t *
obj_shape(VALUE obj)
{
return RSHAPE(rb_obj_shape_id(obj));
}
static rb_shape_t *
shape_alloc(void)
{
@ -461,7 +455,6 @@ rb_shape_alloc(ID edge_name, rb_shape_t *parent, enum shape_type type)
{
rb_shape_t *shape = rb_shape_alloc_with_parent_id(edge_name, raw_shape_id(parent));
shape->type = (uint8_t)type;
shape->flags = parent->flags;
shape->heap_index = parent->heap_index;
shape->capacity = parent->capacity;
shape->edges = 0;
@ -510,8 +503,6 @@ rb_shape_alloc_new_child(ID id, rb_shape_t *shape, enum shape_type shape_type)
switch (shape_type) {
case SHAPE_OBJ_ID:
new_shape->flags |= SHAPE_FL_HAS_OBJECT_ID;
// fallthrough
case SHAPE_IVAR:
if (UNLIKELY(shape->next_field_index >= shape->capacity)) {
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)
}
}
static inline shape_id_t
transition_frozen(shape_id_t shape_id)
{
if (rb_shape_has_object_id(shape_id)) {
return ROOT_TOO_COMPLEX_WITH_OBJ_ID | (shape_id & SHAPE_ID_FLAGS_MASK);
}
return ROOT_TOO_COMPLEX_SHAPE_ID | (shape_id & SHAPE_ID_FLAGS_MASK);
}
shape_id_t
rb_shape_transition_remove_ivar(VALUE obj, ID id, shape_id_t *removed_shape_id)
{
@ -732,7 +733,7 @@ rb_shape_transition_remove_ivar(VALUE obj, ID id, shape_id_t *removed_shape_id)
else if (removed_shape) {
// We found the shape to remove, but couldn't create a new variation.
// We must transition to TOO_COMPLEX.
return ROOT_TOO_COMPLEX_SHAPE_ID | (original_shape_id & SHAPE_ID_FLAGS_MASK);
return transition_frozen(original_shape_id);
}
return original_shape_id;
}
@ -749,20 +750,7 @@ rb_shape_transition_frozen(VALUE obj)
shape_id_t
rb_shape_transition_complex(VALUE obj)
{
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
return ROOT_TOO_COMPLEX_SHAPE_ID | (original_shape_id & SHAPE_ID_FLAGS_MASK);
}
static inline bool
shape_has_object_id(rb_shape_t *shape)
{
return shape->flags & SHAPE_FL_HAS_OBJECT_ID;
}
bool
rb_shape_has_object_id(shape_id_t shape_id)
{
return shape_has_object_id(RSHAPE(shape_id));
return transition_frozen(RBASIC_SHAPE_ID(obj));
}
shape_id_t
@ -770,20 +758,34 @@ rb_shape_transition_object_id(VALUE obj)
{
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
rb_shape_t* shape = RSHAPE(original_shape_id);
RUBY_ASSERT(shape);
RUBY_ASSERT(!rb_shape_has_object_id(original_shape_id));
if (shape->flags & SHAPE_FL_HAS_OBJECT_ID) {
while (shape->type != SHAPE_OBJ_ID) {
shape = RSHAPE(shape->parent_id);
}
}
else {
rb_shape_t *shape = NULL;
if (!rb_shape_too_complex_p(original_shape_id)) {
bool dont_care;
shape = get_next_shape_internal(shape, ruby_internal_object_id, SHAPE_OBJ_ID, &dont_care, true);
shape = get_next_shape_internal(RSHAPE(original_shape_id), ruby_internal_object_id, SHAPE_OBJ_ID, &dont_care, true);
}
RUBY_ASSERT(shape);
return shape_id(shape, original_shape_id);
if (!shape) {
shape = RSHAPE(ROOT_TOO_COMPLEX_WITH_OBJ_ID);
}
return shape_id(shape, original_shape_id) | SHAPE_ID_FL_HAS_OBJECT_ID;
}
shape_id_t
rb_shape_object_id(shape_id_t original_shape_id)
{
RUBY_ASSERT(rb_shape_has_object_id(original_shape_id));
rb_shape_t *shape = RSHAPE(original_shape_id);
while (shape->type != SHAPE_OBJ_ID) {
if (UNLIKELY(shape->parent_id == INVALID_SHAPE_ID)) {
rb_bug("Missing object_id in shape tree");
}
shape = RSHAPE(shape->parent_id);
}
return shape_id(shape, original_shape_id) | SHAPE_ID_FL_HAS_OBJECT_ID;
}
/*
@ -892,17 +894,19 @@ shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings)
shape_id_t
rb_shape_transition_add_ivar(VALUE obj, ID id)
{
RUBY_ASSERT(!shape_frozen_p(RBASIC_SHAPE_ID(obj)));
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
RUBY_ASSERT(!shape_frozen_p(original_shape_id));
return raw_shape_id(shape_get_next(obj_shape(obj), obj, id, true));
return shape_id(shape_get_next(RSHAPE(original_shape_id), obj, id, true), original_shape_id);
}
shape_id_t
rb_shape_transition_add_ivar_no_warnings(VALUE obj, ID id)
{
RUBY_ASSERT(!shape_frozen_p(RBASIC_SHAPE_ID(obj)));
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
RUBY_ASSERT(!shape_frozen_p(original_shape_id));
return raw_shape_id(shape_get_next(obj_shape(obj), obj, id, false));
return shape_id(shape_get_next(RSHAPE(original_shape_id), obj, id, false), original_shape_id);
}
// 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)
return memsize;
}
#if RUBY_DEBUG
bool
rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id)
{
rb_shape_t *shape = RSHAPE(shape_id);
bool has_object_id = false;
while (shape->parent_id != INVALID_SHAPE_ID) {
if (shape->type == SHAPE_OBJ_ID) {
has_object_id = true;
break;
}
shape = RSHAPE(shape->parent_id);
}
if (rb_shape_has_object_id(shape_id)) {
if (!has_object_id) {
rb_p(obj);
rb_bug("shape_id claim having obj_id but doesn't shape_id=%u, obj=%s", shape_id, rb_obj_info(obj));
}
}
else {
if (has_object_id) {
rb_p(obj);
rb_bug("shape_id claim not having obj_id but it does shape_id=%u, obj=%s", shape_id, rb_obj_info(obj));
}
}
return true;
}
#endif
#if SHAPE_DEBUG
/*
* Exposing Shape to Ruby via RubyVM.debug_shape
*/
@ -1203,8 +1240,7 @@ static VALUE
shape_has_object_id_p(VALUE self)
{
shape_id_t shape_id = NUM2INT(rb_struct_getmember(self, rb_intern("id")));
rb_shape_t *shape = RSHAPE(shape_id);
return RBOOL(shape_has_object_id(shape));
return RBOOL(rb_shape_has_object_id(shape_id));
}
static VALUE
@ -1441,6 +1477,13 @@ Init_default_shapes(void)
GET_SHAPE_TREE()->root_shape = root;
RUBY_ASSERT(raw_shape_id(GET_SHAPE_TREE()->root_shape) == ROOT_SHAPE_ID);
rb_shape_t *root_with_obj_id = rb_shape_alloc_with_parent_id(0, ROOT_SHAPE_ID);
root_with_obj_id->type = SHAPE_OBJ_ID;
root_with_obj_id->edge_name = ruby_internal_object_id;
root_with_obj_id->next_field_index++;
root_with_obj_id->heap_index = 0;
RUBY_ASSERT(raw_shape_id(root_with_obj_id) == ROOT_SHAPE_WITH_OBJ_ID);
// Make shapes for T_OBJECT
size_t *sizes = rb_gc_heap_sizes();
for (int i = 0; sizes[i] > 0; i++) {
@ -1489,7 +1532,6 @@ Init_shape(void)
rb_define_const(rb_cShape, "SHAPE_ID_NUM_BITS", INT2NUM(SHAPE_ID_NUM_BITS));
rb_define_const(rb_cShape, "SHAPE_FLAG_SHIFT", INT2NUM(SHAPE_FLAG_SHIFT));
rb_define_const(rb_cShape, "SPECIAL_CONST_SHAPE_ID", INT2NUM(SPECIAL_CONST_SHAPE_ID));
rb_define_const(rb_cShape, "ROOT_TOO_COMPLEX_SHAPE_ID", INT2NUM(ROOT_TOO_COMPLEX_SHAPE_ID));
rb_define_const(rb_cShape, "FIRST_T_OBJECT_SHAPE_ID", INT2NUM(FIRST_T_OBJECT_SHAPE_ID));
rb_define_const(rb_cShape, "SHAPE_MAX_VARIATIONS", INT2NUM(SHAPE_MAX_VARIATIONS));
rb_define_const(rb_cShape, "SIZEOF_RB_SHAPE_T", INT2NUM(sizeof(rb_shape_t)));

28
shape.h
View File

@ -14,7 +14,9 @@ STATIC_ASSERT(shape_id_num_bits, SHAPE_ID_NUM_BITS == sizeof(shape_id_t) * CHAR_
#define SHAPE_ID_OFFSET_MASK (SHAPE_BUFFER_SIZE - 1)
#define SHAPE_ID_FLAGS_MASK (shape_id_t)(((1 << (SHAPE_ID_NUM_BITS - SHAPE_ID_OFFSET_NUM_BITS)) - 1) << SHAPE_ID_OFFSET_NUM_BITS)
#define SHAPE_ID_FL_FROZEN (SHAPE_FL_FROZEN << SHAPE_ID_OFFSET_NUM_BITS)
#define SHAPE_ID_FL_HAS_OBJECT_ID (SHAPE_FL_HAS_OBJECT_ID << SHAPE_ID_OFFSET_NUM_BITS)
#define SHAPE_ID_FL_TOO_COMPLEX (SHAPE_FL_TOO_COMPLEX << SHAPE_ID_OFFSET_NUM_BITS)
#define SHAPE_ID_FL_NON_CANONICAL_MASK (SHAPE_FL_NON_CANONICAL_MASK << SHAPE_ID_OFFSET_NUM_BITS)
#define SHAPE_ID_READ_ONLY_MASK (~SHAPE_ID_FL_FROZEN)
typedef uint32_t redblack_id_t;
@ -28,10 +30,12 @@ typedef uint32_t redblack_id_t;
#define INVALID_SHAPE_ID ((shape_id_t)-1)
#define ATTR_INDEX_NOT_SET ((attr_index_t)-1)
#define ROOT_SHAPE_ID 0x0
#define ROOT_TOO_COMPLEX_SHAPE_ID (ROOT_SHAPE_ID | SHAPE_ID_FL_TOO_COMPLEX)
#define SPECIAL_CONST_SHAPE_ID (ROOT_SHAPE_ID | SHAPE_ID_FL_FROZEN)
#define FIRST_T_OBJECT_SHAPE_ID 0x1
#define ROOT_SHAPE_ID 0x0
#define ROOT_SHAPE_WITH_OBJ_ID 0x1
#define ROOT_TOO_COMPLEX_SHAPE_ID (ROOT_SHAPE_ID | SHAPE_ID_FL_TOO_COMPLEX)
#define ROOT_TOO_COMPLEX_WITH_OBJ_ID (ROOT_SHAPE_WITH_OBJ_ID | SHAPE_ID_FL_TOO_COMPLEX | SHAPE_ID_FL_HAS_OBJECT_ID)
#define SPECIAL_CONST_SHAPE_ID (ROOT_SHAPE_ID | SHAPE_ID_FL_FROZEN)
#define FIRST_T_OBJECT_SHAPE_ID 0x2
extern ID ruby_internal_object_id;
@ -46,7 +50,6 @@ struct rb_shape {
attr_index_t capacity; // Total capacity of the object with this shape
uint8_t type;
uint8_t heap_index;
uint8_t flags;
};
typedef struct rb_shape rb_shape_t;
@ -119,11 +122,16 @@ RBASIC_SHAPE_ID_FOR_READ(VALUE obj)
return RBASIC_SHAPE_ID(obj) & SHAPE_ID_READ_ONLY_MASK;
}
#if RUBY_DEBUG
bool rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id);
#endif
static inline void
RBASIC_SET_SHAPE_ID(VALUE obj, shape_id_t shape_id)
{
RUBY_ASSERT(!RB_SPECIAL_CONST_P(obj));
RUBY_ASSERT(!RB_TYPE_P(obj, T_IMEMO));
RUBY_ASSERT(rb_shape_verify_consistency(obj, shape_id));
#if RBASIC_SHAPE_ID_FIELD
RBASIC(obj)->shape_id = (VALUE)shape_id;
#else
@ -142,7 +150,6 @@ RUBY_FUNC_EXPORTED shape_id_t rb_obj_shape_id(VALUE obj);
shape_id_t rb_shape_get_next_iv_shape(shape_id_t shape_id, ID id);
bool rb_shape_get_iv_index(shape_id_t shape_id, ID id, attr_index_t *value);
bool rb_shape_get_iv_index_with_hint(shape_id_t shape_id, ID id, attr_index_t *value, shape_id_t *shape_id_hint);
bool rb_shape_has_object_id(shape_id_t shape_id);
shape_id_t rb_shape_transition_frozen(VALUE obj);
shape_id_t rb_shape_transition_complex(VALUE obj);
@ -150,6 +157,7 @@ shape_id_t rb_shape_transition_remove_ivar(VALUE obj, ID id, shape_id_t *removed
shape_id_t rb_shape_transition_add_ivar(VALUE obj, ID id);
shape_id_t rb_shape_transition_add_ivar_no_warnings(VALUE obj, ID id);
shape_id_t rb_shape_transition_object_id(VALUE obj);
shape_id_t rb_shape_object_id(shape_id_t original_shape_id);
void rb_shape_free_all(void);
@ -169,10 +177,16 @@ rb_shape_obj_too_complex_p(VALUE obj)
return !RB_SPECIAL_CONST_P(obj) && rb_shape_too_complex_p(RBASIC_SHAPE_ID(obj));
}
static inline bool
rb_shape_has_object_id(shape_id_t shape_id)
{
return shape_id & SHAPE_ID_FL_HAS_OBJECT_ID;
}
static inline bool
rb_shape_canonical_p(shape_id_t shape_id)
{
return !(shape_id & SHAPE_ID_FLAGS_MASK) && !RSHAPE(shape_id)->flags;
return !(shape_id & SHAPE_ID_FL_NON_CANONICAL_MASK);
}
static inline shape_id_t

18
test/ruby/test_shapes.rb
View File

@ -651,6 +651,22 @@ class TestShapes < Test::Unit::TestCase
end;
end
def test_object_id_transition_too_complex
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
begin;
class Hi; end
obj = Hi.new
obj.instance_variable_set(:@a, 1)
obj.instance_variable_set(:@b, 2)
old_id = obj.object_id
RubyVM::Shape.exhaust_shapes
obj.remove_instance_variable(:@a)
assert_equal old_id, obj.object_id
end;
end
def test_too_complex_and_frozen_and_object_id
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
begin;
@ -676,7 +692,7 @@ class TestShapes < Test::Unit::TestCase
assert_predicate frozen_shape, :shape_frozen?
refute_predicate frozen_shape, :has_object_id?
tc.object_id
assert_equal tc.object_id, tc.object_id
id_shape = RubyVM::Shape.of(tc)
refute_equal frozen_shape.id, id_shape.id

16
variable.c
View File

@ -1343,6 +1343,13 @@ rb_obj_field_get(VALUE obj, shape_id_t target_shape_id)
}
VALUE value = Qundef;
st_lookup(fields_hash, RSHAPE(target_shape_id)->edge_name, &value);
#if RUBY_DEBUG
if (UNDEF_P(value)) {
rb_bug("Object's shape includes object_id, but it's missing %s", rb_obj_info(obj));
}
#endif
RUBY_ASSERT(!UNDEF_P(value));
return value;
}
@ -1617,13 +1624,13 @@ obj_transition_too_complex(VALUE obj, st_table *table)
if (!(RBASIC(obj)->flags & ROBJECT_EMBED)) {
old_fields = ROBJECT_FIELDS(obj);
}
rb_obj_set_shape_id(obj, shape_id);
RBASIC_SET_SHAPE_ID(obj, shape_id);
ROBJECT_SET_FIELDS_HASH(obj, table);
break;
case T_CLASS:
case T_MODULE:
old_fields = RCLASS_PRIME_FIELDS(obj);
rb_obj_set_shape_id(obj, shape_id);
RBASIC_SET_SHAPE_ID(obj, shape_id);
RCLASS_SET_FIELDS_HASH(obj, table);
break;
default:
@ -1647,7 +1654,7 @@ obj_transition_too_complex(VALUE obj, st_table *table)
fields_tbl->as.complex.table = table;
st_insert(gen_ivs, (st_data_t)obj, (st_data_t)fields_tbl);
rb_obj_set_shape_id(obj, shape_id);
RBASIC_SET_SHAPE_ID(obj, shape_id);
}
}
@ -1776,8 +1783,9 @@ general_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val, void *data,
}
st_table *table = too_complex_table_func(obj, data);
if (RSHAPE_LEN(target_shape_id) > RSHAPE_LEN(current_shape_id)) {
set_shape_id_func(obj, target_shape_id, data);
RBASIC_SET_SHAPE_ID(obj, target_shape_id);
}
st_insert(table, (st_data_t)RSHAPE_EDGE_NAME(target_shape_id), (st_data_t)val);

1
yjit/src/cruby_bindings.inc.rs generated
View File

@ -699,7 +699,6 @@ pub struct rb_shape {
pub capacity: attr_index_t,
pub type_: u8,
pub heap_index: u8,
pub flags: u8,
}
pub type rb_shape_t = rb_shape;
#[repr(C)]

1
zjit/src/cruby_bindings.inc.rs generated
View File

@ -407,7 +407,6 @@ pub struct rb_shape {
pub capacity: attr_index_t,
pub type_: u8,
pub heap_index: u8,
pub flags: u8,
}
pub type rb_shape_t = rb_shape;
#[repr(C)]