Skip to content

Commit 504a8f2

Browse files
committed
imemo_fields_set: save copying when reassigning a variable
If we still fit in the existing imemo/fields object we can update it atomically, saving a reallocation.
1 parent d314673 commit 504a8f2

3 files changed

Lines changed: 41 additions & 8 deletions

File tree

internal/class.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,9 +566,7 @@ RCLASSEXT_SET_FIELDS_OBJ(VALUE obj, rb_classext_t *ext, VALUE fields_obj)
566566
{
567567
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
568568

569-
VALUE old_fields_obj = ext->fields_obj;
570-
RUBY_ATOMIC_VALUE_SET(ext->fields_obj, fields_obj);
571-
RB_OBJ_WRITTEN(obj, old_fields_obj, fields_obj);
569+
RB_OBJ_ATOMIC_WRITE(obj, &ext->fields_obj, fields_obj);
572570
}
573571

574572
static inline void

internal/gc.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,26 @@ int rb_gc_modular_gc_loaded_p(void);
264264

265265
RUBY_SYMBOL_EXPORT_END
266266

267+
static inline VALUE
268+
rb_obj_atomic_write(
269+
VALUE a, VALUE *slot, VALUE b,
270+
RBIMPL_ATTR_MAYBE_UNUSED()
271+
const char *filename,
272+
RBIMPL_ATTR_MAYBE_UNUSED()
273+
int line)
274+
{
275+
#ifdef RGENGC_LOGGING_WRITE
276+
RGENGC_LOGGING_WRITE(a, slot, b, filename, line);
277+
#endif
278+
279+
RUBY_ATOMIC_VALUE_SET(*slot, b);
280+
281+
rb_obj_written(a, RUBY_Qundef /* ignore `oldv' now */, b, filename, line);
282+
return a;
283+
}
284+
#define RB_OBJ_ATOMIC_WRITE(old, slot, young) \
285+
RBIMPL_CAST(rb_obj_atomic_write((VALUE)(old), (VALUE *)(slot), (VALUE)(young), __FILE__, __LINE__))
286+
267287
int rb_ec_stack_check(struct rb_execution_context_struct *ec);
268288
void rb_gc_writebarrier_remember(VALUE obj);
269289
const char *rb_obj_info(VALUE obj);

variable.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1844,6 +1844,9 @@ imemo_fields_set(VALUE klass, VALUE fields_obj, shape_id_t target_shape_id, ID f
18441844
if (UNLIKELY(rb_shape_too_complex_p(target_shape_id))) {
18451845
if (rb_shape_too_complex_p(current_shape_id)) {
18461846
if (concurrent) {
1847+
// In multi-ractor case, we must always work on a copy because
1848+
// even if the field already exist, inserting in a st_table may
1849+
// cause a rebuild.
18471850
fields_obj = rb_imemo_fields_clone(fields_obj);
18481851
}
18491852
}
@@ -4680,9 +4683,7 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc
46804683
attr_index_t next_capacity = RSHAPE_CAPACITY(next_shape_id);
46814684
attr_index_t current_capacity = RSHAPE_CAPACITY(current_shape_id);
46824685

4683-
if (concurrent || next_capacity != current_capacity) {
4684-
RUBY_ASSERT(concurrent || next_capacity > current_capacity);
4685-
4686+
if (next_capacity > current_capacity) {
46864687
// We allocate a new fields_obj even when concurrency isn't a concern
46874688
// so that we're embedded as long as possible.
46884689
fields_obj = imemo_fields_copy_capa(rb_singleton_class(klass), fields_obj, next_capacity);
@@ -4693,7 +4694,18 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc
46934694
}
46944695

46954696
VALUE *fields = rb_imemo_fields_ptr(fields_obj);
4696-
RB_OBJ_WRITE(fields_obj, &fields[index], val);
4697+
4698+
if (concurrent && original_fields_obj == fields_obj) {
4699+
// In the concurrent case, if we're mutating the existing
4700+
// fields_obj, we must use an atomic write, because if we're
4701+
// adding a new field, the shape_id must be written after the field
4702+
// and if we're updating an existing field, we at least need a relaxed
4703+
// write to avoid reaping.
4704+
RB_OBJ_ATOMIC_WRITE(fields_obj, &fields[index], val);
4705+
}
4706+
else {
4707+
RB_OBJ_WRITE(fields_obj, &fields[index], val);
4708+
}
46974709

46984710
if (!existing) {
46994711
RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id);
@@ -4705,9 +4717,12 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc
47054717
too_complex:
47064718
{
47074719
if (concurrent && fields_obj == original_fields_obj) {
4708-
// If we're in the multi-ractor mode, we can't directly insert in the table.
4720+
// In multi-ractor case, we must always work on a copy because
4721+
// even if the field already exist, inserting in a st_table may
4722+
// cause a rebuild.
47094723
fields_obj = rb_imemo_fields_clone(fields_obj);
47104724
}
4725+
47114726
st_table *table = rb_imemo_fields_complex_tbl(fields_obj);
47124727
existing = st_insert(table, (st_data_t)id, (st_data_t)val);
47134728
RB_OBJ_WRITTEN(fields_obj, Qundef, val);

0 commit comments

Comments
 (0)