Skip to content

Commit b51078f

Browse files
committed
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.
1 parent f2d7c6a commit b51078f

File tree

4 files changed

+60
-19
lines changed

4 files changed

+60
-19
lines changed

gc.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2072,7 +2072,6 @@ rb_gc_obj_free_vm_weak_references(VALUE obj)
20722072

20732073
if (FL_TEST_RAW(obj, FL_EXIVAR)) {
20742074
rb_free_generic_ivar((VALUE)obj);
2075-
FL_UNSET_RAW(obj, FL_EXIVAR);
20762075
}
20772076

20782077
switch (BUILTIN_TYPE(obj)) {

shape.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,6 +1266,13 @@ rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id)
12661266
}
12671267
}
12681268

1269+
if (FL_TEST_RAW(obj, FL_EXIVAR)) {
1270+
RUBY_ASSERT(rb_obj_has_exivar(obj));
1271+
}
1272+
else {
1273+
RUBY_ASSERT(!rb_obj_has_exivar(obj));
1274+
}
1275+
12691276
return true;
12701277
}
12711278
#endif

shape.h

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,14 @@ RBASIC_SET_SHAPE_ID(VALUE obj, shape_id_t shape_id)
136136
{
137137
RUBY_ASSERT(!RB_SPECIAL_CONST_P(obj));
138138
RUBY_ASSERT(!RB_TYPE_P(obj, T_IMEMO) || IMEMO_TYPE_P(obj, imemo_class_fields));
139-
RUBY_ASSERT(rb_shape_verify_consistency(obj, shape_id));
140-
141139
#if RBASIC_SHAPE_ID_FIELD
142140
RBASIC(obj)->shape_id = (VALUE)shape_id;
143141
#else
144142
// Object shapes are occupying top bits
145143
RBASIC(obj)->flags &= SHAPE_FLAG_MASK;
146144
RBASIC(obj)->flags |= ((VALUE)(shape_id) << SHAPE_FLAG_SHIFT);
147145
#endif
146+
RUBY_ASSERT(rb_shape_verify_consistency(obj, shape_id));
148147
}
149148

150149
static inline rb_shape_t *
@@ -343,6 +342,34 @@ rb_shape_obj_has_ivars(VALUE obj)
343342
return rb_shape_has_ivars(RBASIC_SHAPE_ID(obj));
344343
}
345344

345+
static inline bool
346+
rb_shape_has_fields(shape_id_t shape_id)
347+
{
348+
return shape_id & (SHAPE_ID_OFFSET_MASK | SHAPE_ID_FL_TOO_COMPLEX);
349+
}
350+
351+
static inline bool
352+
rb_shape_obj_has_fields(VALUE obj)
353+
{
354+
return rb_shape_has_fields(RBASIC_SHAPE_ID(obj));
355+
}
356+
357+
static inline bool
358+
rb_obj_has_exivar(VALUE obj)
359+
{
360+
switch (TYPE(obj)) {
361+
case T_NONE:
362+
case T_OBJECT:
363+
case T_CLASS:
364+
case T_MODULE:
365+
case T_IMEMO:
366+
return false;
367+
default:
368+
break;
369+
}
370+
return rb_shape_obj_has_fields(obj);
371+
}
372+
346373
// For ext/objspace
347374
RUBY_SYMBOL_EXPORT_BEGIN
348375
typedef void each_shape_callback(shape_id_t shape_id, void *data);

variable.c

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,6 +1270,7 @@ rb_free_generic_ivar(VALUE obj)
12701270
xfree(fields_tbl);
12711271
}
12721272
}
1273+
FL_UNSET_RAW(obj, FL_EXIVAR);
12731274
}
12741275

12751276
size_t
@@ -1542,23 +1543,30 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef)
15421543

15431544
RUBY_ASSERT(removed_shape_id != INVALID_SHAPE_ID);
15441545

1545-
attr_index_t new_fields_count = RSHAPE_LEN(next_shape_id);
1546-
15471546
attr_index_t removed_index = RSHAPE_INDEX(removed_shape_id);
15481547
val = fields[removed_index];
1549-
size_t trailing_fields = new_fields_count - removed_index;
15501548

1551-
MEMMOVE(&fields[removed_index], &fields[removed_index + 1], VALUE, trailing_fields);
1552-
1553-
if (RB_TYPE_P(obj, T_OBJECT) &&
1554-
!RB_FL_TEST_RAW(obj, ROBJECT_EMBED) &&
1555-
rb_obj_embedded_size(new_fields_count) <= rb_gc_obj_slot_size(obj)) {
1556-
// Re-embed objects when instances become small enough
1557-
// This is necessary because YJIT assumes that objects with the same shape
1558-
// have the same embeddedness for efficiency (avoid extra checks)
1559-
RB_FL_SET_RAW(obj, ROBJECT_EMBED);
1560-
MEMCPY(ROBJECT_FIELDS(obj), fields, VALUE, new_fields_count);
1561-
xfree(fields);
1549+
attr_index_t new_fields_count = RSHAPE_LEN(next_shape_id);
1550+
if (new_fields_count) {
1551+
size_t trailing_fields = new_fields_count - removed_index;
1552+
1553+
MEMMOVE(&fields[removed_index], &fields[removed_index + 1], VALUE, trailing_fields);
1554+
1555+
if (RB_TYPE_P(obj, T_OBJECT) &&
1556+
!RB_FL_TEST_RAW(obj, ROBJECT_EMBED) &&
1557+
rb_obj_embedded_size(new_fields_count) <= rb_gc_obj_slot_size(obj)) {
1558+
// Re-embed objects when instances become small enough
1559+
// This is necessary because YJIT assumes that objects with the same shape
1560+
// have the same embeddedness for efficiency (avoid extra checks)
1561+
RB_FL_SET_RAW(obj, ROBJECT_EMBED);
1562+
MEMCPY(ROBJECT_FIELDS(obj), fields, VALUE, new_fields_count);
1563+
xfree(fields);
1564+
}
1565+
}
1566+
else {
1567+
if (FL_TEST_RAW(obj, FL_EXIVAR)) {
1568+
rb_free_generic_ivar(obj);
1569+
}
15621570
}
15631571
rb_obj_set_shape_id(obj, next_shape_id);
15641572

@@ -1881,8 +1889,8 @@ generic_ivar_set_set_shape_id(VALUE obj, shape_id_t shape_id, void *data)
18811889
static shape_id_t
18821890
generic_ivar_set_transition_too_complex(VALUE obj, void *_data)
18831891
{
1884-
shape_id_t new_shape_id = rb_evict_fields_to_hash(obj);
18851892
FL_SET_RAW(obj, FL_EXIVAR);
1893+
shape_id_t new_shape_id = rb_evict_fields_to_hash(obj);
18861894
return new_shape_id;
18871895
}
18881896

@@ -2407,9 +2415,9 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj)
24072415

24082416
clear:
24092417
if (FL_TEST(dest, FL_EXIVAR)) {
2410-
RBASIC_SET_SHAPE_ID(dest, ROOT_SHAPE_ID);
24112418
rb_free_generic_ivar(dest);
24122419
FL_UNSET(dest, FL_EXIVAR);
2420+
RBASIC_SET_SHAPE_ID(dest, ROOT_SHAPE_ID);
24132421
}
24142422
}
24152423

0 commit comments

Comments
 (0)