Enforce consistency between shape_id and FL_EXIVAR

The FL_EXIVAR is a bit redundant with the shape_id.
Now that the `shape_id` is embedded in all objects on all archs,
we can cheaply check if an object has any fields with a simple
bitmask.
This commit is contained in:
Jean Boussier 2025-06-13 15:22:28 +02:00
parent f2d7c6afee
commit b51078f82e
Notes: git 2025-06-13 21:50:44 +00:00
4 changed files with 59 additions and 18 deletions

1
gc.c
View file

@ -2072,7 +2072,6 @@ rb_gc_obj_free_vm_weak_references(VALUE obj)
if (FL_TEST_RAW(obj, FL_EXIVAR)) { if (FL_TEST_RAW(obj, FL_EXIVAR)) {
rb_free_generic_ivar((VALUE)obj); rb_free_generic_ivar((VALUE)obj);
FL_UNSET_RAW(obj, FL_EXIVAR);
} }
switch (BUILTIN_TYPE(obj)) { switch (BUILTIN_TYPE(obj)) {

View file

@ -1266,6 +1266,13 @@ rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id)
} }
} }
if (FL_TEST_RAW(obj, FL_EXIVAR)) {
RUBY_ASSERT(rb_obj_has_exivar(obj));
}
else {
RUBY_ASSERT(!rb_obj_has_exivar(obj));
}
return true; return true;
} }
#endif #endif

31
shape.h
View file

@ -136,8 +136,6 @@ RBASIC_SET_SHAPE_ID(VALUE obj, shape_id_t shape_id)
{ {
RUBY_ASSERT(!RB_SPECIAL_CONST_P(obj)); RUBY_ASSERT(!RB_SPECIAL_CONST_P(obj));
RUBY_ASSERT(!RB_TYPE_P(obj, T_IMEMO) || IMEMO_TYPE_P(obj, imemo_class_fields)); RUBY_ASSERT(!RB_TYPE_P(obj, T_IMEMO) || IMEMO_TYPE_P(obj, imemo_class_fields));
RUBY_ASSERT(rb_shape_verify_consistency(obj, shape_id));
#if RBASIC_SHAPE_ID_FIELD #if RBASIC_SHAPE_ID_FIELD
RBASIC(obj)->shape_id = (VALUE)shape_id; RBASIC(obj)->shape_id = (VALUE)shape_id;
#else #else
@ -145,6 +143,7 @@ RBASIC_SET_SHAPE_ID(VALUE obj, shape_id_t shape_id)
RBASIC(obj)->flags &= SHAPE_FLAG_MASK; RBASIC(obj)->flags &= SHAPE_FLAG_MASK;
RBASIC(obj)->flags |= ((VALUE)(shape_id) << SHAPE_FLAG_SHIFT); RBASIC(obj)->flags |= ((VALUE)(shape_id) << SHAPE_FLAG_SHIFT);
#endif #endif
RUBY_ASSERT(rb_shape_verify_consistency(obj, shape_id));
} }
static inline rb_shape_t * static inline rb_shape_t *
@ -343,6 +342,34 @@ rb_shape_obj_has_ivars(VALUE obj)
return rb_shape_has_ivars(RBASIC_SHAPE_ID(obj)); return rb_shape_has_ivars(RBASIC_SHAPE_ID(obj));
} }
static inline bool
rb_shape_has_fields(shape_id_t shape_id)
{
return shape_id & (SHAPE_ID_OFFSET_MASK | SHAPE_ID_FL_TOO_COMPLEX);
}
static inline bool
rb_shape_obj_has_fields(VALUE obj)
{
return rb_shape_has_fields(RBASIC_SHAPE_ID(obj));
}
static inline bool
rb_obj_has_exivar(VALUE obj)
{
switch (TYPE(obj)) {
case T_NONE:
case T_OBJECT:
case T_CLASS:
case T_MODULE:
case T_IMEMO:
return false;
default:
break;
}
return rb_shape_obj_has_fields(obj);
}
// For ext/objspace // For ext/objspace
RUBY_SYMBOL_EXPORT_BEGIN RUBY_SYMBOL_EXPORT_BEGIN
typedef void each_shape_callback(shape_id_t shape_id, void *data); typedef void each_shape_callback(shape_id_t shape_id, void *data);

View file

@ -1270,6 +1270,7 @@ rb_free_generic_ivar(VALUE obj)
xfree(fields_tbl); xfree(fields_tbl);
} }
} }
FL_UNSET_RAW(obj, FL_EXIVAR);
} }
size_t size_t
@ -1542,23 +1543,30 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef)
RUBY_ASSERT(removed_shape_id != INVALID_SHAPE_ID); RUBY_ASSERT(removed_shape_id != INVALID_SHAPE_ID);
attr_index_t new_fields_count = RSHAPE_LEN(next_shape_id);
attr_index_t removed_index = RSHAPE_INDEX(removed_shape_id); attr_index_t removed_index = RSHAPE_INDEX(removed_shape_id);
val = fields[removed_index]; val = fields[removed_index];
size_t trailing_fields = new_fields_count - removed_index;
MEMMOVE(&fields[removed_index], &fields[removed_index + 1], VALUE, trailing_fields); attr_index_t new_fields_count = RSHAPE_LEN(next_shape_id);
if (new_fields_count) {
size_t trailing_fields = new_fields_count - removed_index;
if (RB_TYPE_P(obj, T_OBJECT) && MEMMOVE(&fields[removed_index], &fields[removed_index + 1], VALUE, trailing_fields);
!RB_FL_TEST_RAW(obj, ROBJECT_EMBED) &&
rb_obj_embedded_size(new_fields_count) <= rb_gc_obj_slot_size(obj)) { if (RB_TYPE_P(obj, T_OBJECT) &&
// Re-embed objects when instances become small enough !RB_FL_TEST_RAW(obj, ROBJECT_EMBED) &&
// This is necessary because YJIT assumes that objects with the same shape rb_obj_embedded_size(new_fields_count) <= rb_gc_obj_slot_size(obj)) {
// have the same embeddedness for efficiency (avoid extra checks) // Re-embed objects when instances become small enough
RB_FL_SET_RAW(obj, ROBJECT_EMBED); // This is necessary because YJIT assumes that objects with the same shape
MEMCPY(ROBJECT_FIELDS(obj), fields, VALUE, new_fields_count); // have the same embeddedness for efficiency (avoid extra checks)
xfree(fields); RB_FL_SET_RAW(obj, ROBJECT_EMBED);
MEMCPY(ROBJECT_FIELDS(obj), fields, VALUE, new_fields_count);
xfree(fields);
}
}
else {
if (FL_TEST_RAW(obj, FL_EXIVAR)) {
rb_free_generic_ivar(obj);
}
} }
rb_obj_set_shape_id(obj, next_shape_id); rb_obj_set_shape_id(obj, next_shape_id);
@ -1881,8 +1889,8 @@ generic_ivar_set_set_shape_id(VALUE obj, shape_id_t shape_id, void *data)
static shape_id_t static shape_id_t
generic_ivar_set_transition_too_complex(VALUE obj, void *_data) generic_ivar_set_transition_too_complex(VALUE obj, void *_data)
{ {
shape_id_t new_shape_id = rb_evict_fields_to_hash(obj);
FL_SET_RAW(obj, FL_EXIVAR); FL_SET_RAW(obj, FL_EXIVAR);
shape_id_t new_shape_id = rb_evict_fields_to_hash(obj);
return new_shape_id; return new_shape_id;
} }
@ -2407,9 +2415,9 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj)
clear: clear:
if (FL_TEST(dest, FL_EXIVAR)) { if (FL_TEST(dest, FL_EXIVAR)) {
RBASIC_SET_SHAPE_ID(dest, ROOT_SHAPE_ID);
rb_free_generic_ivar(dest); rb_free_generic_ivar(dest);
FL_UNSET(dest, FL_EXIVAR); FL_UNSET(dest, FL_EXIVAR);
RBASIC_SET_SHAPE_ID(dest, ROOT_SHAPE_ID);
} }
} }