Make RCLASS_EXT(c)->subclasses a doubly linked list

Updating RCLASS_PARENT_SUBCLASSES and RCLASS_MODULE_SUBCLASSES while
compacting can trigger the read barrier. This commit makes
RCLASS_SUBCLASSES a doubly linked list with a dedicated head object so
that we can add and remove entries from the list without having to touch
an object in the Ruby heap

class.c

@@ -37,87 +37,113 @@
 
 RUBY_EXTERN rb_serial_t ruby_vm_global_cvar_state;
 
-void
+static rb_subclass_entry_t *
-rb_class_subclass_add(VALUE super, VALUE klass)
+push_subclass_entry_to_list(VALUE super, VALUE klass)
 {
     rb_subclass_entry_t *entry, *head;
 
-    if (super && super != Qundef) {
+    entry = ZALLOC(rb_subclass_entry_t);
-	entry = ALLOC(rb_subclass_entry_t);
     entry->klass = klass;
-	entry->next = NULL;
 
     head = RCLASS_SUBCLASSES(super);
-	if (head) {
+    if (!head) {
-	    entry->next = head;
+        head = ZALLOC(rb_subclass_entry_t);
-	    RCLASS_PARENT_SUBCLASSES(head->klass) = &entry->next;
+        RCLASS_SUBCLASSES(super) = head;
+    }
+    entry->next = head->next;
+    entry->prev = head;
+
+    if (head->next) {
+        head->next->prev = entry;
+    }
+    head->next = entry;
+
+    return entry;
 }
 
-	RCLASS_SUBCLASSES(super) = entry;
+void
-	RCLASS_PARENT_SUBCLASSES(klass) = &RCLASS_SUBCLASSES(super);
+rb_class_subclass_add(VALUE super, VALUE klass)
+{
+    if (super && super != Qundef) {
+        rb_subclass_entry_t *entry = push_subclass_entry_to_list(super, klass);
+        RCLASS_SUBCLASS_ENTRY(klass) = entry;
     }
 }
 
 static void
 rb_module_add_to_subclasses_list(VALUE module, VALUE iclass)
 {
-    rb_subclass_entry_t *entry, *head;
+    rb_subclass_entry_t *entry = push_subclass_entry_to_list(module, iclass);
-
+    RCLASS_MODULE_SUBCLASS_ENTRY(iclass) = entry;
-    entry = ALLOC(rb_subclass_entry_t);
-    entry->klass = iclass;
-    entry->next = NULL;
-
-    head = RCLASS_SUBCLASSES(module);
-    if (head) {
-	entry->next = head;
-	RCLASS_MODULE_SUBCLASSES(head->klass) = &entry->next;
 }
 
-    RCLASS_SUBCLASSES(module) = entry;
+void
-    RCLASS_MODULE_SUBCLASSES(iclass) = &RCLASS_SUBCLASSES(module);
+rb_class_remove_subclass_head(VALUE klass)
+{
+    rb_subclass_entry_t *head = RCLASS_SUBCLASSES(klass);
+
+    if (head) {
+        if (head->next) {
+            head->next->prev = NULL;
+        }
+        RCLASS_SUBCLASSES(klass) = NULL;
+        xfree(head);
+    }
 }
 
 void
 rb_class_remove_from_super_subclasses(VALUE klass)
 {
-    rb_subclass_entry_t **prev = RCLASS_PARENT_SUBCLASSES(klass);
+    rb_subclass_entry_t *entry = RCLASS_SUBCLASS_ENTRY(klass);
+
+    if (entry) {
+        rb_subclass_entry_t *prev = entry->prev, *next = entry->next;
 
         if (prev) {
-	rb_subclass_entry_t *entry = *prev, *next = entry->next;
+            prev->next = next;
-
-	*prev = next;
-	if (next) {
-	    RCLASS_PARENT_SUBCLASSES(next->klass) = prev;
         }
+        if (next) {
+            next->prev = prev;
+        }
+
 	xfree(entry);
     }
 
-    RCLASS_PARENT_SUBCLASSES(klass) = NULL;
+    RCLASS_SUBCLASS_ENTRY(klass) = NULL;
 }
 
 void
 rb_class_remove_from_module_subclasses(VALUE klass)
 {
-    rb_subclass_entry_t **prev = RCLASS_MODULE_SUBCLASSES(klass);
+    rb_subclass_entry_t *entry = RCLASS_MODULE_SUBCLASS_ENTRY(klass);
+
+    if (entry) {
+        rb_subclass_entry_t *prev = entry->prev, *next = entry->next;
 
         if (prev) {
-        rb_subclass_entry_t *entry = *prev, *next = entry->next;
+            prev->next = next;
-
+        }
-	*prev = next;
 	if (next) {
-	    RCLASS_MODULE_SUBCLASSES(next->klass) = prev;
+            next->prev = prev;
 	}
 
 	xfree(entry);
     }
 
-    RCLASS_MODULE_SUBCLASSES(klass) = NULL;
+    RCLASS_MODULE_SUBCLASS_ENTRY(klass) = NULL;
 }
 
 void
 rb_class_foreach_subclass(VALUE klass, void (*f)(VALUE, VALUE), VALUE arg)
 {
+    // RCLASS_SUBCLASSES should always point to our head element which has NULL klass
     rb_subclass_entry_t *cur = RCLASS_SUBCLASSES(klass);
+    // if we have a subclasses list, then the head is a placeholder with no valid
+    // class. So ignore it and use the next element in the list (if one exists)
+    if (cur) {
+        RUBY_ASSERT(!cur->klass);
+        cur = cur->next;
+    }
 
     /* do not be tempted to simplify this loop into a for loop, the order of
        operations is important here if `f` modifies the linked list */
@@ -963,6 +989,12 @@ rb_include_module(VALUE klass, VALUE module)
 
     if (RB_TYPE_P(klass, T_MODULE)) {
         rb_subclass_entry_t *iclass = RCLASS_SUBCLASSES(klass);
+        // skip the placeholder subclass entry at the head of the list
+        if (iclass && iclass->next) {
+            RUBY_ASSERT(!iclass->klass);
+            iclass = iclass->next;
+        }
+
         int do_include = 1;
         while (iclass) {
             VALUE check_class = iclass->klass;
@@ -1202,6 +1234,12 @@ rb_prepend_module(VALUE klass, VALUE module)
     }
     if (RB_TYPE_P(klass, T_MODULE)) {
         rb_subclass_entry_t *iclass = RCLASS_SUBCLASSES(klass);
+        // skip the placeholder subclass entry at the head of the list if it exists
+        if (iclass && iclass->next) {
+            RUBY_ASSERT(!iclass->klass);
+            iclass = iclass->next;
+        }
+
         VALUE klass_origin = RCLASS_ORIGIN(klass);
         struct rb_id_table *klass_m_tbl = RCLASS_M_TBL(klass);
         struct rb_id_table *klass_origin_m_tbl = RCLASS_M_TBL(klass_origin);

gc.c

@@ -3134,15 +3134,7 @@ obj_free(rb_objspace_t *objspace, VALUE obj)
             rb_id_table_foreach_values(RCLASS_CVC_TBL(obj), cvar_table_free_i, NULL);
             rb_id_table_free(RCLASS_CVC_TBL(obj));
 	}
-	if (RCLASS_SUBCLASSES(obj)) {
+        rb_class_remove_subclass_head(obj);
-	    if (BUILTIN_TYPE(obj) == T_MODULE) {
-		rb_class_detach_module_subclasses(obj);
-	    }
-	    else {
-		rb_class_detach_subclasses(obj);
-	    }
-	    RCLASS_SUBCLASSES(obj) = NULL;
-	}
 	rb_class_remove_from_module_subclasses(obj);
 	rb_class_remove_from_super_subclasses(obj);
 #if !USE_RVARGC
@@ -3307,10 +3299,7 @@ obj_free(rb_objspace_t *objspace, VALUE obj)
 	if (RCLASS_CALLABLE_M_TBL(obj) != NULL) {
 	    rb_id_table_free(RCLASS_CALLABLE_M_TBL(obj));
 	}
-	if (RCLASS_SUBCLASSES(obj)) {
+        rb_class_remove_subclass_head(obj);
-	    rb_class_detach_subclasses(obj);
-	    RCLASS_SUBCLASSES(obj) = NULL;
-	}
         cc_table_free(objspace, obj, FALSE);
 	rb_class_remove_from_module_subclasses(obj);
 	rb_class_remove_from_super_subclasses(obj);

internal/class.h

@@ -22,6 +22,7 @@
 struct rb_subclass_entry {
     VALUE klass;
     struct rb_subclass_entry *next;
+    struct rb_subclass_entry *prev;
 };
 
 struct rb_iv_index_tbl_entry {
@@ -47,13 +48,13 @@ struct rb_classext_struct {
     struct rb_id_table *cc_tbl; /* ID -> [[ci, cc1], cc2, ...] */
     struct rb_id_table *cvc_tbl;
     struct rb_subclass_entry *subclasses;
-    struct rb_subclass_entry **parent_subclasses;
+    struct rb_subclass_entry *subclass_entry;
     /**
      * In the case that this is an `ICLASS`, `module_subclasses` points to the link
      * in the module's `subclasses` list that indicates that the klass has been
      * included. Hopefully that makes sense.
      */
-    struct rb_subclass_entry **module_subclasses;
+    struct rb_subclass_entry *module_subclass_entry;
 #if SIZEOF_SERIAL_T != SIZEOF_VALUE /* otherwise class_serial is in struct RClass */
     rb_serial_t class_serial;
 #endif
@@ -105,8 +106,8 @@ typedef struct rb_classext_struct rb_classext_t;
 # define RCLASS_SERIAL(c) (RCLASS_EXT(c)->class_serial)
 #endif
 #define RCLASS_INCLUDER(c) (RCLASS_EXT(c)->includer)
-#define RCLASS_PARENT_SUBCLASSES(c) (RCLASS_EXT(c)->parent_subclasses)
+#define RCLASS_SUBCLASS_ENTRY(c) (RCLASS_EXT(c)->subclass_entry)
-#define RCLASS_MODULE_SUBCLASSES(c) (RCLASS_EXT(c)->module_subclasses)
+#define RCLASS_MODULE_SUBCLASS_ENTRY(c) (RCLASS_EXT(c)->module_subclass_entry)
 #define RCLASS_ALLOCATOR(c) (RCLASS_EXT(c)->allocator)
 #define RCLASS_SUBCLASSES(c) (RCLASS_EXT(c)->subclasses)
 
@@ -117,6 +118,7 @@ typedef struct rb_classext_struct rb_classext_t;
 /* class.c */
 void rb_class_subclass_add(VALUE super, VALUE klass);
 void rb_class_remove_from_super_subclasses(VALUE);
+void rb_class_remove_subclass_head(VALUE);
 int rb_singleton_class_internal_p(VALUE sklass);
 VALUE rb_class_boot(VALUE);
 VALUE rb_class_s_alloc(VALUE klass);