8000 Make setting and accessing class ivars lock-free · ruby/ruby@32b8aeb · GitHub
[go: up one dir, main page]

Skip to content

Commit 32b8aeb

Browse files
committed
Make setting and accessing class ivars lock-free
Now that class fields have been deletated to a T_IMEMO/class_fields when we're in multi-ractor mode, we can read and write class instance variable in an atomic way using Read-Copy-Update (RCU). Note when in multi-ractor mode, we always use RCU. In theory we don't need to, instead if we ensured the field is written before the shape is updated it would be safe. Benchmark: ```ruby Warning[:experimental] = false class Foo @foo = 1 @bar = 2 @baz = 3 @Egg = 4 @spam = 5 class << self attr_reader :foo, :bar, :baz, :egg, :spam end end ractors = 8.times.map do Ractor.new do 1_000_000.times do Foo.bar + Foo.baz * Foo.egg - Foo.spam end end end if Ractor.method_defined?(:value) ractors.each(&:value) else ractors.each(&:take) end ``` This branch vs Ruby 3.4: ```bash $ hyperfine -w 1 'ruby --disable-all ../test.rb' './miniruby ../test.rb' Benchmark 1: ruby --disable-all ../test.rb Time (mean ± σ): 3.162 s ± 0.071 s [User: 2.783 s, System: 10.809 s] Range (min … max): 3.093 s … 3.337 s 10 runs Benchmark 2: ./miniruby ../test.rb Time (mean ± σ): 208.7 ms ± 4.6 ms [User: 889.7 ms, System: 6.9 ms] Range (min … max): 202.8 ms … 222.0 ms 14 runs Summary ./miniruby ../test.rb ran 15.15 ± 0.47 times faster than ruby --disable-all ../test.rb ```
1 parent 8120971 commit 32b8aeb

File tree

4 files changed

+89
-77
lines changed

4 files changed

+89
-77
lines changed

imemo.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,13 @@ rb_imemo_class_fields_clone(VALUE fields_obj)
155155

156156
if (rb_shape_too_complex_p(shape_id)) {
157157
clone = rb_imemo_class_fields_new_complex(CLASS_OF(fields_obj), 0);
158+
RBASIC_SET_SHAPE_ID(clone, shape_id);
158159
st_table *src_table = rb_imemo_class_fields_complex_tbl(fields_obj);
159160
st_replace(rb_imemo_class_fields_complex_tbl(clone), src_table);
160161
}
161162
else {
162163
clone = imemo_class_fields_new(CLASS_OF(fields_obj), RSHAPE_CAPACITY(shape_id));
164+
RBASIC_SET_SHAPE_ID(clone, shape_id);
163165
MEMCPY(rb_imemo_class_fields_ptr(clone), rb_imemo_class_fields_ptr(fields_obj), VALUE, RSHAPE_LEN(shape_id));
164166
}
165167

internal/class.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,10 @@ static inline void
553553
RCLASSEXT_SET_FIELDS_OBJ(VALUE obj, rb_classext_t *ext, VALUE fields_obj)
554554
{
555555
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
556-
RB_OBJ_WRITE(obj, &ext->fields_obj, fields_obj);
556+
557+
VALUE old_fields_obj = ext->fields_obj;
558+
RUBY_ATOMIC_VALUE_SET(ext->fields_obj, fields_obj);
559+
RB_OBJ_WRITTEN(obj, old_fields_obj, fields_obj);
557560
}
558561

559562
static inline void

test/ruby/test_ractor.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,26 @@ def test_default_thread_group
7979
end;
8080
end
8181

82+
def test_class_instance_variables
83+
assert_ractor(<<~'RUBY')
84+
# Once we're in multi-ractor mode, the codepaths
85+
# for class instance variables are a bit different.
86+
Ractor.new {}.value
87+
88+
class TestClass
89+
@a = 1
90+
@b = 2
91+
@c = 3
92+
@d = 4
93+
end
94+
95+
assert_equal 4, TestClass.remove_instance_variable(:@d)
96+
assert_nil TestClass.instance_variable_get(:@d)
97+
assert_equal 4, TestClass.instance_variable_set(:@d, 4)
98+
assert_equal 4, TestClass.instance_variable_get(:@d)
99+
RUBY
100+
end
101+
82102
def test_require_raises_and_no_ractor_belonging_issue
83103
assert_ractor(<<~'RUBY')
84104
require "tempfile"

variable.c

Lines changed: 63 additions & 76 deletions
741A
Original file line numberDiff line numberDiff line change
@@ -1372,43 +1372,38 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef)
13721372
if (SPECIAL_CONST_P(obj)) return undef;
13731373

13741374
if (BUILTIN_TYPE(obj) == T_CLASS || BUILTIN_TYPE(obj) == T_MODULE) {
1375-
VALUE val = undef;
1376-
RB_VM_LOCK_ENTER();
1377-
{
1378-
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
1379-
if (fields_obj) {
1380-
val = rb_ivar_lookup(fields_obj, id, undef);
1381-
}
1382-
}
1383-
RB_VM_LOCK_LEAVE();
1384-
1385-
if (val != undef &&
1386-
rb_is_instance_id(id) &&
1387-
UNLIKELY(!rb_ractor_main_p()) &&
1388-
!rb_ractor_shareable_p(val)) {
1389-
rb_raise(rb_eRactorIsolationError,
1390-
"can not get unshareable values from instance variables of classes/modules from non-main Ractors");
1391-
}
1392-
return val;
13931375
}
13941376

13951377
shape_id_t shape_id;
1396-
VALUE * ivar_list;
1397-
shape_id = RBASIC_SHAPE_ID(obj);
1378+
VALUE *ivar_list;
13981379

13991380
switch (BUILTIN_TYPE(obj)) {
14001381
case T_CLASS:
14011382
case T_MODULE:
14021383
{
1403-
rb_bug("Unreachable");
1384+
VALUE val = undef;
1385+
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
1386+
if (fields_obj) {
1387+
val = rb_ivar_lookup(fields_obj, id, undef);
1388+
}
1389+
1390+
if (val != undef &&
1391+
rb_is_instance_id(id) &&
1392+
UNLIKELY(!rb_ractor_main_p()) &&
1393+
!rb_ractor_shareable_p(val)) {
1394+
rb_raise(rb_eRactorIsolationError,
1395+
"can not get unshareable values from instance variables of classes/modules from non-main Ractors");
1396+
}
1397+
return val;
14041398
}
14051399
case T_IMEMO:
14061400
// Handled like T_OBJECT
14071401
{
14081402
RUBY_ASSERT(IMEMO_TYPE_P(obj, imemo_class_fields));
1403+
shape_id = RBASIC_SHAPE_ID(obj);
14091404

14101405
if (rb_shape_too_complex_p(shape_id)) {
1411-
st_table * iv_table = rb_imemo_class_fields_complex_tbl(obj);
1406+
st_table *iv_table = rb_imemo_class_fields_complex_tbl(obj);
14121407
VALUE val;
14131408
if (rb_st_lookup(iv_table, (st_data_t)id, (st_data_t *)&val)) {
14141409
return val;
@@ -1424,8 +1419,9 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef)
14241419
}
14251420
case T_OBJECT:
14261421
{
1422+
shape_id = RBASIC_SHAPE_ID(obj);
14271423
if (rb_shape_too_complex_p(shape_id)) {
1428-
st_table * iv_table = ROBJECT_FIELDS_HASH(obj);
1424+
st_table *iv_table = ROBJECT_FIELDS_HASH(obj);
14291425
VALUE val;
14301426
if (rb_st_lookup(iv_table, (st_data_t)id, (st_data_t *)&val)) {
14311427
return val;
@@ -1440,6 +1436,7 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef)
14401436
break;
14411437
}
14421438
default:
1439+
shape_id = RBASIC_SHAPE_ID(obj);
14431440
if (FL_TEST_RAW(obj, FL_EXIVAR)) {
14441441
struct gen_fields_tbl *fields_tbl;
14451442
rb_gen_fields_tbl_get(obj, id, &fields_tbl);
@@ -1494,13 +1491,16 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef)
14941491

14951492
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
14961493
if (fields_obj) {
1497-
RB_VM_LOCK_ENTER();
1498-
{
1494+
if (rb_multi_ractor_p()) {
1495+
fields_obj = rb_imemo_class_fields_clone(fields_obj);
1496+
val = rb_ivar_delete(fields_obj, id, undef);
1497+
RCLASS_SET_FIELDS_OBJ(obj, fields_obj);
1498+
}
1499+
else {
14991500
val = rb_ivar_delete(fields_obj, id, undef);
15001501
}
1501-
RB_VM_LOCK_LEAVE();
1502-
return val;
15031502
}
1503+
return val;
15041504
}
15051505

15061506
shape_id_t old_shape_id = rb_obj_shape_id(obj);
@@ -2128,8 +2128,6 @@ rb_ivar_set_internal(VALUE obj, ID id, VALUE val)
21282128
ivar_set(obj, id, val);
21292129
}
21302130

2131-
static void class_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val);
2132-
21332131
void
21342132
rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
21352133
{
@@ -2139,8 +2137,8 @@ rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
21392137
break;
21402138
case T_CLASS:
21412139
case T_MODULE:
2142-
ASSERT_vm_locking();
2143-
class_field_set(obj, target_shape_id, val);
2140+
// The only field is object_id and T_CLASS handle it differently.
2141+
rb_bug("Unreachable");
21442142
break;
21452143
default:
21462144
generic_field_set(obj, target_shape_id, val);
@@ -2200,7 +2198,7 @@ rb_ivar_defined(VALUE obj, ID id)
22002198
switch (BUILTIN_TYPE(obj)) {
22012199
case T_CLASS:
22022200
case T_MODULE:
2203-
RB_VM_LOCKING() {
2201+
{
22042202
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
22052203
if (fields_obj) {
22062204
defined = ivar_defined0(fields_obj, id);
@@ -2454,11 +2452,9 @@ rb_field_foreach(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg,
24542452
case T_CLASS:
24552453
case T_MODULE:
24562454
IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(0);
2457-
RB_VM_LOCKING() {
2458-
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
2459-
if (fields_obj) {
2460-
class_fields_each(fields_obj, func, arg, ivar_only);
2461-
}
2455+
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
2456+
if (fields_obj) {
2457+
class_fields_each(fields_obj, func, arg, ivar_only);
24622458
}
24632459
break;
24642460
default:
@@ -4702,15 +4698,16 @@ rb_iv_set(VALUE obj, const char *name, VALUE val)
47024698
return rb_ivar_set(obj, id, val);
47034699
}
47044700

4705-
static int
4706-
class_ivar_set(VALUE obj, ID id, VALUE val)
4701+
static bool
4702+
class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool concurrent, VALUE *new_fields_obj)
47074703
{
47084704
bool existing = true;
4709-
const VALUE original_fields_obj = RCLASS_FIELDS_OBJ(obj);
4710-
VALUE fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_class_fields_new(obj, 1);
4705+
const VALUE original_fields_obj = fields_obj;
4706+
fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_class_fields_new(klass, 1);
47114707

4712-
shape_id_t next_shape_id = 0;
47134708
shape_id_t current_shape_id = RBASIC_SHAPE_ID(fields_obj);
4709+
shape_id_t next_shape_id = current_shape_id;
4710+
47144711
if (UNLIKELY(rb_shape_too_complex_p(current_shape_id))) {
47154712
goto too_complex;
47164713
}
@@ -4727,7 +4724,7 @@ class_ivar_set(VALUE obj, ID id, VALUE val)
47274724
next_shape_id = rb_shape_transition_add_ivar(fields_obj, id);
47284725
if (UNLIKELY(rb_shape_too_complex_p(next_shape_id))) {
47294726
attr_index_t current_len = RSHAPE_LEN(current_shape_id);
4730-
fields_obj = rb_imemo_class_fields_new_complex(obj, current_len + 1);
4727+
fields_obj = rb_imemo_class_fields_new_complex(klass, current_len + 1);
47314728
if (current_len) {
47324729
rb_obj_copy_fields_to_hash_table(original_fields_obj, rb_imemo_class_fields_complex_tbl(fields_obj));
47334730
RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id);
@@ -4738,10 +4735,12 @@ class_ivar_set(VALUE obj, ID id, VALUE val)
47384735
attr_index_t next_capacity = RSHAPE_CAPACITY(next_shape_id);
47394736
attr_index_t current_capacity = RSHAPE_CAPACITY(current_shape_id);
47404737

4741-
if (UNLIKELY(next_capacity != current_capacity)) {
4742-
RUBY_ASSERT(next_capacity > current_capacity);
4743-
// We allocate a new fields_obj so that we're embedded as long as possible
4744-
fields_obj = rb_imemo_class_fields_new(obj, next_capacity);
4738+
if (concurrent || next_capacity != current_capacity) {
4739+
RUBY_ASSERT(concurrent || next_capacity > current_capacity);
4740+
4741+
// We allocate a new fields_obj even when concurrency isn't a concern
4742+
// so that we're embedded as long as possible.
4743+
fields_obj = rb_imemo_class_fields_new(klass, next_capacity);
47454744
if (original_fields_obj) {
47464745
MEMCPY(rb_imemo_class_fields_ptr(fields_obj), rb_imemo_class_fields_ptr(original_fields_obj), VALUE, RSHAPE_LEN(current_shape_id));
47474746
}
@@ -4753,20 +4752,12 @@ class_ivar_set(VALUE obj, ID id, VALUE val)
47534752

47544753
VALUE *fields = rb_imemo_class_fields_ptr(fields_obj);
47554754
RB_OBJ_WRITE(fields_obj, &fields[index], val);
4755+
47564756
if (!existing) {
47574757
RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id);
47584758
}
47594759

4760-
if (fields_obj != original_fields_obj) {
4761-
RCLASS_SET_FIELDS_OBJ(obj, fields_obj);
4762-
// TODO: What should we set as the T_CLASS shape_id?
4763-
// In most case we can replicate the single `fields_obj` shape
4764-
// but in namespaced case?
4765-
// Perhaps INVALID_SHAPE_ID?
4766-
RBASIC_SET_SHAPE_ID(obj, next_shape_id);
4767-
}
4768-
4769-
RB_GC_GUARD(fields_obj);
4760+
*new_fields_obj = fields_obj;
47704761
return existing;
47714762

47724763
too_complex:
@@ -4777,41 +4768,37 @@ class_ivar_set(VALUE obj, ID id, VALUE val)
47774768

47784769
if (fields_obj != original_fields_obj) {
47794770
RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id);
4780-
RCLASS_SET_FIELDS_OBJ(obj, fields_obj);
4781-
// TODO: What should we set as the T_CLASS shape_id?
4782-
// In most case we can replicate the single `fields_obj` shape
4783-
// but in namespaced case?
4784-
// Perhaps INVALID_SHAPE_ID?
4785-
RBASIC_SET_SHAPE_ID(obj, next_shape_id);
47864771
}
47874772
}
4788-
RB_GC_GUARD(fields_obj);
4773+
4774+
*new_fields_obj = fields_obj;
47894775
return existing;
47904776
}
47914777

47924778
int
47934779
rb_class_ivar_set(VALUE obj, ID id, VALUE val)
47944780
{
47954781
RUBY_ASSERT(RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE));
4796-
bool existing = false;
47974782
rb_check_frozen(obj);
47984783

47994784
rb_class_ensure_writable(obj);
48004785

4801-
RB_VM_LOCKING() {
4802-
existing = class_ivar_set(obj, id, val);
4803-
}
4786+
const VALUE original_fields_obj = RCLASS_FIELDS_OBJ(obj);
4787+
VALUE new_fields_obj = 0;
48044788

4805-
return existing;
4806-
}
4789+
bool existing = class_fields_ivar_set(obj, original_fields_obj, id, val, rb_multi_ractor_p(), &new_fields_obj);
48074790

4808-
static void
4809-
class_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
4810-
{
4811-
RUBY_ASSERT(RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE));
4812-
obj_field_set(RCLASS_ENSURE_FIELDS_OBJ(obj), target_shape_id, val);
4813-
}
4791+
if (new_fields_obj != original_fields_obj) {
4792+
RCLASS_SET_FIELDS_OBJ(obj, new_fields_obj);
48144793

4794+
// TODO: What should we set as the T_CLASS shape_id?
4795+
// In most case we can replicate the single `fields_obj` shape
4796+
// but in namespaced case?
4797+
// Perhaps INVALID_SHAPE_ID?
4798+
RBASIC_SET_SHAPE_ID(obj, RBASIC_SHAPE_ID(new_fields_obj));
4799+
}
4800+
return existing;
4801+
}
48154802
static int
48164803
tbl_copy_i(ID key, VALUE val, st_data_t dest)
48174804
{

0 commit comments

Comments
 (0)
0