Skip to content

Commit 989bce8

Browse files
byrootbyroot
committed
Turn rb_classext_t.fields into a T_OBJECT
This aims to solve two problems. First, it solves the problem of namspaced classes having a single `shape_id`. Now each namespaced classext has a `T_OBJECT` that can hold the namespace specific shape. Second, it open the door to later make class instance variable writes atomics, hence be able to read class variables without locking the VM. In the future, in multi-ractor mode, we can do the write on a copy of the `fields_obj` and then atomically swap it. Considerations: - T_OBJECT isn't ideal, it probably should be a `T_IMEMO` but this require a very large refatoring. - Right now the `RClass` shape_id is always synchronized, but with namespace we should likely mark classes that have multiple namespace with `INVALID_SHAPE`.
1 parent 1435ea7 commit 989bce8

12 files changed

Lines changed: 172 additions & 262 deletions

File tree

‎class.c‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -317,16 +317,8 @@ rb_class_duplicate_classext(rb_classext_t *orig, VALUE klass, const rb_namespace
317317

318318
RCLASSEXT_M_TBL(ext) = duplicate_classext_m_tbl(RCLASSEXT_M_TBL(orig), klass, dup_iclass);
319319

320-
// TODO: consider shapes for performance
321-
if (RCLASSEXT_FIELDS(orig)) {
322-
RUBY_ASSERT(!RB_TYPE_P(klass, T_ICLASS));
323-
RCLASSEXT_FIELDS(ext) = (VALUE *)st_copy((st_table *)RCLASSEXT_FIELDS(orig));
324-
rb_autoload_copy_table_for_namespace((st_table *)RCLASSEXT_FIELDS(ext), ns);
325-
}
326-
else {
327-
if (!RB_TYPE_P(klass, T_ICLASS)) {
328-
RCLASSEXT_FIELDS(ext) = (VALUE *)st_init_numtable();
329-
}
320+
if (orig->fields_obj) {
321+
ext->fields_obj = rb_obj_clone(orig->fields_obj);
330322
}
331323

332324
if (RCLASSEXT_SHARED_CONST_TBL(orig)) {

‎common.mk‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4135,6 +4135,7 @@ debug.$(OBJEXT): $(top_srcdir)/internal/compilers.h
41354135
debug.$(OBJEXT): $(top_srcdir)/internal/gc.h
41364136
debug.$(OBJEXT): $(top_srcdir)/internal/imemo.h
41374137
debug.$(OBJEXT): $(top_srcdir)/internal/namespace.h
4138+
debug.$(OBJEXT): $(top_srcdir)/internal/object.h
41384139
debug.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
41394140
debug.$(OBJEXT): $(top_srcdir)/internal/serial.h
41404141
debug.$(OBJEXT): $(top_srcdir)/internal/set_table.h
@@ -6450,6 +6451,7 @@ enumerator.$(OBJEXT): $(top_srcdir)/internal/hash.h
64506451
enumerator.$(OBJEXT): $(top_srcdir)/internal/imemo.h
64516452
enumerator.$(OBJEXT): $(top_srcdir)/internal/namespace.h
64526453
enumerator.$(OBJEXT): $(top_srcdir)/internal/numeric.h
6454+
enumerator.$(OBJEXT): $(top_srcdir)/internal/object.h
64536455
enumerator.$(OBJEXT): $(top_srcdir)/internal/range.h
64546456
enumerator.$(OBJEXT): $(top_srcdir)/internal/rational.h
64556457
enumerator.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
@@ -8124,6 +8126,7 @@ imemo.$(OBJEXT): $(top_srcdir)/internal/compilers.h
81248126
imemo.$(OBJEXT): $(top_srcdir)/internal/gc.h
81258127
imemo.$(OBJEXT): $(top_srcdir)/internal/imemo.h
81268128
imemo.$(OBJEXT): $(top_srcdir)/internal/namespace.h
8129+
imemo.$(OBJEXT): $(top_srcdir)/internal/object.h
81278130
imemo.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
81288131
imemo.$(OBJEXT): $(top_srcdir)/internal/serial.h
81298132
imemo.$(OBJEXT): $(top_srcdir)/internal/set_table.h
@@ -8949,6 +8952,7 @@ iseq.$(OBJEXT): $(top_srcdir)/internal/imemo.h
89498952
iseq.$(OBJEXT): $(top_srcdir)/internal/io.h
89508953
iseq.$(OBJEXT): $(top_srcdir)/internal/namespace.h
89518954
iseq.$(OBJEXT): $(top_srcdir)/internal/numeric.h
8955+
iseq.$(OBJEXT): $(top_srcdir)/internal/object.h
89528956
iseq.$(OBJEXT): $(top_srcdir)/internal/parse.h
89538957
iseq.$(OBJEXT): $(top_srcdir)/internal/rational.h
89548958
iseq.$(OBJEXT): $(top_srcdir)/internal/ruby_parser.h
@@ -9197,6 +9201,7 @@ jit.$(OBJEXT): $(top_srcdir)/internal/compilers.h
91979201
jit.$(OBJEXT): $(top_srcdir)/internal/gc.h
91989202
jit.$(OBJEXT): $(top_srcdir)/internal/imemo.h
91999203
jit.$(OBJEXT): $(top_srcdir)/internal/namespace.h
9204+
jit.$(OBJEXT): $(top_srcdir)/internal/object.h
92009205
jit.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
92019206
jit.$(OBJEXT): $(top_srcdir)/internal/serial.h
92029207
jit.$(OBJEXT): $(top_srcdir)/internal/set_table.h
@@ -11054,6 +11059,7 @@ namespace.$(OBJEXT): $(top_srcdir)/internal/hash.h
1105411059
namespace.$(OBJEXT): $(top_srcdir)/internal/imemo.h
1105511060
namespace.$(OBJEXT): $(top_srcdir)/internal/load.h
1105611061
namespace.$(OBJEXT): $(top_srcdir)/internal/namespace.h
11062+
namespace.$(OBJEXT): $(top_srcdir)/internal/object.h
1105711063
namespace.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
1105811064
namespace.$(OBJEXT): $(top_srcdir)/internal/serial.h
1105911065
namespace.$(OBJEXT): $(top_srcdir)/internal/set_table.h
@@ -11473,6 +11479,7 @@ node_dump.$(OBJEXT): $(top_srcdir)/internal/hash.h
1147311479
node_dump.$(OBJEXT): $(top_srcdir)/internal/imemo.h
1147411480
node_dump.$(OBJEXT): $(top_srcdir)/internal/namespace.h
1147511481
node_dump.$(OBJEXT): $(top_srcdir)/internal/numeric.h
11482+
node_dump.$(OBJEXT): $(top_srcdir)/internal/object.h
1147611483
node_dump.$(OBJEXT): $(top_srcdir)/internal/parse.h
1147711484
node_dump.$(OBJEXT): $(top_srcdir)/internal/rational.h
1147811485
node_dump.$(OBJEXT): $(top_srcdir)/internal/ruby_parser.h
@@ -20381,6 +20388,7 @@ vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/error.h
2038120388
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/gc.h
2038220389
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/imemo.h
2038320390
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/namespace.h
20391+
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/object.h
2038420392
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
2038520393
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/serial.h
2038620394
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/set_table.h
@@ -21059,6 +21067,7 @@ vm_trace.$(OBJEXT): $(top_srcdir)/internal/gc.h
2105921067
vm_trace.$(OBJEXT): $(top_srcdir)/internal/hash.h
2106021068
vm_trace.$(OBJEXT): $(top_srcdir)/internal/imemo.h
2106121069
vm_trace.$(OBJEXT): $(top_srcdir)/internal/namespace.h
21070+
vm_trace.$(OBJEXT): $(top_srcdir)/internal/object.h
2106221071
vm_trace.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
2106321072
vm_trace.$(OBJEXT): $(top_srcdir)/internal/serial.h
2106421073
vm_trace.$(OBJEXT): $(top_srcdir)/internal/set_table.h
@@ -21506,6 +21515,7 @@ yjit.$(OBJEXT): $(top_srcdir)/internal/hash.h
2150621515
yjit.$(OBJEXT): $(top_srcdir)/internal/imemo.h
2150721516
yjit.$(OBJEXT): $(top_srcdir)/internal/namespace.h
2150821517
yjit.$(OBJEXT): $(top_srcdir)/internal/numeric.h
21518+
yjit.$(OBJEXT): $(top_srcdir)/internal/object.h
2150921519
yjit.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
2151021520
yjit.$(OBJEXT): $(top_srcdir)/internal/serial.h
2151121521
yjit.$(OBJEXT): $(top_srcdir)/internal/set_table.h

‎ext/objspace/depend‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ objspace.o: $(top_srcdir)/internal/gc.h
395395
objspace.o: $(top_srcdir)/internal/hash.h
396396
objspace.o: $(top_srcdir)/internal/imemo.h
397397
objspace.o: $(top_srcdir)/internal/namespace.h
398+
objspace.o: $(top_srcdir)/internal/object.h
398399
objspace.o: $(top_srcdir)/internal/sanitizers.h
399400
objspace.o: $(top_srcdir)/internal/serial.h
400401
objspace.o: $(top_srcdir)/internal/set_table.h
@@ -609,6 +610,7 @@ objspace_dump.o: $(top_srcdir)/internal/hash.h
609610
objspace_dump.o: $(top_srcdir)/internal/imemo.h
610611
objspace_dump.o: $(top_srcdir)/internal/io.h
611612
objspace_dump.o: $(top_srcdir)/internal/namespace.h
613+
objspace_dump.o: $(top_srcdir)/internal/object.h
612614
objspace_dump.o: $(top_srcdir)/internal/sanitizers.h
613615
objspace_dump.o: $(top_srcdir)/internal/serial.h
614616
objspace_dump.o: $(top_srcdir)/internal/set_table.h

‎gc.c‎

Lines changed: 5 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,6 @@ rb_data_free(void *objspace, VALUE obj)
12121212

12131213
struct classext_foreach_args {
12141214
VALUE klass;
1215-
bool obj_too_complex;
12161215
rb_objspace_t *objspace; // used for update_*
12171216
};
12181217

@@ -1224,12 +1223,6 @@ classext_free(rb_classext_t *ext, bool is_prime, VALUE namespace, void *arg)
12241223

12251224
rb_id_table_free(RCLASSEXT_M_TBL(ext));
12261225
rb_cc_tbl_free(RCLASSEXT_CC_TBL(ext), args->klass);
1227-
if (args->obj_too_complex) {
1228-
st_free_table((st_table *)RCLASSEXT_FIELDS(ext));
1229-
}
1230-
else {
1231-
xfree(RCLASSEXT_FIELDS(ext));
1232-
}
12331226
if (!RCLASSEXT_SHARED_CONST_TBL(ext) && (tbl = RCLASSEXT_CONST_TBL(ext)) != NULL) {
12341227
rb_free_const_table(tbl);
12351228
}
@@ -1302,8 +1295,6 @@ rb_gc_obj_free(void *objspace, VALUE obj)
13021295
case T_MODULE:
13031296
case T_CLASS:
13041297
args.klass = obj;
1305-
args.obj_too_complex = rb_shape_obj_too_complex_p(obj) ? true : false;
1306-
13071298
rb_class_classext_foreach(obj, classext_free, (void *)&args);
13081299
if (RCLASS(obj)->ns_classext_tbl) {
13091300
st_free_table(RCLASS(obj)->ns_classext_tbl);
@@ -1495,6 +1486,8 @@ internal_object_p(VALUE obj)
14951486
return rb_singleton_class_internal_p(obj);
14961487
}
14971488
return 0;
1489+
case T_OBJECT:
1490+
if (FL_TEST_RAW(obj, ROBJECT_HIDDEN)) break;
14981491
default:
14991492
if (!RBASIC(obj)->klass) break;
15001493
return 0;
@@ -2274,20 +2267,6 @@ classext_memsize(rb_classext_t *ext, bool prime, VALUE namespace, void *arg)
22742267
*size += s;
22752268
}
22762269

2277-
static void
2278-
classext_fields_hash_memsize(rb_classext_t *ext, bool prime, VALUE namespace, void *arg)
2279-
{
2280-
size_t *size = (size_t *)arg;
2281-
size_t count;
2282-
RB_VM_LOCK_ENTER();
2283-
{
2284-
count = rb_st_table_size((st_table *)RCLASSEXT_FIELDS(ext));
2285-
}
2286-
RB_VM_LOCK_LEAVE();
2287-
// class IV sizes are allocated as powers of two
2288-
*size += SIZEOF_VALUE << bit_length(count);
2289-
}
2290-
22912270
static void
22922271
classext_superclasses_memsize(rb_classext_t *ext, bool prime, VALUE namespace, void *arg)
22932272
{
@@ -2326,15 +2305,6 @@ rb_obj_memsize_of(VALUE obj)
23262305
case T_MODULE:
23272306
case T_CLASS:
23282307
rb_class_classext_foreach(obj, classext_memsize, (void *)&size);
2329-
2330-
if (rb_shape_obj_too_complex_p(obj)) {
2331-
rb_class_classext_foreach(obj, classext_fields_hash_memsize, (void *)&size);
2332-
}
2333-
else {
2334-
// class IV sizes are allocated as powers of two
2335-
size += SIZEOF_VALUE << bit_length(RCLASS_FIELDS_COUNT(obj));
2336-
}
2337-
23382308
rb_class_classext_foreach(obj, classext_superclasses_memsize, (void *)&size);
23392309
break;
23402310
case T_ICLASS:
@@ -3107,10 +3077,7 @@ gc_mark_classext_module(rb_classext_t *ext, bool prime, VALUE namespace, void *a
31073077
gc_mark_internal(RCLASSEXT_SUPER(ext));
31083078
}
31093079
mark_m_tbl(objspace, RCLASSEXT_M_TBL(ext));
3110-
if (rb_shape_obj_too_complex_p(obj)) {
3111-
gc_mark_tbl_no_pin((st_table *)RCLASSEXT_FIELDS(ext));
3112-
// for the case ELSE is written in rb_gc_mark_children() because it's per RClass, not classext
3113-
}
3080+
gc_mark_internal(RCLASSEXT_FIELDS_OBJ(ext));
31143081
if (!RCLASSEXT_SHARED_CONST_TBL(ext) && RCLASSEXT_CONST_TBL(ext)) {
31153082
mark_const_tbl(objspace, RCLASSEXT_CONST_TBL(ext));
31163083
}
@@ -3191,11 +3158,7 @@ rb_gc_mark_children(void *objspace, VALUE obj)
31913158
foreach_args.obj = obj;
31923159
rb_class_classext_foreach(obj, gc_mark_classext_module, (void *)&foreach_args);
31933160

3194-
if (!rb_shape_obj_too_complex_p(obj)) {
3195-
for (attr_index_t i = 0; i < RCLASS_FIELDS_COUNT(obj); i++) {
3196-
gc_mark_internal(RCLASS_PRIME_FIELDS(obj)[i]);
3197-
}
3198-
}
3161+
gc_mark_internal(RCLASS_PRIME_FIELDS_OBJ(obj));
31993162
break;
32003163

32013164
case T_ICLASS:
@@ -3827,7 +3790,6 @@ static void
38273790
update_classext(rb_classext_t *ext, bool is_prime, VALUE namespace, void *arg)
38283791
{
38293792
struct classext_foreach_args *args = (struct classext_foreach_args *)arg;
3830-
VALUE klass = args->klass;
38313793
rb_objspace_t *objspace = args->objspace;
38323794

38333795
if (RCLASSEXT_SUPER(ext)) {
@@ -3836,16 +3798,7 @@ update_classext(rb_classext_t *ext, bool is_prime, VALUE namespace, void *arg)
38363798

38373799
update_m_tbl(objspace, RCLASSEXT_M_TBL(ext));
38383800

3839-
if (args->obj_too_complex) {
3840-
gc_ref_update_table_values_only((st_table *)RCLASSEXT_FIELDS(ext));
3841-
}
3842-
else {
3843-
// Classext is not copied in this case
3844-
for (attr_index_t i = 0; i < RCLASS_FIELDS_COUNT(klass); i++) {
3845-
UPDATE_IF_MOVED(objspace, RCLASSEXT_FIELDS(RCLASS_EXT_PRIME(klass))[i]);
3846-
}
3847-
}
3848-
3801+
UPDATE_IF_MOVED(objspace, ext->fields_obj);
38493802
if (!RCLASSEXT_SHARED_CONST_TBL(ext)) {
38503803
update_const_tbl(objspace, RCLASSEXT_CONST_TBL(ext));
38513804
}
@@ -4202,7 +4155,6 @@ rb_gc_update_object_references(void *objspace, VALUE obj)
42024155
// Continue to the shared T_CLASS/T_MODULE
42034156
case T_MODULE:
42044157
args.klass = obj;
4205-
args.obj_too_complex = rb_shape_obj_too_complex_p(obj);
42064158
args.objspace = objspace;
42074159
rb_class_classext_foreach(obj, update_classext, (void *)&args);
42084160
break;

‎include/ruby/internal/core/robject.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ enum ruby_robject_flags {
7171
* 3rd parties must not be aware that there even is more than one way to
7272
* store instance variables. Might better be hidden.
7373
*/
74-
ROBJECT_EMBED = RUBY_FL_USER1
74+
ROBJECT_EMBED = RUBY_FL_USER1,
75+
76+
ROBJECT_HIDDEN = RUBY_FL_USER2, // HACK
7577
};
7678

7779
struct st_table;

0 commit comments

Comments
 (0)