Lock-free rb_sym2id/rb_sym2str

This shouldn't slow anything down on x86 (plain load vs acquire load). A slight
slowdown for ARM is expected.

The new top-level id_entry_dir is replacing the hidden array and is RCU'd under
a lock.

Co-authored-by: Matt Valentine-House <matt@eightbitraptor.com>
Co-authored-by: John Hawthorn <john@hawthorn.email>

symbol.c

@@ -19,6 +19,7 @@
 #include "internal/symbol.h"
 #include "internal/vm.h"
 #include "probes.h"
+#include "ruby/atomic.h"
 #include "ruby/encoding.h"
 #include "ruby/ractor.h"
 #include "ruby/st.h"
@@ -41,8 +42,6 @@
 #define IDSET_ATTRSET_FOR_SYNTAX ((1U<<ID_LOCAL)|(1U<<ID_CONST))
 #define IDSET_ATTRSET_FOR_INTERN (~(~0U<<(1<<ID_SCOPE_SHIFT)) & ~(1U<<ID_ATTRSET))
 
-#define SYMBOL_PINNED_P(sym) (RSYMBOL(sym)->id&~ID_SCOPE_MASK)
-
 #define STATIC_SYM2ID(sym) RSHIFT((VALUE)(sym), RUBY_SPECIAL_SHIFT)
 
 static ID register_static_symid(ID, const char *, long, rb_encoding *);
@@ -212,6 +211,73 @@ static const rb_data_type_t sym_id_entry_list_type = {
     0, 0, RUBY_TYPED_THREAD_SAFE_FREE | RUBY_TYPED_WB_PROTECTED
 };
 
+/* Directory mapping an ID serial (via idx = serial / ID_ENTRY_UNIT) to its
+ * sym_id_entry bucket. */
+struct id_entry_dir {
+    long capa;
+    VALUE *entries;
+};
+
+static void
+id_entry_dir_mark(void *ptr)
+{
+    struct id_entry_dir *dir = ptr;
+    for (long i = 0; i < dir->capa; i++) {
+        if (dir->entries[i]) {
+            rb_gc_mark_movable(dir->entries[i]);
+        }
+    }
+}
+
+static void
+id_entry_dir_free(void *ptr)
+{
+    struct id_entry_dir *dir = ptr;
+    SIZED_FREE_N(dir->entries, dir->capa);
+}
+
+static size_t
+id_entry_dir_memsize(const void *ptr)
+{
+    const struct id_entry_dir *dir = ptr;
+    return sizeof(struct id_entry_dir) + dir->capa * sizeof(VALUE);
+}
+
+static void
+id_entry_dir_compact(void *ptr)
+{
+    struct id_entry_dir *dir = ptr;
+    for (long i = 0; i < dir->capa; i++) {
+        if (dir->entries[i]) {
+            dir->entries[i] = rb_gc_location(dir->entries[i]);
+        }
+    }
+}
+
+static const rb_data_type_t id_entry_dir_type = {
+    "symbol_id_entry_directory",
+    {
+        id_entry_dir_mark,
+        id_entry_dir_free,
+        id_entry_dir_memsize,
+        id_entry_dir_compact,
+    },
+    0, 0, RUBY_TYPED_THREAD_SAFE_FREE | RUBY_TYPED_WB_PROTECTED
+};
+
+static const long ID_ENTRY_DIR_INITIAL_CAPA = 16;
+
+static VALUE
+id_entry_dir_new(long capa)
+{
+    struct id_entry_dir *dir;
+    VALUE obj = TypedData_Make_Struct(0, struct id_entry_dir, &id_entry_dir_type, dir);
+    RB_OBJ_SET_SHAREABLE(obj);
+    dir->entries = ZALLOC_N(VALUE, capa);
+    dir->capa = capa; // must be set after the allocation in case of GC
+    return obj;
+}
+
 static int
 sym_check_asciionly(VALUE str, bool fake_str)
 {
@@ -264,6 +330,30 @@ next_id_base(void)
     return (ID)serial << ID_SCOPE_SHIFT;
 }
 
+static struct id_entry_dir *
+id_entry_dir_grow(rb_symbols_t *symbols, struct id_entry_dir *old_dir, size_t min_idx)
+{
+    ASSERT_vm_locking();
+
+    long new_capa = old_dir->capa ? old_dir->capa * 2 : ID_ENTRY_DIR_INITIAL_CAPA;
+    while ((size_t)new_capa <= min_idx) new_capa *= 2;
+
+    // id_entry_dir_new may trigger GC, but the old directory is still reachable
+    // via symbols->ids until we publish below, so it and its buckets stay marked.
+    VALUE new_obj = id_entry_dir_new(new_capa);
+    struct id_entry_dir *new_dir = RTYPEDDATA_GET_DATA(new_obj);
+
+    memcpy(new_dir->entries, old_dir->entries, old_dir->capa * sizeof(VALUE));
+    rb_gc_writebarrier_remember(new_obj);
+
+    // Publish: this release pairs with the acquire load in get_id_serial_entry.
+    // The old directory becomes unreachable from roots, but is kept alive for any
+    // in-flight lock-free reader by that reader's own machine stack
+    rbimpl_atomic_value_store(&symbols->ids, new_obj, RBIMPL_ATOMIC_RELEASE);
+
+    return new_dir;
+}
+
 static void
 set_id_entry(rb_symbols_t *symbols, rb_id_serial_t num, VALUE str, VALUE sym)
 {
@@ -273,18 +363,24 @@ set_id_entry(rb_symbols_t *symbols, rb_id_serial_t num, VALUE str, VALUE sym)
 
     size_t idx = num / ID_ENTRY_UNIT;
 
-    VALUE id_entry_list, ids = symbols->ids;
+    struct id_entry_dir *dir = RTYPEDDATA_GET_DATA(symbols->ids);
+    if (idx >= (size_t)dir->capa) {
+        dir = id_entry_dir_grow(symbols, dir, idx);
+    }
+
+    VALUE bucket = dir->entries[idx];
     rb_darray(struct sym_id_entry) entries;
-    if (idx >= (size_t)RARRAY_LEN(ids) || NIL_P(id_entry_list = rb_ary_entry(ids, (long)idx))) {
+    if (!bucket) {
         rb_darray_make(&entries, ID_ENTRY_UNIT);
-        id_entry_list = TypedData_Wrap_Struct(0, &sym_id_entry_list_type, entries);
-        /* Reachable from every Ractor via the global symbol table, so mark it
-         * shareable. */
-        RB_OBJ_SET_SHAREABLE(id_entry_list);
-        rb_ary_store(ids, (long)idx, id_entry_list);
+        bucket = TypedData_Wrap_Struct(0, &sym_id_entry_list_type, entries);
+        /* Reachable from every Ractor via the global symbol table, so mark it shareable. */
+        RB_OBJ_SET_SHAREABLE(bucket);
+        // Publish the (calloc-zeroed) bucket; release pairs with the reader's acquire load.
+        rbimpl_atomic_value_store(&dir->entries[idx], bucket, RBIMPL_ATOMIC_RELEASE);
+        RB_OBJ_WRITTEN(symbols->ids, Qundef, bucket);
     }
     else {
-        entries = RTYPEDDATA_GET_DATA(id_entry_list);
+        entries = RTYPEDDATA_GET_DATA(bucket);
     }
 
     idx = num % ID_ENTRY_UNIT;
@@ -292,8 +388,8 @@ set_id_entry(rb_symbols_t *symbols, rb_id_serial_t num, VALUE str, VALUE sym)
     RUBY_ASSERT(entry->str == 0);
     RUBY_ASSERT(entry->sym == 0);
 
-    RB_OBJ_WRITE(id_entry_list, &entry->str, str);
-    RB_OBJ_WRITE(id_entry_list, &entry->sym, sym);
+    RB_OBJ_WRITE(bucket, &entry->str, str);
+    RB_OBJ_WRITE(bucket, &entry->sym, sym);
 }
 
 static VALUE
@@ -419,7 +515,7 @@ Init_sym(void)
     rb_symbols_t *symbols = &ruby_global_symbols;
 
     symbols->sym_set = rb_concurrent_set_new(&sym_set_funcs, 1024);
-    symbols->ids = rb_ary_hidden_new(0);
+    symbols->ids = id_entry_dir_new(ID_ENTRY_DIR_INITIAL_CAPA);
 
     Init_op_tbl();
     Init_id();
@@ -765,19 +861,24 @@ get_id_serial_entry(rb_id_serial_t num)
 {
     struct sym_id_entry *entry = NULL;
 
-    GLOBAL_SYMBOLS_LOCKING(symbols) {
-        if (num && num < RUBY_ATOMIC_LOAD(symbols->next_id)) {
-            size_t idx = num / ID_ENTRY_UNIT;
-            VALUE ids = symbols->ids;
-            VALUE id_entry_list;
-            if (idx < (size_t)RARRAY_LEN(ids) && !NIL_P(id_entry_list = rb_ary_entry(ids, (long)idx))) {
-                rb_darray(struct sym_id_entry) entries = RTYPEDDATA_GET_DATA(id_entry_list);
+    if (num && num < RUBY_ATOMIC_LOAD(ruby_global_symbols.next_id)) {
+        size_t idx = num / ID_ENTRY_UNIT;
+
+        VALUE dir_obj = rbimpl_atomic_value_load(&ruby_global_symbols.ids, RBIMPL_ATOMIC_ACQUIRE);
+        struct id_entry_dir *dir = RTYPEDDATA_GET_DATA(dir_obj);
+
+        if (idx < (size_t)dir->capa) {
+            VALUE bucket = rbimpl_atomic_value_load(&dir->entries[idx], RBIMPL_ATOMIC_ACQUIRE);
+            if (bucket) {
+                rb_darray(struct sym_id_entry) entries = RTYPEDDATA_GET_DATA(bucket);
 
                 size_t pos = (size_t)(num % ID_ENTRY_UNIT);
                 RUBY_ASSERT(pos < rb_darray_size(entries));
                 entry = rb_darray_ref(entries, pos);
             }
         }
+
+        RB_GC_GUARD(dir_obj);
     }
 
     return entry;
@@ -852,6 +953,15 @@ sym_find(VALUE str)
     }
 }
 
+/* A dynamic symbol's id is assigned lazily by rb_sym2id, which performs a release store of
+ * id after set_id_entry has populated the serial->{str,sym} entry. */
+STATIC_ASSERT(value_id_same_width, sizeof(VALUE) == sizeof(ID));
+static inline ID
+sym_id_load_acquire(VALUE sym)
+{
+    return (ID)rbimpl_atomic_value_load((VALUE *)&RSYMBOL(sym)->id, RBIMPL_ATOMIC_ACQUIRE);
+}
+
 static ID
 lookup_str_id(VALUE str)
 {
@@ -865,7 +975,7 @@ lookup_str_id(VALUE str)
         return STATIC_SYM2ID(sym);
     }
     else if (DYNAMIC_SYM_P(sym)) {
-        ID id = RSYMBOL(sym)->id;
+        ID id = sym_id_load_acquire(sym);
         if (id & ~ID_SCOPE_MASK) return id;
     }
     else {
@@ -981,18 +1091,25 @@ rb_sym2id(VALUE sym)
         id = STATIC_SYM2ID(sym);
     }
     else if (DYNAMIC_SYM_P(sym)) {
+        id = sym_id_load_acquire(sym);
+        if (LIKELY(id & ~ID_SCOPE_MASK)) {
+            return id;
+        }
+
         GLOBAL_SYMBOLS_LOCKING(symbols) {
             RUBY_ASSERT(!rb_objspace_garbage_object_p(sym));
             id = RSYMBOL(sym)->id;
 
-            if (UNLIKELY(!(id & ~ID_SCOPE_MASK))) {
+            if (UNLIKELY(!(id & ~ID_SCOPE_MASK))) { // double-checked
                 VALUE fstr = RSYMBOL(sym)->fstr;
                 ID num = next_id_base();
+                id |= num;
 
-                RSYMBOL(sym)->id = id |= num;
-                /* make it permanent object */
-
+                /* Populate the serial->{str,sym} entry before publishing the pinned id below.
+                 * The publish is a release store paired with the acquire in sym_id_load_acquire
+                 * so a lock-free reader that observes the pinned id is guaranteed to see this entry. */
                 set_id_entry(symbols, rb_id_to_serial(num), fstr, sym);
+                rbimpl_atomic_value_store((VALUE *)&RSYMBOL(sym)->id, (VALUE)id, RBIMPL_ATOMIC_RELEASE);
             }
         }
     }
@@ -1173,8 +1290,9 @@ rb_check_id(volatile VALUE *namep)
         return STATIC_SYM2ID(name);
     }
     else if (DYNAMIC_SYM_P(name)) {
-        if (SYMBOL_PINNED_P(name)) {
-            return RSYMBOL(name)->id;
+        ID id = sym_id_load_acquire(name);
+        if (id & ~ID_SCOPE_MASK) {
+            return id;
         }
         else {
             *namep = RSYMBOL(name)->fstr;
@@ -1204,8 +1322,9 @@ rb_get_symbol_id(VALUE name)
         return STATIC_SYM2ID(name);
     }
     else if (DYNAMIC_SYM_P(name)) {
-        if (SYMBOL_PINNED_P(name)) {
-            return RSYMBOL(name)->id;
+        ID id = sym_id_load_acquire(name);
+        if (id & ~ID_SCOPE_MASK) {
+            return id;
         }
         else {
             return 0;