Always use atomics to get the shape count

When sharing between threads we need both atomic reads and writes. We
probably didn't need to use this in some cases (where we weren't running
in multi-ractor mode) but I think it's best to be consistent.
This commit is contained in:
John Hawthorn 2025-07-07 16:18:40 -07:00
parent 5dfd86cf3f
commit cfc006d410
4 changed files with 14 additions and 8 deletions

12
shape.c
View file

@ -371,7 +371,7 @@ rb_shape_each_shape_id(each_shape_callback callback, void *data)
{ {
rb_shape_t *start = rb_shape_get_root_shape(); rb_shape_t *start = rb_shape_get_root_shape();
rb_shape_t *cursor = start; rb_shape_t *cursor = start;
rb_shape_t *end = RSHAPE(rb_shape_tree.next_shape_id); rb_shape_t *end = RSHAPE(rb_shapes_count());
while (cursor < end) { while (cursor < end) {
callback((shape_id_t)(cursor - start), data); callback((shape_id_t)(cursor - start), data);
cursor += 1; cursor += 1;
@ -560,7 +560,7 @@ retry:
if (!res) { if (!res) {
// If we're not allowed to create a new variation, of if we're out of shapes // If we're not allowed to create a new variation, of if we're out of shapes
// we return TOO_COMPLEX_SHAPE. // we return TOO_COMPLEX_SHAPE.
if (!new_variations_allowed || rb_shape_tree.next_shape_id > MAX_SHAPE_ID) { if (!new_variations_allowed || rb_shapes_count() > MAX_SHAPE_ID) {
res = NULL; res = NULL;
} }
else { else {
@ -636,7 +636,7 @@ get_next_shape_internal(rb_shape_t *shape, ID id, enum shape_type shape_type, bo
if (!res) { if (!res) {
// If we're not allowed to create a new variation, of if we're out of shapes // If we're not allowed to create a new variation, of if we're out of shapes
// we return TOO_COMPLEX_SHAPE. // we return TOO_COMPLEX_SHAPE.
if (!new_variations_allowed || rb_shape_tree.next_shape_id > MAX_SHAPE_ID) { if (!new_variations_allowed || rb_shapes_count() > MAX_SHAPE_ID) {
res = NULL; res = NULL;
} }
else { else {
@ -1433,7 +1433,7 @@ rb_shape_root_shape(VALUE self)
static VALUE static VALUE
rb_shape_shapes_available(VALUE self) rb_shape_shapes_available(VALUE self)
{ {
return INT2NUM(MAX_SHAPE_ID - (rb_shape_tree.next_shape_id - 1)); return INT2NUM(MAX_SHAPE_ID - (rb_shapes_count() - 1));
} }
static VALUE static VALUE
@ -1441,7 +1441,7 @@ rb_shape_exhaust(int argc, VALUE *argv, VALUE self)
{ {
rb_check_arity(argc, 0, 1); rb_check_arity(argc, 0, 1);
int offset = argc == 1 ? NUM2INT(argv[0]) : 0; int offset = argc == 1 ? NUM2INT(argv[0]) : 0;
rb_shape_tree.next_shape_id = MAX_SHAPE_ID - offset + 1; RUBY_ATOMIC_SET(rb_shape_tree.next_shape_id, MAX_SHAPE_ID - offset + 1);
return Qnil; return Qnil;
} }
@ -1497,7 +1497,7 @@ static VALUE
rb_shape_find_by_id(VALUE mod, VALUE id) rb_shape_find_by_id(VALUE mod, VALUE id)
{ {
shape_id_t shape_id = NUM2UINT(id); shape_id_t shape_id = NUM2UINT(id);
if (shape_id >= rb_shape_tree.next_shape_id) { if (shape_id >= rb_shapes_count()) {
rb_raise(rb_eArgError, "Shape ID %d is out of bounds\n", shape_id); rb_raise(rb_eArgError, "Shape ID %d is out of bounds\n", shape_id);
} }
return shape_id_t_to_rb_cShape(shape_id); return shape_id_t_to_rb_cShape(shape_id);

View file

@ -122,6 +122,12 @@ RUBY_SYMBOL_EXPORT_BEGIN
RUBY_EXTERN rb_shape_tree_t rb_shape_tree; RUBY_EXTERN rb_shape_tree_t rb_shape_tree;
RUBY_SYMBOL_EXPORT_END RUBY_SYMBOL_EXPORT_END
static inline shape_id_t
rb_shapes_count(void)
{
return (shape_id_t)RUBY_ATOMIC_LOAD(rb_shape_tree.next_shape_id);
}
union rb_attr_index_cache { union rb_attr_index_cache {
uint64_t pack; uint64_t pack;
struct { struct {

2
vm.c
View file

@ -732,7 +732,7 @@ vm_stat(int argc, VALUE *argv, VALUE self)
SET(constant_cache_invalidations, ruby_vm_constant_cache_invalidations); SET(constant_cache_invalidations, ruby_vm_constant_cache_invalidations);
SET(constant_cache_misses, ruby_vm_constant_cache_misses); SET(constant_cache_misses, ruby_vm_constant_cache_misses);
SET(global_cvar_state, ruby_vm_global_cvar_state); SET(global_cvar_state, ruby_vm_global_cvar_state);
SET(next_shape_id, (rb_serial_t)rb_shape_tree.next_shape_id); SET(next_shape_id, (rb_serial_t)rb_shapes_count());
SET(shape_cache_size, (rb_serial_t)rb_shape_tree.cache_size); SET(shape_cache_size, (rb_serial_t)rb_shape_tree.cache_size);
#undef SET #undef SET

2
yjit.c
View file

@ -765,7 +765,7 @@ VALUE
rb_object_shape_count(void) rb_object_shape_count(void)
{ {
// next_shape_id starts from 0, so it's the same as the count // next_shape_id starts from 0, so it's the same as the count
return ULONG2NUM((unsigned long)rb_shape_tree.next_shape_id); return ULONG2NUM((unsigned long)rb_shapes_count());
} }
bool bool