Skip to content

Commit c33f17c

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 135583e commit c33f17c

11 files changed

Lines changed: 162 additions & 245 deletions

File tree

‎class.c‎

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

298298
RCLASSEXT_M_TBL(ext) = duplicate_classext_m_tbl(RCLASSEXT_M_TBL(orig), klass, dup_iclass);
299299

300-
// TODO: consider shapes for performance
301-
if (RCLASSEXT_FIELDS(orig)) {
302-
RUBY_ASSERT(!RB_TYPE_P(klass, T_ICLASS));
303-
RCLASSEXT_FIELDS(ext) = (VALUE *)st_copy((st_table *)RCLASSEXT_FIELDS(orig));
304-
rb_autoload_copy_table_for_namespace((st_table *)RCLASSEXT_FIELDS(ext), ns);
305-
}
306-
else {
307-
if (!RB_TYPE_P(klass, T_ICLASS)) {
308-
RCLASSEXT_FIELDS(ext) = (VALUE *)st_init_numtable();
309-
}
300+
if (orig->fields_obj) {
301+
ext->fields_obj = rb_obj_clone(orig->fields_obj);
310302
}
311303

312304
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
@@ -20382,6 +20389,7 @@ vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/error.h
2038220389
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/gc.h
2038320390
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/imemo.h
2038420391
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/namespace.h
20392+
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/object.h
2038520393
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
2038620394
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/serial.h
2038720395
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/set_table.h
@@ -21060,6 +21068,7 @@ vm_trace.$(OBJEXT): $(top_srcdir)/internal/gc.h
2106021068
vm_trace.$(OBJEXT): $(top_srcdir)/internal/hash.h
2106121069
vm_trace.$(OBJEXT): $(top_srcdir)/internal/imemo.h
2106221070
vm_trace.$(OBJEXT): $(top_srcdir)/internal/namespace.h
21071+
vm_trace.$(OBJEXT): $(top_srcdir)/internal/object.h
2106321072
vm_trace.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
2106421073
vm_trace.$(OBJEXT): $(top_srcdir)/internal/serial.h
2106521074
vm_trace.$(OBJEXT): $(top_srcdir)/internal/set_table.h
@@ -21507,6 +21516,7 @@ yjit.$(OBJEXT): $(top_srcdir)/internal/hash.h
2150721516
yjit.$(OBJEXT): $(top_srcdir)/internal/imemo.h
2150821517
yjit.$(OBJEXT): $(top_srcdir)/internal/namespace.h
2150921518
yjit.$(OBJEXT): $(top_srcdir)/internal/numeric.h
21519+
yjit.$(OBJEXT): $(top_srcdir)/internal/object.h
2151021520
yjit.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
2151121521
yjit.$(OBJEXT): $(top_srcdir)/internal/serial.h
2151221522
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 & 51 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
}
@@ -1303,8 +1296,6 @@ rb_gc_obj_free(void *objspace, VALUE obj)
13031296
case T_MODULE:
13041297
case T_CLASS:
13051298
args.klass = obj;
1306-
args.obj_too_complex = rb_shape_obj_too_complex_p(obj) ? true : false;
1307-
13081299
rb_class_classext_foreach(obj, classext_free, (void *)&args);
13091300
if (RCLASS(obj)->ns_classext_tbl) {
13101301
st_free_table(RCLASS(obj)->ns_classext_tbl);
@@ -1499,6 +1490,8 @@ internal_object_p(VALUE obj)
14991490
return rb_singleton_class_internal_p(obj);
15001491
}
15011492
return 0;
1493+
case T_OBJECT:
1494+
if (FL_TEST_RAW(obj, ROBJECT_HIDDEN)) break;
15021495
default:
15031496
if (!RBASIC(obj)->klass) break;
15041497
return 0;
@@ -2267,18 +2260,6 @@ classext_memsize(rb_classext_t *ext, bool prime, VALUE namespace, void *arg)
22672260
*size += s;
22682261
}
22692262

2270-
static void
2271-
classext_fields_hash_memsize(rb_classext_t *ext, bool prime, VALUE namespace, void *arg)
2272-
{
2273-
size_t *size = (size_t *)arg;
2274-
size_t count;
2275-
RB_VM_LOCKING() {
2276-
count = rb_st_table_size((st_table *)RCLASSEXT_FIELDS(ext));
2277-
}
2278-
// class IV sizes are allocated as powers of two
2279-
*size += SIZEOF_VALUE << bit_length(count);
2280-
}
2281-
22822263
static void
22832264
classext_superclasses_memsize(rb_classext_t *ext, bool prime, VALUE namespace, void *arg)
22842265
{
@@ -2316,15 +2297,6 @@ rb_obj_memsize_of(VALUE obj)
23162297
case T_MODULE:
23172298
case T_CLASS:
23182299
rb_class_classext_foreach(obj, classext_memsize, (void *)&size);
2319-
2320-
if (rb_shape_obj_too_complex_p(obj)) {
2321-
rb_class_classext_foreach(obj, classext_fields_hash_memsize, (void *)&size);
2322-
}
2323-
else {
2324-
// class IV sizes are allocated as powers of two
2325-
size += SIZEOF_VALUE << bit_length(RCLASS_FIELDS_COUNT(obj));
2326-
}
2327-
23282300
rb_class_classext_foreach(obj, classext_superclasses_memsize, (void *)&size);
23292301
break;
23302302
case T_ICLASS:
@@ -3097,10 +3069,7 @@ gc_mark_classext_module(rb_classext_t *ext, bool prime, VALUE namespace, void *a
30973069
gc_mark_internal(RCLASSEXT_SUPER(ext));
30983070
}
30993071
mark_m_tbl(objspace, RCLASSEXT_M_TBL(ext));
3100-
if (rb_shape_obj_too_complex_p(obj)) {
3101-
gc_mark_tbl_no_pin((st_table *)RCLASSEXT_FIELDS(ext));
3102-
// for the case ELSE is written in rb_gc_mark_children() because it's per RClass, not classext
3103-
}
3072+
gc_mark_internal(RCLASSEXT_FIELDS_OBJ(ext));
31043073
if (!RCLASSEXT_SHARED_CONST_TBL(ext) && RCLASSEXT_CONST_TBL(ext)) {
31053074
mark_const_tbl(objspace, RCLASSEXT_CONST_TBL(ext));
31063075
}
@@ -3181,11 +3150,7 @@ rb_gc_mark_children(void *objspace, VALUE obj)
31813150
foreach_args.obj = obj;
31823151
rb_class_classext_foreach(obj, gc_mark_classext_module, (void *)&foreach_args);
31833152

3184-
if (!rb_shape_obj_too_complex_p(obj)) {
3185-
for (attr_index_t i = 0; i < RCLASS_FIELDS_COUNT(obj); i++) {
3186-
gc_mark_internal(RCLASS_PRIME_FIELDS(obj)[i]);
3187-
}
3188-
}
3153+
gc_mark_internal(RCLASS_PRIME_FIELDS_OBJ(obj));
31893154
break;
31903155

31913156
case T_ICLASS:
@@ -3811,7 +3776,6 @@ static void
38113776
update_classext(rb_classext_t *ext, bool is_prime, VALUE namespace, void *arg)
38123777
{
38133778
struct classext_foreach_args *args = (struct classext_foreach_args *)arg;
3814-
VALUE klass = args->klass;
38153779
rb_objspace_t *objspace = args->objspace;
38163780

38173781
if (RCLASSEXT_SUPER(ext)) {
@@ -3820,16 +3784,7 @@ update_classext(rb_classext_t *ext, bool is_prime, VALUE namespace, void *arg)
38203784

38213785
update_m_tbl(objspace, RCLASSEXT_M_TBL(ext));
38223786

3823-
if (args->obj_too_complex) {
3824-
gc_ref_update_table_values_only((st_table *)RCLASSEXT_FIELDS(ext));
3825-
}
3826-
else {
3827-
// Classext is not copied in this case
3828-
for (attr_index_t i = 0; i < RCLASS_FIELDS_COUNT(klass); i++) {
3829-
UPDATE_IF_MOVED(objspace, RCLASSEXT_FIELDS(RCLASS_EXT_PRIME(klass))[i]);
3830-
}
3831-
}
3832-
3787+
UPDATE_IF_MOVED(objspace, ext->fields_obj);
38333788
if (!RCLASSEXT_SHARED_CONST_TBL(ext)) {
38343789
update_const_tbl(objspace, RCLASSEXT_CONST_TBL(ext));
38353790
}
@@ -4186,7 +4141,6 @@ rb_gc_update_object_references(void *objspace, VALUE obj)
41864141
// Continue to the shared T_CLASS/T_MODULE
41874142
case T_MODULE:
41884143
args.klass = obj;
4189-
args.obj_too_complex = rb_shape_obj_too_complex_p(obj);
41904144
args.objspace = objspace;
41914145
rb_class_classext_foreach(obj, update_classext, (void *)&args);
41924146
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)