Skip to content

Commit e7fb87e

Browse files
committed
Populate ivar caches for types other than T_OBJECT
`vm_setinstancevariable` had a codepath to try to match the inline cache for types other than T_OBJECT, but the cache population path in `vm_setivar_slowpath` was exclusive to T_OBJECT, so `vm_setivar_default` would never match anything. This commit improves `vm_setivar_slowpath` so that it is capable of filling the cache for all types, and adds a `vm_setivar_class` codepath for `T_CLASS` and `T_MODULE`. `vm_setivar`, `vm_setivar_default` and `vm_setivar_class` could be unified, but based on the very explicit `NOINLINE` I assume they were split to minimize codesize. ``` compare-ruby: ruby 3.5.0dev (2025-08-27T14:58:58Z merge-vm-setivar-d.. 5b749d8e53) +PRISM [arm64-darwin24] built-ruby: ruby 3.5.0dev (2025-08-27T16:30:31Z setivar-cache-gene.. 4fe78ff) +PRISM [arm64-darwin24] | |compare-ruby|built-ruby| |:------------------------|-----------:|---------:| |vm_ivar_set_on_instance | 161.809| 164.688| | | -| 1.02x| |vm_ivar_set_on_generic | 58.769| 115.638| | | -| 1.97x| |vm_ivar_set_on_class | 70.034| 141.042| | | -| 2.01x| ```
1 parent b85b2b8 commit e7fb87e

File tree

5 files changed

+233
-51
lines changed

5 files changed

+233
-51
lines changed

benchmark/vm_ivar_set_on_instance.yml

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,44 @@
11
prelude: |
22
class TheClass
33
def initialize
4+
@levar = 1
45
@v0 = 1
56
@v1 = 2
67
@v3 = 3
8+
end
9+
10+
def set_value_loop
11+
# 100k
12+
i = 0
13+
while i < 100_000
14+
# 10 times to de-emphasize loop overhead
15+
@levar = i
16+
@levar = i
17+
@levar = i
18+
@levar = i
19+
@levar = i
20+
@levar = i
21+
@levar = i
22+
@levar = i
23+
@levar = i
24+
@levar = i
25+
i += 1
26+
end
27+
end
28+
end
29+
30+
class Generic < Time
31+
def initialize
732
@levar = 1
33+
@v0 = 1
34+
@v1 = 2
35+
@v3 = 3
836
end
937
1038
def set_value_loop
11-
# 1M
39+
# 100k
1240
i = 0
13-
while i < 1000000
41+
while i < 100_000
1442
# 10 times to de-emphasize loop overhead
1543
@levar = i
1644
@levar = i
@@ -28,8 +56,39 @@ prelude: |
2856
end
2957
3058
obj = TheClass.new
59+
gen_obj = Generic.new
60+
61+
class SomeClass
62+
@levar = 1
63+
@v0 = 1
64+
@v1 = 2
65+
@v3 = 3
66+
67+
def self.set_value_loop
68+
# 100k
69+
i = 0
70+
while i < 100_000
71+
# 10 times to de-emphasize loop overhead
72+
@levar = i
73+
@levar = i
74+
@levar = i
75+
@levar = i
76+
@levar = i
77+
@levar = i
78+
@levar = i
79+
@levar = i
80+
@levar = i
81+
@levar = i
82+
i += 1
83+
end
84+
end
85+
end
3186
3287
benchmark:
3388
vm_ivar_set_on_instance: |
3489
obj.set_value_loop
90+
vm_ivar_set_on_generic: |
91+
gen_obj.set_value_loop
92+
vm_ivar_set_on_class: |
93+
SomeClass.set_value_loop
3594
loop_count: 100

internal/variable.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ void rb_obj_init_too_complex(VALUE obj, st_table *table);
5151
void rb_evict_ivars_to_hash(VALUE obj);
5252
VALUE rb_obj_field_get(VALUE obj, shape_id_t target_shape_id);
5353
void rb_ivar_set_internal(VALUE obj, ID id, VALUE val);
54-
void rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val);
54+
attr_index_t rb_ivar_set_index(VALUE obj, ID id, VALUE val);
55+
attr_index_t rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val);
5556

5657
RUBY_SYMBOL_EXPORT_BEGIN
5758
/* variable.c (export) */
@@ -67,6 +68,5 @@ VALUE rb_gvar_set(ID, VALUE);
6768
VALUE rb_gvar_defined(ID);
6869
void rb_const_warn_if_deprecated(const rb_const_entry_t *, VALUE, ID);
6970
void rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t newsize);
70-
attr_index_t rb_obj_ivar_set(VALUE obj, ID id, VALUE val);
7171

7272
#endif /* INTERNAL_VARIABLE_H */

test/ruby/test_variable.rb

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,61 @@ def test_special_constant_ivars
388388
end
389389
end
390390

391+
class RemoveIvar
392+
class << self
393+
attr_reader :ivar
394+
395+
def add_ivar
396+
@ivar = 1
397+
end
398+
end
399+
400+
attr_reader :ivar
401+
402+
def add_ivar
403+
@ivar = 1
404+
end
405+
end
406+
407+
def add_and_remove_ivar(obj)
408+
assert_nil obj.ivar
409+
assert_equal 1, obj.add_ivar
410+
assert_equal 1, obj.instance_variable_get(:@ivar)
411+
assert_equal 1, obj.ivar
412+
413+
obj.remove_instance_variable(:@ivar)
414+
assert_nil obj.ivar
415+
416+
assert_raise NameError do
417+
obj.remove_instance_variable(:@ivar)
418+
end
419+
end
420+
421+
def test_remove_instance_variables_object
422+
obj = RemoveIvar.new
423+
add_and_remove_ivar(obj)
424+
add_and_remove_ivar(obj)
425+
end
426+
427+
def test_remove_instance_variables_class
428+
add_and_remove_ivar(RemoveIvar)
429+
add_and_remove_ivar(RemoveIvar)
430+
end
431+
432+
class RemoveIvarGeneric < Array
433+
attr_reader :ivar
434+
435+
def add_ivar
436+
@ivar = 1
437+
end
438+
end
439+
440+
def test_remove_instance_variables_generic
441+
obj = RemoveIvarGeneric.new
442+
add_and_remove_ivar(obj)
443+
add_and_remove_ivar(obj)
444+
end
445+
391446
class ExIvar < Hash
392447
def initialize
393448
@a = 1

variable.c

Lines changed: 51 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,7 +1300,7 @@ rb_free_generic_ivar(VALUE obj)
13001300
}
13011301
}
13021302

1303-
void
1303+
static void
13041304
rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fields_obj)
13051305
{
13061306
ivar_ractor_check(obj, field_name);
@@ -1555,6 +1555,10 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef)
15551555
else {
15561556
val = rb_ivar_delete(fields_obj, id, undef);
15571557
}
1558+
// TODO: What should we set as the T_CLASS shape_id?
1559+
// In most case we can replicate the single `fields_obj` shape
1560+
// but in namespaced case? Perhaps INVALID_SHAPE_ID?
1561+
RBASIC_SET_SHAPE_ID(obj, RBASIC_SHAPE_ID(fields_obj));
15581562
}
15591563
return val;
15601564
}
@@ -1764,7 +1768,7 @@ imemo_fields_set(VALUE owner, VALUE fields_obj, shape_id_t target_shape_id, ID f
17641768
return fields_obj;
17651769
}
17661770

1767-
static void
1771+
static attr_index_t
17681772
generic_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val)
17691773
{
17701774
if (!field_name) {
@@ -1776,6 +1780,7 @@ generic_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE va
17761780
VALUE fields_obj = imemo_fields_set(obj, original_fields_obj, target_shape_id, field_name, val, false);
17771781

17781782
rb_obj_set_fields(obj, fields_obj, field_name, original_fields_obj);
1783+
return rb_shape_too_complex_p(target_shape_id) ? ATTR_INDEX_NOT_SET : RSHAPE_INDEX(target_shape_id);
17791784
}
17801785

17811786
static shape_id_t
@@ -1800,12 +1805,12 @@ generic_shape_ivar(VALUE obj, ID id, bool *new_ivar_out)
18001805
return target_shape_id;
18011806
}
18021807

1803-
static void
1808+
static attr_index_t
18041809
generic_ivar_set(VALUE obj, ID id, VALUE val)
18051810
{
18061811
bool dontcare;
18071812
shape_id_t target_shape_id = generic_shape_ivar(obj, id, &dontcare);
1808-
generic_field_set(obj, target_shape_id, id, val);
1813+
return generic_field_set(obj, target_shape_id, id, val);
18091814
}
18101815

18111816
void
@@ -1886,8 +1891,8 @@ obj_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val)
18861891
}
18871892
}
18881893

1889-
attr_index_t
1890-
rb_obj_ivar_set(VALUE obj, ID id, VALUE val)
1894+
static attr_index_t
1895+
obj_ivar_set(VALUE obj, ID id, VALUE val)
18911896
{
18921897
bool dontcare;
18931898
shape_id_t target_shape_id = generic_shape_ivar(obj, id, &dontcare);
@@ -1902,7 +1907,7 @@ VALUE
19021907
rb_vm_set_ivar_id(VALUE obj, ID id, VALUE val)
19031908
{
19041909
rb_check_frozen(obj);
1905-
rb_obj_ivar_set(obj, id, val);
1910+
obj_ivar_set(obj, id, val);
19061911
return val;
19071912
}
19081913

@@ -1922,26 +1927,25 @@ void rb_obj_freeze_inline(VALUE x)
19221927
}
19231928
}
19241929

1925-
static void
1930+
static attr_index_t class_ivar_set(VALUE obj, ID id, VALUE val, bool *new_ivar);
1931+
1932+
static attr_index_t
19261933
ivar_set(VALUE obj, ID id, VALUE val)
19271934
{
19281935
RB_DEBUG_COUNTER_INC(ivar_set_base);
19291936

19301937
switch (BUILTIN_TYPE(obj)) {
19311938
case T_OBJECT:
1932-
{
1933-
rb_obj_ivar_set(obj, id, val);
1934-
break;
1935-
}
1939+
return obj_ivar_set(obj, id, val);
19361940
case T_CLASS:
19371941
case T_MODULE:
1938-
IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id);
1939-
rb_class_ivar_set(obj, id, val);
1940-
1941-
break;
1942+
{
1943+
IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id);
1944+
bool dontcare;
1945+
return class_ivar_set(obj, id, val, &dontcare);
1946+
}
19421947
default:
1943-
generic_ivar_set(obj, id, val);
1944-
break;
1948+
return generic_ivar_set(obj, id, val);
19451949
}
19461950
}
19471951

@@ -1953,6 +1957,12 @@ rb_ivar_set(VALUE obj, ID id, VALUE val)
19531957
return val;
19541958
}
19551959

1960+
attr_index_t
1961+
rb_ivar_set_index(VALUE obj, ID id, VALUE val)
1962+
{
1963+
return ivar_set(obj, id, val);
1964+
}
1965+
19561966
void
19571967
rb_ivar_set_internal(VALUE obj, ID id, VALUE val)
19581968
{
@@ -1962,21 +1972,19 @@ rb_ivar_set_internal(VALUE obj, ID id, VALUE val)
19621972
ivar_set(obj, id, val);
19631973
}
19641974

1965-
void
1975+
attr_index_t
19661976
rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val)
19671977
{
19681978
switch (BUILTIN_TYPE(obj)) {
19691979
case T_OBJECT:
1970-
obj_field_set(obj, target_shape_id, field_name, val);
1971-
break;
1980+
return obj_field_set(obj, target_shape_id, field_name, val);
19721981
case T_CLASS:
19731982
case T_MODULE:
19741983
// The only field is object_id and T_CLASS handle it differently.
19751984
rb_bug("Unreachable");
19761985
break;
19771986
default:
1978-
generic_field_set(obj, target_shape_id, field_name, val);
1979-
break;
1987+
return generic_field_set(obj, target_shape_id, field_name, val);
19801988
}
19811989
}
19821990

@@ -4483,8 +4491,8 @@ rb_iv_set(VALUE obj, const char *name, VALUE val)
44834491
return rb_ivar_set(obj, id, val);
44844492
}
44854493

4486-
static bool
4487-
class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool concurrent, VALUE *new_fields_obj)
4494+
static attr_index_t
4495+
class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool concurrent, VALUE *new_fields_obj, bool *new_ivar_out)
44884496
{
44894497
const VALUE original_fields_obj = fields_obj;
44904498
fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_fields_new(klass, 1);
@@ -4531,7 +4539,8 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc
45314539
}
45324540

45334541
*new_fields_obj = fields_obj;
4534-
return new_ivar;
4542+
*new_ivar_out = new_ivar;
4543+
return index;
45354544

45364545
too_complex:
45374546
{
@@ -4552,32 +4561,39 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc
45524561
}
45534562

45544563
*new_fields_obj = fields_obj;
4555-
return new_ivar;
4564+
*new_ivar_out = new_ivar;
4565+
return ATTR_INDEX_NOT_SET;
45564566
}
45574567

4558-
bool
4559-
rb_class_ivar_set(VALUE obj, ID id, VALUE val)
4568+
static attr_index_t
4569+
class_ivar_set(VALUE obj, ID id, VALUE val, bool *new_ivar)
45604570
{
4561-
RUBY_ASSERT(RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE));
4562-
rb_check_frozen(obj);
4563-
45644571
rb_class_ensure_writable(obj);
45654572

45664573
const VALUE original_fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
45674574
VALUE new_fields_obj = 0;
45684575

4569-
bool new_ivar = class_fields_ivar_set(obj, original_fields_obj, id, val, rb_multi_ractor_p(), &new_fields_obj);
4576+
attr_index_t index = class_fields_ivar_set(obj, original_fields_obj, id, val, rb_multi_ractor_p(), &new_fields_obj, new_ivar);
45704577

45714578
if (new_fields_obj != original_fields_obj) {
45724579
RCLASS_WRITABLE_SET_FIELDS_OBJ(obj, new_fields_obj);
45734580
}
45744581

45754582
// TODO: What should we set as the T_CLASS shape_id?
45764583
// In most case we can replicate the single `fields_obj` shape
4577-
// but in namespaced case?
4578-
// Perhaps INVALID_SHAPE_ID?
4584+
// but in namespaced case? Perhaps INVALID_SHAPE_ID?
45794585
RBASIC_SET_SHAPE_ID(obj, RBASIC_SHAPE_ID(new_fields_obj));
4586+
return index;
4587+
}
45804588

4589+
bool
4590+
rb_class_ivar_set(VALUE obj, ID id, VALUE val)
4591+
{
4592+
RUBY_ASSERT(RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE));
4593+
rb_check_frozen(obj);
4594+
4595+
bool new_ivar;
4596+
class_ivar_set(obj, id, val, &new_ivar);
45814597
return new_ivar;
45824598
}
45834599

0 commit comments

Comments
 (0)