8000 Make setting and accessing class ivars lock-free · ruby/ruby@631f026 · GitHub
[go: up one dir, main page]
Skip to content

Commit 631f026

Browse files
byrootbyroot
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 8b5ac5a commit 631f026

File tree

4 files changed

+91
-82
lines changed

4 files changed

+91
-82
lines changed

‎imemo.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ rb_imemo_class_fields_clone(VALUE fields_obj)
156156
if (rb_shape_too_complex_p(shape_id)) {
157157
clone = rb_imemo_class_fields_new_complex(CLASS_OF(fields_obj), 0);
158158
RBASIC_SET_SHAPE_ID(clone, shape_id);
159-
160159
st_table *src_table = rb_imemo_class_fields_complex_tbl(fields_obj);
161160
st_replace(rb_imemo_class_fields_complex_tbl(clone), src_table);
162161
}

‎internal/class.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -531,18 +531,21 @@ RCLASS_WRITABLE_ENSURE_FIELDS_OBJ(VALUE obj)
531531
return ext->fields_obj;
532532
}
533533

534-
static inline void
535-
RCLASSEXT_SET_FIELDS_OBJ(VALUE obj, rb_classext_t *ext, VALUE fields_obj)
534+
static inline VALUE
535+
RCLASS_WRITABLE_FIELDS_OBJ(VALUE obj)
536536
{
537537
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
538-
RB_OBJ_WRITE(obj, &ext->fields_obj, fields_obj);
538+
return RCLASSEXT_FIELDS_OBJ(RCLASS_EXT_WRITABLE(obj));
539539
}
540540

541-
static inline VALUE
542-
RCLASS_WRITABLE_FIELDS_OBJ(VALUE obj)
541+
static inline void
542+
RCLASSEXT_SET_FIELDS_OBJ(VALUE obj, rb_classext_t *ext, VALUE fields_obj)
543543
{
544544
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
545-
return RCLASSEXT_FIELDS_OBJ(RCLASS_EXT_WRITABLE(obj));
545+
546+
VALUE old_fields_obj = ext->fields_obj;
547+
RUBY_ATOMIC_VALUE_SET(ext->fields_obj, fields_obj);
548+
RB_OBJ_WRITTEN(obj, old_fields_obj, fields_obj);
546549
}
547550

548551
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: 62 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,44 +1371,36 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef)
13711371
{
13721372
if (SPECIAL_CONST_P(obj)) return undef;
13731373

1374-
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_WRITABLE_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;
1393-
}
1394-
13951374
shape_id_t shape_id;
1396-
VALUE * ivar_list;
1397-
shape_id = RBASIC_SHAPE_ID(obj);
1375+
VALUE *ivar_list;
13981376

13991377
switch (BUILTIN_TYPE(obj)) {
14001378
case T_CLASS:
14011379
case T_MODULE:
14021380
{
1403-
rb_bug("Unreachable");
1381+
VALUE val = undef;
1382+
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
1383+
if (fields_obj) {
1384+
val = rb_ivar_lookup(fields_obj, id, undef);
1385+
}
1386+
1387+
if (val != undef &&
1388+
rb_is_instance_id(id) &&
1389+
UNLIKELY(!rb_ractor_main_p()) &&
1390+
!rb_ractor_shareable_p(val)) {
1391+
rb_raise(rb_eRactorIsolationError,
1392+
"can not get unshareable values from instance variables of classes/modules from non-main Ractors");
1393+
}
1394+
return val;
14041395
}
14051396
case T_IMEMO:
14061397
// Handled like T_OBJECT
14071398
{
14081399
RUBY_ASSERT(IMEMO_TYPE_P(obj, imemo_class_fields));
1400+
shape_id = RBASIC_SHAPE_ID(obj);
14091401

14101402
if (rb_shape_too_complex_p(shape_id)) {
1411-
st_table * iv_table = rb_imemo_class_fields_complex_tbl(obj);
1403+
st_table *iv_table = rb_imemo_class_fields_complex_tbl(obj);
14121404
VALUE val;
14131405
if (rb_st_lookup(iv_table, (st_data_t)id, (st_data_t *)&val)) {
14141406
return val;
@@ -1424,8 +1416,9 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef)
14241416
}
14251417
case T_OBJECT:
14261418
{
1419+
shape_id = RBASIC_SHAPE_ID(obj);
14271420
if (rb_shape_too_complex_p(shape_id)) {
1428-
st_table * iv_table = ROBJECT_FIELDS_HASH(obj);
1421+
st_table *iv_table = ROBJECT_FIELDS_HASH(obj);
14291422
VALUE val;
14301423
if (rb_st_lookup(iv_table, (st_data_t)id, (st_data_t *)&val)) {
14311424
return val;
@@ -1440,6 +1433,7 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef)
14401433
break;
14411434
}
14421435
default:
1436+
shape_id = RBASIC_SHAPE_ID(obj);
14431437
if (FL_TEST_RAW(obj, FL_EXIVAR)) {
14441438
struct gen_fields_tbl *fields_tbl;
14451439
rb_gen_fields_tbl_get(obj, id, &fields_tbl);
@@ -1494,13 +1488,16 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef)
14941488

14951489
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
14961490
if (fields_obj) {
1497-
RB_VM_LOCK_ENTER();
1498-
{
1491+
if (rb_multi_ractor_p()) {
1492+
fields_obj = rb_imemo_class_fields_clone(fields_obj);
1493+
val = rb_ivar_delete(fields_obj, id, undef);
1494+
RCLASS_WRITABLE_SET_FIELDS_OBJ(obj, fields_obj);
1495+
}
1496+
else {
14991497
val = rb_ivar_delete(fields_obj, id, undef);
15001498
}
1501-
RB_VM_LOCK_LEAVE();
1502-
return val;
15031499
}
1500+
return val;
15041501
}
15051502

15061503
shape_id_t old_shape_id = rb_obj_shape_id(obj);
@@ -2127,8 +2124,6 @@ rb_ivar_set_internal(VALUE obj, ID id, VALUE val)
21272124
ivar_set(obj, id, val);
21282125
}
21292126

2130-
static void class_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val);
2131-
21322127
void
21332128
rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
21342129
{
@@ -2138,8 +2133,8 @@ rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
21382133
break;
21392134
case T_CLASS:
21402135
case T_MODULE:
2141-
ASSERT_vm_locking();
2142-
class_field_set(obj, target_shape_id, val);
2136+
// The only field is object_id and T_CLASS handle it differently.
2137+
rb_bug("Unreachable");
21432138
break;
21442139
default:
21452140
generic_field_set(obj, target_shape_id, val);
@@ -2199,7 +2194,7 @@ rb_ivar_defined(VALUE obj, ID id)
21992194
switch (BUILTIN_TYPE(obj)) {
22002195
case T_CLASS:
22012196
case T_MODULE:
2202-
RB_VM_LOCKING() {
2197+
{
22032198
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
22042199
if (fields_obj) {
22052200
defined = ivar_defined0(fields_obj, id);
@@ -2452,8 +2447,8 @@ rb_field_foreach(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg,
24522447
break;
24532448
case T_CLASS:
24542449
case T_MODULE:
2455-
IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(0);
2456-
RB_VM_LOCKING() {
2450+
{
2451+
IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(0);
24572452
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
24582453
if (fields_obj) {
24592454
class_fields_each(fields_obj, func, arg, ivar_only);
@@ -4701,15 +4696,16 @@ rb_iv_set(VALUE obj, const char *name, VALUE val)
47014696
return rb_ivar_set(obj, id, val);
47024697
}
47034698

4704-
static int
4705-
class_ivar_set(VALUE obj, ID id, VALUE val)
4699+
static bool
4700+
class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool concurrent, VALUE *new_fields_obj)
47064701
{
47074702
bool existing = true;
4708-
const VALUE original_fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
4709-
VALUE fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_class_fields_new(obj, 1);
4703+
const VALUE original_fields_obj = fields_obj;
4704+
fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_class_fields_new(klass, 1);
47104705

4711-
shape_id_t next_shape_id = 0;
47124706
shape_id_t current_shape_id = RBASIC_SHAPE_ID(fields_obj);
4707+
shape_id_t next_shape_id = current_shape_id;
4708+
47134709
if (UNLIKELY(rb_shape_too_complex_p(current_shape_id))) {
47144710
goto too_complex;
47154711
}
@@ -4726,7 +4722,7 @@ class_ivar_set(VALUE obj, ID id, VALUE val)
47264722
next_shape_id = rb_shape_transition_add_ivar(fields_obj, id);
47274723
if (UNLIKELY(rb_shape_too_complex_p(next_shape_id))) {
47284724
attr_index_t current_len = RSHAPE_LEN(current_shape_id);
4729-
fields_obj = rb_imemo_class_fields_new_complex(obj, current_len + 1);
4725+
fields_obj = rb_imemo_class_fields_new_complex(klass, current_len + 1);
47304726
if (current_len) {
47314727
rb_obj_copy_fields_to_hash_table(original_fields_obj, rb_imemo_class_fields_complex_tbl(fields_obj));
47324728
RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id);
@@ -4737,10 +4733,12 @@ class_ivar_set(VALUE obj, ID id, VALUE val)
47374733
attr_index_t next_capacity = RSHAPE_CAPACITY(next_shape_id);
47384734
attr_index_t current_capacity = RSHAPE_CAPACITY(current_shape_id);
47394735

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

47534751
VALUE *fields = rb_imemo_class_fields_ptr(fields_obj);
47544752
RB_OBJ_WRITE(fields_obj, &fields[index], val);
4753+
47554754
if (!existing) {
47564755
RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id);
47574756
}
47584757

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

47714761
too_complex:
@@ -4776,39 +4766,36 @@ class_ivar_set(VALUE obj, ID id, VALUE val)
47764766

47774767
if (fields_obj != original_fields_obj) {
47784768
RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id);
4779-
RCLASS_WRITABLE_SET_FIELDS_OBJ(obj, fields_obj);
4780-
// TODO: What should we set as the T_CLASS shape_id?
4781-
// In most case we can replicate the single `fields_obj` shape
4782-
// but in namespaced case?
4783-
// Perhaps INVALID_SHAPE_ID?
4784-
RBASIC_SET_SHAPE_ID(obj, next_shape_id);
47854769
}
47864770
}
4787-
RB_GC_GUARD(fields_obj);
4771+
4772+
*new_fields_obj = fields_obj;
47884773
return existing;
47894774
}
47904775

47914776
int
47924777
rb_class_ivar_set(VALUE obj, ID id, VALUE val)
47934778
{
47944779
RUBY_ASSERT(RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE));
4795-
bool existing = false;
47964780
rb_check_frozen(obj);
47974781

47984782
rb_class_ensure_writable(obj);
47994783

4800-
RB_VM_LOCKING() {
4801-
existing = class_ivar_set(obj, id, val);
4802-
}
4784+
const VALUE original_fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
4785+
VALUE new_fields_obj = 0;
48034786

4804-
return existing;
4805-
}
4787+
bool existing = class_fields_ivar_set(obj, original_fields_obj, id, val, rb_multi_ractor_p(), &new_fields_obj);
48064788

4807-
static void
4808-
class_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
4809-
{
4810-
RUBY_ASSERT(RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE));
4811-
obj_field_set(RCLASS_WRITABLE_ENSURE_FIELDS_OBJ(obj), target_shape_id, val);
4789+
if (new_fields_obj != original_fields_obj) {
4790+
RCLASS_WRITABLE_SET_FIELDS_OBJ(obj, new_fields_obj);
4791+
4792+
// TODO: What should we set as the T_CLASS shape_id?
4793+
// In most case we can replicate the single `fields_obj` shape
4794+
// but in namespaced case?
4795+
// Perhaps INVALID_SHAPE_ID?
4796+
RBASIC_SET_SHAPE_ID(obj, RBASIC_SHAPE_ID(new_fields_obj));
4797+
}
4798+
return existing;
48124799
}
48134800

48144801
static int

0 commit comments

Comments
 (0)
0