Get rid of gen_fields_tbl.fields_count

byroot · byroot · commit f9966b9b7670 · 2025-06-09T16:38:29.000+02:00
This data is redundant because the shape already contains both the
length and capacity of the object's fields.

So it both waste space and create the possibility of a desync between
the two.

We also do not need to initialize everything to Qundef, this seem
to be a left-over from pre-shape instance variables.
diff --git a/gc.c b/gc.c
@@ -4093,7 +4093,8 @@ vm_weak_table_gen_fields_foreach(st_data_t key, st_data_t value, st_data_t data)
             );
         }
         else {
-            for (uint32_t i = 0; i < fields_tbl->as.shape.fields_count; i++) {
+            uint32_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID((VALUE)key));
+            for (uint32_t i = 0; i < fields_count; i++) {
                 if (SPECIAL_CONST_P(fields_tbl->as.shape.fields[i])) continue;
 
                 int ivar_ret = iter_data->callback(fields_tbl->as.shape.fields[i], iter_data->data);
diff --git a/ractor.c b/ractor.c
@@ -1658,10 +1658,9 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data)
             if (d.stop) return 1;
         }
         else {
-            for (uint32_t i = 0; i < fields_tbl->as.shape.fields_count; i++) {
-                if (!UNDEF_P(fields_tbl->as.shape.fields[i])) {
-                    CHECK_AND_REPLACE(fields_tbl->as.shape.fields[i]);
-                }
+            uint32_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID(obj));
+            for (uint32_t i = 0; i < fields_count; i++) {
+                CHECK_AND_REPLACE(fields_tbl->as.shape.fields[i]);
             }
         }
     }
diff --git a/variable.c b/variable.c
@@ -1228,19 +1228,10 @@ gen_fields_tbl_bytes(size_t n)
 }
 
 static struct gen_fields_tbl *
-gen_fields_tbl_resize(struct gen_fields_tbl *old, uint32_t n)
+gen_fields_tbl_resize(struct gen_fields_tbl *old, uint32_t new_capa)
 {
-    RUBY_ASSERT(n > 0);
-
-    uint32_t len = old ? old->as.shape.fields_count : 0;
-    struct gen_fields_tbl *fields_tbl = xrealloc(old, gen_fields_tbl_bytes(n));
-
-    fields_tbl->as.shape.fields_count = n;
-    for (; len < n; len++) {
-        fields_tbl->as.shape.fields[len] = Qundef;
-    }
-
-    return fields_tbl;
+    RUBY_ASSERT(new_capa > 0);
+    return xrealloc(old, gen_fields_tbl_bytes(new_capa));
 }
 
 void
@@ -1253,7 +1244,8 @@ rb_mark_generic_ivar(VALUE obj)
             rb_mark_tbl_no_pin(fields_tbl->as.complex.table);
         }
         else {
-            for (uint32_t i = 0; i < fields_tbl->as.shape.fields_count; i++) {
+            uint32_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID(obj));
+            for (uint32_t i = 0; i < fields_count; i++) {
                 rb_gc_mark_movable(fields_tbl->as.shape.fields[i]);
             }
         }
@@ -1290,7 +1282,7 @@ rb_generic_ivar_memsize(VALUE obj)
             return sizeof(struct gen_fields_tbl) + st_memsize(fields_tbl->as.complex.table);
         }
         else {
-            return gen_fields_tbl_bytes(fields_tbl->as.shape.fields_count);
+            return gen_fields_tbl_bytes(RSHAPE_CAPACITY(RBASIC_SHAPE_ID(obj)));
         }
     }
     return 0;
@@ -1299,21 +1291,12 @@ rb_generic_ivar_memsize(VALUE obj)
 static size_t
 gen_fields_tbl_count(VALUE obj, const struct gen_fields_tbl *fields_tbl)
 {
-    uint32_t i;
-    size_t n = 0;
-
     if (rb_shape_obj_too_complex_p(obj)) {
-        n = st_table_size(fields_tbl->as.complex.table);
+        return st_table_size(fields_tbl->as.complex.table);
     }
     else {
-        for (i = 0; i < fields_tbl->as.shape.fields_count; i++) {
-            if (!UNDEF_P(fields_tbl->as.shape.fields[i])) {
-                n++;
-            }
-        }
+        return RSHAPE_LEN(RBASIC_SHAPE_ID(obj));
     }
-
-    return n;
 }
 
 VALUE
diff --git a/variable.h b/variable.h
@@ -15,7 +15,6 @@
 struct gen_fields_tbl {
     union {
         struct {
-            uint32_t fields_count;
             VALUE fields[1];
         } shape;
         struct {

Original file line number	Diff line number	Diff line change
`@@ -4093,7 +4093,8 @@ vm_weak_table_gen_fields_foreach(st_data_t key, st_data_t value, st_data_t data)`
`4093`	`4093`	`);`
`4094`	`4094`	`}`
`4095`	`4095`	`else {`
`4096`		`- for (uint32_t i = 0; i < fields_tbl->as.shape.fields_count; i++) {`
	`4096`	`+ uint32_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID((VALUE)key));`
	`4097`	`+ for (uint32_t i = 0; i < fields_count; i++) {`
`4097`	`4098`	`if (SPECIAL_CONST_P(fields_tbl->as.shape.fields[i])) continue;`
`4098`	`4099`
`4099`	`4100`	`int ivar_ret = iter_data->callback(fields_tbl->as.shape.fields[i], iter_data->data);`
Original file line number	Diff line number	Diff line change
`@@ -1658,10 +1658,9 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data)`
`1658`	`1658`	`if (d.stop) return 1;`
`1659`	`1659`	`}`
`1660`	`1660`	`else {`
`1661`		`- for (uint32_t i = 0; i < fields_tbl->as.shape.fields_count; i++) {`
`1662`		`- if (!UNDEF_P(fields_tbl->as.shape.fields[i])) {`
`1663`		`- CHECK_AND_REPLACE(fields_tbl->as.shape.fields[i]);`
`1664`		`- }`
	`1661`	`+ uint32_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID(obj));`
	`1662`	`+ for (uint32_t i = 0; i < fields_count; i++) {`
	`1663`	`+ CHECK_AND_REPLACE(fields_tbl->as.shape.fields[i]);`
`1665`	`1664`	`}`
`1666`	`1665`	`}`
`1667`	`1666`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1228,19 +1228,10 @@ gen_fields_tbl_bytes(size_t n)`
`1228`	`1228`	`}`
`1229`	`1229`
`1230`	`1230`	`static struct gen_fields_tbl *`
`1231`		`-gen_fields_tbl_resize(struct gen_fields_tbl *old, uint32_t n)`
	`1231`	`+gen_fields_tbl_resize(struct gen_fields_tbl *old, uint32_t new_capa)`
`1232`	`1232`	`{`
`1233`		`- RUBY_ASSERT(n > 0);`
`1234`		`-`
`1235`		`- uint32_t len = old ? old->as.shape.fields_count : 0;`
`1236`		`- struct gen_fields_tbl *fields_tbl = xrealloc(old, gen_fields_tbl_bytes(n));`
`1237`		`-`
`1238`		`- fields_tbl->as.shape.fields_count = n;`
`1239`		`- for (; len < n; len++) {`
`1240`		`- fields_tbl->as.shape.fields[len] = Qundef;`
`1241`		`- }`
`1242`		`-`
`1243`		`- return fields_tbl;`
	`1233`	`+ RUBY_ASSERT(new_capa > 0);`
	`1234`	`+ return xrealloc(old, gen_fields_tbl_bytes(new_capa));`
`1244`	`1235`	`}`
`1245`	`1236`
`1246`	`1237`	`void`
`@@ -1253,7 +1244,8 @@ rb_mark_generic_ivar(VALUE obj)`
`1253`	`1244`	`rb_mark_tbl_no_pin(fields_tbl->as.complex.table);`
`1254`	`1245`	`}`
`1255`	`1246`	`else {`
`1256`		`- for (uint32_t i = 0; i < fields_tbl->as.shape.fields_count; i++) {`
	`1247`	`+ uint32_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID(obj));`
	`1248`	`+ for (uint32_t i = 0; i < fields_count; i++) {`
`1257`	`1249`	`rb_gc_mark_movable(fields_tbl->as.shape.fields[i]);`
`1258`	`1250`	`}`
`1259`	`1251`	`}`
`@@ -1290,7 +1282,7 @@ rb_generic_ivar_memsize(VALUE obj)`
`1290`	`1282`	`return sizeof(struct gen_fields_tbl) + st_memsize(fields_tbl->as.complex.table);`
`1291`	`1283`	`}`
`1292`	`1284`	`else {`
`1293`		`- return gen_fields_tbl_bytes(fields_tbl->as.shape.fields_count);`
	`1285`	`+ return gen_fields_tbl_bytes(RSHAPE_CAPACITY(RBASIC_SHAPE_ID(obj)));`
`1294`	`1286`	`}`
`1295`	`1287`	`}`
`1296`	`1288`	`return 0;`
`@@ -1299,21 +1291,12 @@ rb_generic_ivar_memsize(VALUE obj)`
`1299`	`1291`	`static size_t`
`1300`	`1292`	`gen_fields_tbl_count(VALUE obj, const struct gen_fields_tbl *fields_tbl)`
`1301`	`1293`	`{`
`1302`		`- uint32_t i;`
`1303`		`- size_t n = 0;`
`1304`		`-`
`1305`	`1294`	`if (rb_shape_obj_too_complex_p(obj)) {`
`1306`		`- n = st_table_size(fields_tbl->as.complex.table);`
	`1295`	`+ return st_table_size(fields_tbl->as.complex.table);`
`1307`	`1296`	`}`
`1308`	`1297`	`else {`
`1309`		`- for (i = 0; i < fields_tbl->as.shape.fields_count; i++) {`
`1310`		`- if (!UNDEF_P(fields_tbl->as.shape.fields[i])) {`
`1311`		`- n++;`
`1312`		`- }`
`1313`		`- }`
	`1298`	`+ return RSHAPE_LEN(RBASIC_SHAPE_ID(obj));`
`1314`	`1299`	`}`
`1315`		`-`
`1316`		`- return n;`
`1317`	`1300`	`}`
`1318`	`1301`
`1319`	`1302`	`VALUE`