From 18a0354a6c7745cdc140b33f24220abf7f008938 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Sun, 12 Jul 2026 02:20:14 +0000 Subject: [PATCH 01/13] Remove a dying coroutine thread from the living set before its handoff coroutine_thread_terminated designated the successor first (thread_sched_to_dead_common) and only then removed the dying thread from the living set. The removal's VM-lock work (ractor_check_blocking and a possible barrier join) then ran detached: the thread had already left the running set while sched.running pointed to the successor. Such a joiner corrupts the successor's saved machine context (RB_VM_SAVE_MACHINE_CONTEXT captures the executing thread's state but the join parked cr->threads.sched.running), can sleep on an already-completed barrier until the NEXT one completes (a hang at process exit), is mis-counted in barrier_waiting_cnt, and races ractor_check_blocking's stale will-block decision. Do the removal first, while the thread still owns the Ractor's scheduler slot: the VM-lock work runs as an ordinary counted running thread and the whole class of detached races disappears. After the removal th may be unreachable, but no GC can complete while th still owns the slot (a barrier waits for it to join), so the short handoff tail may keep touching th/sched. The Ractor's last thread keeps the original reverse order: its removal unlinks the Ractor itself, so r/sched must not be touched afterwards. That order is safe only for it: with no successor, sched->running stays NULL and vm_need_barrier requires a running thread, so its removal's VM lock never joins a barrier in the first place. Co-Authored-By: Claude Opus 4.8 (1M context) --- thread_pthread_mn.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/thread_pthread_mn.c b/thread_pthread_mn.c index 422aae7b00034e..5ab33fa2c4c099 100644 --- a/thread_pthread_mn.c +++ b/thread_pthread_mn.c @@ -448,9 +448,8 @@ native_thread_check_and_create_shared(rb_vm_t *vm) } // A coroutine thread's epilogue, run from thread_start_func_2 while th is -// still valid (and, until the living-set removal, still GC-reachable). -// Everything that touches th happens here; co_start then only makes the -// final transfer, touching the execution-owned tctx alone. +// still valid. Everything that touches th happens here; co_start then only +// makes the final transfer, touching the execution-owned tctx alone. static void coroutine_thread_terminated(rb_thread_t *th) { @@ -470,6 +469,20 @@ coroutine_thread_terminated(rb_thread_t *th) rb_thread_t *wake_th; + // Leave the living set BEFORE handing over the scheduler slot: the + // removal's VM-lock work (ractor_check_blocking, a barrier join) then + // runs as an ordinary counted running thread. Afterwards th may be + // unreachable, but no GC can complete while th still owns the slot + // (a barrier waits for it to join), so the handoff below may keep + // touching th/sched. + // + // The Ractor's last thread is the exception and keeps the reverse + // order (below): its removal unlinks the Ractor itself, after which + // r/sched must not be touched. That order is safe only for it: with + // no successor, sched->running stays NULL, so the removal's VM lock + // never joins a barrier (vm_need_barrier requires a running thread). + if (!last) rb_ractor_living_threads_remove(r, th); + thread_sched_lock(sched, th); { // designate the successor (running = next from readyq, or NULL); for @@ -477,12 +490,12 @@ coroutine_thread_terminated(rb_thread_t *th) thread_sched_to_dead_common(sched, th); // Only the successor WE designated here may be enqueued by this - // epilogue (below, after the living-set removal). If readyq was - // empty, running is now NULL and a waker (e.g. the timer thread) - // that later installs a runnable thread enqueues the Ractor itself - // -- enqueuing "whatever is running" at that point would duplicate - // its entry. While running is non-NULL, nobody else re-assigns it, - // so wake_th stays valid until we enqueue. + // epilogue (below). If readyq was empty, running is now NULL and a + // waker (e.g. the timer thread) that later installs a runnable + // thread enqueues the Ractor itself -- enqueuing "whatever is + // running" at that point would duplicate its entry. While running + // is non-NULL, nobody else re-assigns it, so wake_th stays valid + // until we enqueue. wake_th = is_dnt ? NULL : sched->running; tctx->nt = th->nt; // stash the final transfer target for co_start @@ -491,14 +504,10 @@ coroutine_thread_terminated(rb_thread_t *th) } thread_sched_unlock(sched, th); - // Leave the living set. This is the dying thread's last access to th -- - // the removal needs the current ec (VM lock), and after it the GC may - // collect th (and, for a Ractor's main thread, the Ractor). - // (interrupt_lock stays valid up to here -- a concurrent terminate_all - // may interrupt any still-listed thread; it is destroyed in thread_free.) - rb_ractor_living_threads_remove(r, th); - if (last) { + // Last access to th/r: the removal may unlink the Ractor, after + // which the GC may collect th and r. + rb_ractor_living_threads_remove(r, th); rb_current_ec_set(NULL); // TLS only; r may be collectable already } else { From d6464c280ad5465690ddb96de0b7456ed5183ab3 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Sun, 12 Jul 2026 03:32:23 +0000 Subject: [PATCH 02/13] Count every Ractor barrier joiner again The is_running check added by 3fa1c8708 guarded against a terminating thread joining the barrier after leaving the running set. Since the previous commit such a thread leaves the living set before handing over its scheduler slot, so every joiner is a running-set member again. Restore the unconditional count and assert the invariant instead. Co-Authored-By: Claude Opus 4.8 (1M context) --- thread_pthread.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/thread_pthread.c b/thread_pthread.c index e0190093287976..6d47bf07d7a64f 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1664,16 +1664,13 @@ rb_ractor_sched_barrier_join(rb_vm_t *vm, rb_ractor_t *cr) ractor_sched_lock(vm, cr); { // running_cnt - /* A not-yet-running thread (blocking/terminating, joined only to - * sync) must not be counted, or barrier_waiting_cnt > running_cnt-1. */ - if (cr->threads.sched.is_running) { - vm->ractor.sched.barrier_waiting_cnt++; - RUBY_DEBUG_LOG("waiting_cnt:%u serial:%u", vm->ractor.sched.barrier_waiting_cnt, barrier_serial); - ractor_sched_barrier_join_signal_locked(vm); - } - else { - RUBY_DEBUG_LOG("join without counting (not running) serial:%u", barrier_serial); - } + /* Every joiner is a member of the running set: a dying thread now + * leaves the living set before handing over its scheduler slot. */ + VM_ASSERT(ractor_sched_running_threads_contain_p(vm, GET_THREAD())); + vm->ractor.sched.barrier_waiting_cnt++; + RUBY_DEBUG_LOG("waiting_cnt:%u serial:%u", vm->ractor.sched.barrier_waiting_cnt, barrier_serial); + + ractor_sched_barrier_join_signal_locked(vm); ractor_sched_barrier_join_wait_locked(vm, cr->threads.sched.running); } ractor_sched_unlock(vm, cr); From 7c6a30883e0c89133dfa97016ea1abd8b8f4ea9e Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Sat, 11 Jul 2026 08:11:15 +0000 Subject: [PATCH 03/13] Add a stress test for a thread terminating during a compaction barrier Exercises a short-lived thread terminating while another Ractor drives GC.compact barriers, the scenario fixed by the previous commits. Skipped when the GC does not implement compaction (e.g. MMTk). Co-Authored-By: Claude Opus 4.8 (1M context) --- bootstraptest/test_ractor.rb | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 22d2ee47deb1ce..77d15985bce0ed 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -2620,3 +2620,31 @@ def foo(a:, b:, c:) = super(a: a, b: b, c: c) :ok } + +# A thread terminating (here: the pipe writer) while another Ractor runs a +# compaction barrier must not corrupt the machine context of a thread blocked +# in IO, whose locked read buffer lives only on that machine stack. +assert_equal 'ok', %q{ + Warning[:experimental] = false + can_compact = begin + GC.compact + true + rescue NotImplementedError + false + end + if can_compact + b = Ractor.new do + 10.times do + r, w = IO.pipe + t = Thread.new { w.write("x" * 40); w.close } + r.read + r.close + t.join + end + :ok + end + a = Ractor.new { 20.times { GC.compact }; :ok } + [a, b].each(&:value) + end + :ok +} From c2992bbd7aa769395c9fa9ac68a8761d1db39d29 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Mon, 13 Jul 2026 09:44:45 +0000 Subject: [PATCH 04/13] Reset the Ractor barrier state in the child after fork A fork taken while the parent holds the VM barrier (e.g. rb_gc_before_fork) leaves the child with barrier_waiting set and a barrier owner that no longer exists, so the next barrier in the child could never complete. Reset the barrier fields in thread_sched_atfork like the rest of the scheduler state. Co-Authored-By: Claude Opus 4.8 (1M context) --- thread_pthread.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/thread_pthread.c b/thread_pthread.c index 6d47bf07d7a64f..d2fed2764c44e1 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1744,6 +1744,12 @@ thread_sched_atfork(struct rb_thread_sched *sched) ccan_list_head_init(&vm->ractor.sched.grq); vm->ractor.sched.grq_cnt = 0; // the list was just emptied; reset the count with it + // A fork during a VM barrier leaves the child with barrier state that can + // never complete (the other ractors are gone); reset it like the rest. + vm->ractor.sched.barrier_waiting = false; + vm->ractor.sched.barrier_waiting_cnt = 0; + vm->ractor.sched.barrier_ractor = NULL; + vm->ractor.sched.barrier_lock_rec = 0; // Threads that were winding down in the parent do not exist in the child; // without this reset the child's ruby_vm_destruct would wait for their // reclaim (which never comes) forever. From 6a098a07e525ce89b816d5900117bfe137034a09 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Mon, 13 Jul 2026 09:48:09 +0000 Subject: [PATCH 05/13] Assert the serial-epilogue ordering invariants Encode the safety argument of the epilogue reorder as VM_ASSERTs: a non-last dying thread still owns its scheduler slot when it leaves the living set (its VM-lock work stays a counted running thread), and the last thread's reverse order is only safe because it has no successor (running == NULL, so the removal cannot join a barrier). Also check winding_cnt against underflow at the reclaim decrement. Co-Authored-By: Claude Fable 5 --- thread_pthread.c | 1 + thread_pthread_mn.c | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/thread_pthread.c b/thread_pthread.c index d2fed2764c44e1..d6e27950570942 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -2503,6 +2503,7 @@ thread_sched_reclaim(struct coroutine_context *dead_co) SIZED_FREE(tctx); // pairs with the increment at the top of coroutine_thread_terminated: // a waiting VM destruct may proceed once this reclaim is done + VM_ASSERT(RUBY_ATOMIC_LOAD(GET_VM()->ractor.sched.winding_cnt) > 0); RUBY_ATOMIC_DEC(GET_VM()->ractor.sched.winding_cnt); return true; } diff --git a/thread_pthread_mn.c b/thread_pthread_mn.c index 5ab33fa2c4c099..2f94aa248b0743 100644 --- a/thread_pthread_mn.c +++ b/thread_pthread_mn.c @@ -481,6 +481,7 @@ coroutine_thread_terminated(rb_thread_t *th) // r/sched must not be touched. That order is safe only for it: with // no successor, sched->running stays NULL, so the removal's VM lock // never joins a barrier (vm_need_barrier requires a running thread). + VM_ASSERT(sched->running == th); // th owns the slot through the removal if (!last) rb_ractor_living_threads_remove(r, th); thread_sched_lock(sched, th); @@ -505,6 +506,10 @@ coroutine_thread_terminated(rb_thread_t *th) thread_sched_unlock(sched, th); if (last) { + // The reverse order is safe only with no successor: running == NULL + // means the removal's VM lock cannot join a barrier (vm_need_barrier). + VM_ASSERT(sched->running == NULL); + VM_ASSERT(wake_th == NULL); // Last access to th/r: the removal may unlink the Ractor, after // which the GC may collect th and r. rb_ractor_living_threads_remove(r, th); From fc6ff6fbcc9d6a7b3b2f091790898204a5517c6d Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Tue, 14 Jul 2026 00:19:43 +0000 Subject: [PATCH 06/13] ractor: guard the port VALUE across receive/send ractor_port_receive/send derive a raw 'struct ractor_port *rp' from self's embedded (RUBY_TYPED_EMBEDDABLE) data and keep using it across a GC safepoint: receive parks, and send allocates the basket. self is only kept alive movably (rb_gc_mark_movable via the VM stack), so a concurrent GC.compact can relocate the port and leave rp dangling -- the receiver then polls a stale, reused slot's port id forever (a hang of Ractor#value under IO + GC.compact). RB_GC_GUARD(self) keeps self on the machine stack, whose conservative scan pins the port so it cannot move while rp is live. --- ractor_sync.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ractor_sync.c b/ractor_sync.c index 5dfb86e3bff130..5b513a1315d831 100644 --- a/ractor_sync.c +++ b/ractor_sync.c @@ -126,7 +126,9 @@ ractor_port_receive(rb_execution_context_t *ec, VALUE self) rb_raise(rb_eRactorError, "only allowed from the creator Ractor of this port"); } - return ractor_receive(ec, rp); + VALUE v = ractor_receive(ec, rp); + RB_GC_GUARD(self); + return v; } static VALUE @@ -134,6 +136,7 @@ ractor_port_send(rb_execution_context_t *ec, VALUE self, VALUE obj, VALUE move) { const struct ractor_port *rp = RACTOR_PORT_PTR(self); ractor_send(ec, rp, obj, RTEST(move)); + RB_GC_GUARD(self); return self; } From d418ff687054ee1ff90ff81d97cf45e22d153331 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Tue, 14 Jul 2026 06:33:49 +0000 Subject: [PATCH 07/13] Add a test for forking while other Ractors are alive Forking with live Ractors takes a VM barrier in the parent and the child inherits scheduler/barrier state that must be reset. General regression coverage for the parent-side fork barrier and the child teardown, checking the surviving Ractors still work afterwards. (Uses GC.start, not GC.compact, so it also runs on GCs without compaction such as MMTk.) --- bootstraptest/test_ractor.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 77d15985bce0ed..f8c3573e1f24b5 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -2648,3 +2648,23 @@ def foo(a:, b:, c:) = super(a: a, b: b, c: c) end :ok } + +# Forking while other Ractors are alive takes a VM barrier in the parent; the +# child inherits that scheduler/barrier state and must reset it. Exercises the +# parent-side fork barrier and the child teardown, then checks the surviving +# Ractors still work. +assert_equal 'ok', %q{ + begin + rs = 5.times.map { Ractor.new { Ractor.receive } } + 10.times do + pid = fork { GC.start } + _, status = Process.waitpid2(pid) + raise "child failed" unless status.success? + end + rs.each { |r| r.send(nil) } + rs.each(&:value) + :ok + rescue NotImplementedError + :ok # platform without fork + end +} From e0083cffa31ae58f6326fb7a62199f4e59d86e87 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 13 Jul 2026 11:32:10 +0200 Subject: [PATCH 08/13] Refactor RObject to use T_IMEMO/fields_obj on overflow Instead of spilling into a raw buffer, `RObject` now spills into an `IMEMO/fields` object like other types. From an instance variable layout standpoint, an extended `RObject` is now identical to a `RTypedData`, as in they both store the reference to their `IMEMO/fields` at the same offset (`VALUE * 2`). One positive consequence of this is that the only case where a `T_OBJECT` needs sweeping is if a finalizer was registered. YJIT now side exit when it need to write a new ivar into a `RObject` that is out of space. This is unlikely to cause a performance regression as this codepath isn't supposed to happen after warmup given `max_iv_count` is recorded on classes, so future objects should be large enough. Hence it's best not to waste executable memory for such codepath. ZJIT lost support for writting into extended `RObject`. Most of the code to support it is there it's just missing an implementation of `RBASIC_SET_SHAPE_ID`, which recently changed to strip some bits out of the shape (see comments). I will leave it to the ZJIT team to implement it (sorry). There are a few future cleanups planned that I keep for followups: - We can now get rid of the `ROBJECT_HEAP` flag, it's redundant with the shape layout bits. - `imemo_fields` can now embed the `st_table` when complex. Co-Authored-By: John Hawthorn Co-authored-by: Randy Stauner --- gc.c | 86 ++++---- imemo.c | 9 - include/ruby/internal/core/robject.h | 26 +-- internal/imemo.h | 25 ++- internal/object.h | 30 ++- internal/variable.h | 1 - jit.c | 2 +- object.c | 13 +- shape.c | 29 ++- shape.h | 55 +++-- .../library/objectspace/memsize_of_spec.rb | 18 +- test/objspace/test_objspace.rb | 5 + test/ruby/test_shapes.rb | 4 +- variable.c | 208 ++++++++++-------- vm_eval.c | 1 + vm_insnhelper.c | 8 +- yjit/src/codegen.rs | 134 +++++------ yjit/src/cruby_bindings.inc.rs | 1 - zjit/src/cruby.rs | 2 +- zjit/src/cruby_bindings.inc.rs | 3 +- zjit/src/hir.rs | 62 +++--- zjit/src/hir/opt_tests.rs | 12 +- 22 files changed, 402 insertions(+), 332 deletions(-) diff --git a/gc.c b/gc.c index 4bdc164347271b..650e4ea4ff0a57 100644 --- a/gc.c +++ b/gc.c @@ -376,7 +376,17 @@ rb_gc_shutdown_call_finalizer_p(VALUE obj) void rb_gc_obj_changed_slot_size(VALUE obj, size_t slot_size) { - RBASIC_SET_FULL_SHAPE_ID(obj, rb_obj_shape_transition_slot_size(obj, slot_size)); + shape_id_t shape_id = rb_obj_shape_transition_capacity(obj, rb_shape_capacity_for_slot_size(slot_size)); + + if (RB_TYPE_P(obj, T_OBJECT) && FL_TEST_RAW(obj, ROBJECT_HEAP) && !rb_obj_shape_complex_p(obj)) { + VALUE *embedded_fields = ROBJECT_EMBEDDED_FIELDS(obj); + VALUE *extended_fields = ROBJECT_FIELDS(obj); + MEMCPY(embedded_fields, extended_fields, VALUE, RSHAPE_LEN(RBASIC_SHAPE_ID(obj))); + FL_UNSET_RAW(obj, ROBJECT_HEAP); + shape_id = rb_shape_transition_robject(shape_id); + } + + RBASIC_SET_FULL_SHAPE_ID(obj, shape_id); } void rb_vm_update_references(void *ptr); @@ -1584,19 +1594,6 @@ rb_gc_obj_free(void *objspace, VALUE obj) switch (builtin_type) { case T_OBJECT: - if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { - if (rb_obj_shape_complex_p(obj)) { - RB_DEBUG_COUNTER_INC(obj_obj_complex); - st_free_table(ROBJECT_FIELDS_HASH(obj)); - } - else { - SIZED_FREE_N(ROBJECT(obj)->as.heap.fields, ROBJECT_FIELDS_CAPACITY(obj)); - RB_DEBUG_COUNTER_INC(obj_obj_ptr); - } - } - else { - RB_DEBUG_COUNTER_INC(obj_obj_embed); - } break; case T_MODULE: case T_CLASS: @@ -2372,14 +2369,6 @@ rb_obj_memsize_of(VALUE obj) switch (BUILTIN_TYPE(obj)) { case T_OBJECT: - if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { - if (rb_obj_shape_complex_p(obj)) { - size += rb_st_memsize(ROBJECT_FIELDS_HASH(obj)); - } - else { - size += ROBJECT_FIELDS_CAPACITY(obj) * sizeof(VALUE); - } - } break; case T_MODULE: case T_CLASS: @@ -3303,12 +3292,13 @@ rb_gc_mark_children(void *objspace, VALUE obj) case T_OBJECT: { uint32_t len; - if (rb_obj_shape_complex_p(obj)) { - gc_mark_tbl_no_pin(ROBJECT_FIELDS_HASH(obj)); - len = ROBJECT_FIELDS_COUNT_COMPLEX(obj); + if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { + if (!rb_gc_checking_shareable()) { + gc_mark_internal(ROBJECT(obj)->as.extended); + } } else { - const VALUE * const ptr = ROBJECT_FIELDS(obj); + const VALUE * const ptr = ROBJECT(obj)->as.ary; len = ROBJECT_FIELDS_COUNT_NOT_COMPLEX(obj); for (uint32_t i = 0; i < len; i++) { @@ -3678,27 +3668,14 @@ gc_ref_update_array(void *objspace, VALUE v) static void gc_ref_update_object(void *objspace, VALUE v) { - VALUE *ptr = ROBJECT_FIELDS(v); - if (FL_TEST_RAW(v, ROBJECT_HEAP)) { - if (rb_obj_shape_complex_p(v)) { - gc_ref_update_table_values_only(ROBJECT_FIELDS_HASH(v)); - return; - } - - size_t slot_size = rb_gc_obj_slot_size(v); - size_t embed_size = rb_obj_embedded_size(ROBJECT_FIELDS_CAPACITY(v)); - if (slot_size >= embed_size) { - // Object can be re-embedded - memcpy(ROBJECT(v)->as.ary, ptr, sizeof(VALUE) * ROBJECT_FIELDS_COUNT(v)); - SIZED_FREE_N(ptr, ROBJECT_FIELDS_CAPACITY(v)); - FL_UNSET_RAW(v, ROBJECT_HEAP); - ptr = ROBJECT(v)->as.ary; - } + UPDATE_IF_MOVED(objspace, ROBJECT(v)->as.extended); } - - for (uint32_t i = 0; i < ROBJECT_FIELDS_COUNT(v); i++) { - UPDATE_IF_MOVED(objspace, ptr[i]); + else { + VALUE *ptr = ROBJECT_FIELDS(v); + for (uint32_t i = 0; i < ROBJECT_FIELDS_COUNT(v); i++) { + UPDATE_IF_MOVED(objspace, ptr[i]); + } } } @@ -4860,11 +4837,11 @@ rb_raw_obj_info_buitin_type(char *const buff, const size_t buff_size, const VALU APPEND_F("(complex) len:%zu", hash_len); } else { - APPEND_F("(embed) len:%d capa:%d", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj)); + APPEND_F("len:%d capa:%d extended:%p", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj), (void *)ROBJECT_FIELDS_OBJ(obj)); } } else { - APPEND_F("len:%d capa:%d ptr:%p", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj), (void *)ROBJECT_FIELDS(obj)); + APPEND_F("(embed) len:%d capa:%d", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj)); } } break; @@ -4889,6 +4866,21 @@ rb_raw_obj_info_buitin_type(char *const buff, const size_t buff_size, const VALU APPEND_F("<%s> ", rb_imemo_name(imemo_type(obj))); switch (imemo_type(obj)) { + case imemo_fields: + { + if (rb_obj_shape_complex_p(obj)) { + size_t hash_len = rb_st_table_size(rb_imemo_fields_complex_tbl(obj)); + APPEND_F("(complex) len:%zu", hash_len); + } + else { + APPEND_F("(embed) len:%d capa:%d", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj)); + } + + APPEND_S("owner -> "); + rb_raw_obj_info(BUFF_ARGS, CLASS_OF(obj)); + + break; + } case imemo_ment: { const rb_method_entry_t *me = (const rb_method_entry_t *)obj; diff --git a/imemo.c b/imemo.c index a70ae1a5523fcd..06a94d0d19a628 100644 --- a/imemo.c +++ b/imemo.c @@ -151,7 +151,6 @@ rb_imemo_fields_new(VALUE owner, shape_id_t shape_id, bool shareable) size_t embedded_size = offsetof(struct rb_fields, as.embed) + capa * sizeof(VALUE); VALUE fields = imemo_fields_new(owner, 0, shape_id, embedded_size, shareable); - RUBY_ASSERT(IMEMO_TYPE_P(fields, imemo_fields)); RUBY_ASSERT(rb_shape_embedded_capacity(RBASIC_SHAPE_ID(fields)) >= capa); @@ -223,14 +222,6 @@ rb_imemo_fields_clone(VALUE fields_obj) void rb_imemo_fields_clear(VALUE fields_obj) { - // When replacing an imemo/fields by another one, we must clear - // its shape so that gc.c:obj_free_object_id won't be called. - if (rb_obj_shape_complex_p(fields_obj)) { - RBASIC_SET_SHAPE_ID(fields_obj, ROOT_COMPLEX_SHAPE_ID); - } - else { - RBASIC_SET_SHAPE_ID(fields_obj, ROOT_SHAPE_ID); - } // Invalidate the ec->gen_fields_cache. RBASIC_CLEAR_CLASS(fields_obj); } diff --git a/include/ruby/internal/core/robject.h b/include/ruby/internal/core/robject.h index b8743dcb739116..c8917c0f8e2b19 100644 --- a/include/ruby/internal/core/robject.h +++ b/include/ruby/internal/core/robject.h @@ -89,23 +89,8 @@ struct RObject { /** Object's specific fields. */ union { - - /** - * Object that use separated memory region for instance variables use - * this pattern. - */ - struct { - /** Pointer to a C array that holds instance variables. */ - VALUE *fields; - } heap; - - /* When an object is too complex, it uses a st_table to store instance - * variable name to value mappings. - */ - st_table *hash; - - /* Embedded instance variables. When an object is small enough, it - * uses this area to store the instance variables. + /* Embedded instance variables. When an object slot is large enough, it + * uses this area to store the instance variables embedded. * * This is a length 1 array because: * 1. GCC has a bug that does not optimize C flexible array members @@ -113,6 +98,13 @@ struct RObject { * 2. Zero length arrays are not supported by all compilers */ VALUE ary[1]; + + /** + * When an object slot is too small or too complex to store instance + * variables inline, it references another, larger, object that contains + * the instance variables. + */ + VALUE extended; } as; }; diff --git a/internal/imemo.h b/internal/imemo.h index 7562794506f3d6..a66ed1161b424b 100644 --- a/internal/imemo.h +++ b/internal/imemo.h @@ -245,11 +245,7 @@ struct rb_fields { VALUE fields[1]; } embed; struct { - VALUE *ptr; - } external; - struct { - // Note: the st_table could be embedded, but complex T_CLASS should be rare to - // non-existent, so not really worth the trouble. + // TODO: the st_table should be embedded st_table *table; } complex; } as; @@ -260,8 +256,6 @@ struct rb_fields { #define OBJ_FIELD_HEAP ROBJECT_HEAP STATIC_ASSERT(imemo_fields_flags, OBJ_FIELD_HEAP == IMEMO_FL_USER0); STATIC_ASSERT(imemo_fields_embed_offset, offsetof(struct RObject, as.ary) == offsetof(struct rb_fields, as.embed.fields)); -STATIC_ASSERT(imemo_fields_external_offset, offsetof(struct RObject, as.heap.fields) == offsetof(struct rb_fields, as.external.ptr)); -STATIC_ASSERT(imemo_fields_complex_offset, offsetof(struct RObject, as.heap.fields) == offsetof(struct rb_fields, as.complex.table)); #define IMEMO_OBJ_FIELDS(fields) ((struct rb_fields *)fields) @@ -309,11 +303,18 @@ rb_imemo_fields_ptr(VALUE fields_obj) RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields) || RB_TYPE_P(fields_obj, T_OBJECT)); if (UNLIKELY(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP))) { - return IMEMO_OBJ_FIELDS(fields_obj)->as.external.ptr; - } - else { - return IMEMO_OBJ_FIELDS(fields_obj)->as.embed.fields; + if (RB_TYPE_P(fields_obj, T_OBJECT)) { + fields_obj = ROBJECT(fields_obj)->as.extended; + } + RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields)); + + // This will go away once we embed the `st_table` inside `IMEMO/fields`. + if (UNLIKELY(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP))) { + return (VALUE *)IMEMO_OBJ_FIELDS(fields_obj)->as.complex.table; + } } + + return IMEMO_OBJ_FIELDS(fields_obj)->as.embed.fields; } static inline st_table * @@ -323,7 +324,7 @@ rb_imemo_fields_complex_tbl(VALUE fields_obj) return NULL; } - RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields) || RB_TYPE_P(fields_obj, T_OBJECT)); + RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields)); RUBY_ASSERT(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP)); // Some codepaths unconditionally access the fields_ptr, and assume it can be used as st_table if the diff --git a/internal/object.h b/internal/object.h index 7538d46b8b343e..2c531186ba3d75 100644 --- a/internal/object.h +++ b/internal/object.h @@ -68,19 +68,35 @@ RBASIC_SET_CLASS(VALUE obj, VALUE klass) RB_OBJ_WRITTEN(obj, oldv, klass); } +static inline VALUE +ROBJECT_FIELDS_OBJ(VALUE obj) +{ + RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); + + return FL_TEST_RAW(obj, ROBJECT_HEAP) ? ROBJECT(obj)->as.extended : obj; +} + +static inline VALUE * +ROBJECT_EMBEDDED_FIELDS(VALUE obj) +{ + return ROBJECT(obj)->as.ary; +} + static inline VALUE * ROBJECT_FIELDS(VALUE obj) { RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); - struct RObject *const ptr = ROBJECT(obj); + return ROBJECT_EMBEDDED_FIELDS(ROBJECT_FIELDS_OBJ(obj)); +} - if (RB_UNLIKELY(RB_FL_ANY_RAW(obj, ROBJECT_HEAP))) { - return ptr->as.heap.fields; - } - else { - return ptr->as.ary; - } +static inline void +ROBJECT_SET_EXTENDED(VALUE obj, VALUE fields_obj) +{ + RUBY_ASSERT(RB_TYPE_P(obj, T_OBJECT)); + RUBY_ASSERT(RB_TYPE_P(fields_obj, T_IMEMO)); + RB_OBJ_WRITE(obj, &ROBJECT(obj)->as.extended, fields_obj); + FL_SET_RAW(obj, ROBJECT_HEAP); } static inline size_t diff --git a/internal/variable.h b/internal/variable.h index 58364941c12156..70142815e89710 100644 --- a/internal/variable.h +++ b/internal/variable.h @@ -71,6 +71,5 @@ VALUE rb_gvar_get(ID); VALUE rb_gvar_set(ID, VALUE); VALUE rb_gvar_defined(ID); void rb_const_warn_if_deprecated(const rb_const_entry_t *, VALUE, ID); -void rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t newsize); #endif /* INTERNAL_VARIABLE_H */ diff --git a/jit.c b/jit.c index 534a07c848abfd..377ab3fe16177f 100644 --- a/jit.c +++ b/jit.c @@ -29,7 +29,7 @@ enum jit_bindgen_constants { // Field offsets for the RObject struct - ROBJECT_OFFSET_AS_HEAP_FIELDS = offsetof(struct RObject, as.heap.fields), + ROBJECT_OFFSET_AS_HEAP_FIELDS = offsetof(struct RObject, as.extended), ROBJECT_OFFSET_AS_ARY = offsetof(struct RObject, as.ary), // Field offset for prime classext's fields_obj from a class pointer diff --git a/object.c b/object.c index 919a2fd2e7cdec..3b382bcc5c0a7b 100644 --- a/object.c +++ b/object.c @@ -363,12 +363,17 @@ rb_obj_copy_ivar(VALUE dest, VALUE obj) RUBY_ASSERT(src_num_ivs <= dest_capa); if (initial_capa < dest_capa) { - rb_ensure_iv_list_size(dest, 0, dest_capa); + // We we need to transition the object to an extended layout. + VALUE fields_obj = rb_imemo_fields_new(dest, dest_shape_id, false); + ROBJECT_SET_EXTENDED(dest, fields_obj); dest_buf = ROBJECT_FIELDS(dest); + rb_shape_copy_fields(dest, dest_buf, dest_shape_id, src_buf, src_shape_id); + RBASIC_SET_SHAPE_ID_WITH_LAYOUT(dest, dest_shape_id, SHAPE_ID_LAYOUT_EXTENDED); + } + else { + rb_shape_copy_fields(dest, dest_buf, dest_shape_id, src_buf, src_shape_id); + RBASIC_SET_SHAPE_ID(dest, dest_shape_id); } - - rb_shape_copy_fields(dest, dest_buf, dest_shape_id, src_buf, src_shape_id); - RBASIC_SET_SHAPE_ID(dest, dest_shape_id); } static void diff --git a/shape.c b/shape.c index 7bbf1e0328d380..8c1955fc939c43 100644 --- a/shape.c +++ b/shape.c @@ -1224,7 +1224,7 @@ rb_shape_expected_layout(VALUE obj) { switch (BUILTIN_TYPE(obj)) { case T_OBJECT: - return SHAPE_ID_LAYOUT_ROBJECT; + return FL_TEST_RAW(obj, ROBJECT_HEAP) ? SHAPE_ID_LAYOUT_EXTENDED : SHAPE_ID_LAYOUT_ROBJECT; case T_CLASS: case T_MODULE: if (FL_TEST_RAW(obj, RCLASS_BOXABLE)) { @@ -1232,7 +1232,7 @@ rb_shape_expected_layout(VALUE obj) } return SHAPE_ID_LAYOUT_RCLASS; case T_DATA: - return SHAPE_ID_LAYOUT_RDATA; + return SHAPE_ID_LAYOUT_EXTENDED; case T_IMEMO: if (IMEMO_TYPE_P(obj, imemo_fields)) { return SHAPE_ID_LAYOUT_ROBJECT; @@ -1243,6 +1243,23 @@ rb_shape_expected_layout(VALUE obj) } } +static const char * +shape_layout_name(shape_id_t shape_id) +{ + switch (rb_shape_layout(shape_id)) { + case SHAPE_ID_LAYOUT_ROBJECT: + return "robject"; + case SHAPE_ID_LAYOUT_RCLASS: + return "rclass"; + case SHAPE_ID_LAYOUT_EXTENDED: + return "extended (or RData)"; + case SHAPE_ID_LAYOUT_OTHER: + return "other"; + default: + return "invalid"; + } +} + bool rb_shape_verify_capacity_consistency_p(VALUE obj) { @@ -1268,8 +1285,8 @@ rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id) shape_id_t actual_layout = rb_shape_layout(rb_obj_shape_id(obj)); shape_id_t expected_layout = rb_shape_expected_layout(obj); if (actual_layout != expected_layout) { - rb_bug("shape_id layout mismatch: expected=%x actual=%x shape_id=%u obj=%s", - expected_layout, actual_layout, shape_id, rb_obj_info(obj)); + rb_bug("shape_id layout mismatch: expected=%s actual=%s shape_id=%u obj=%s", + shape_layout_name(expected_layout), shape_layout_name(actual_layout), shape_id, rb_obj_info(obj)); } if (shape_id == ROOT_SHAPE_ID) { @@ -1373,8 +1390,8 @@ shape_layout(VALUE self) return ID2SYM(rb_intern("robject")); case SHAPE_ID_LAYOUT_RCLASS: return ID2SYM(rb_intern("rclass")); - case SHAPE_ID_LAYOUT_RDATA: - return ID2SYM(rb_intern("rdata")); + case SHAPE_ID_LAYOUT_EXTENDED: + return ID2SYM(rb_intern("extended_or_rdata")); case SHAPE_ID_LAYOUT_OTHER: return ID2SYM(rb_intern("other")); default: diff --git a/shape.h b/shape.h index 86f98b670782fb..51ebf4f72d762f 100644 --- a/shape.h +++ b/shape.h @@ -35,6 +35,8 @@ STATIC_ASSERT(shape_id_num_bits, SHAPE_ID_NUM_BITS == sizeof(shape_id_t) * CHAR_ // 29-30 SHAPE_ID_LAYOUT_MASK // The object's physical field layout. +STATIC_ASSERT(robject_rdata_fields_offset, offsetof(struct RObject, as.extended) == offsetof(struct RTypedData, fields_obj)); + enum shape_id_fl_type { #define RBIMPL_SHAPE_ID_FL(n) (1<<(SHAPE_ID_FL_USHIFT+n)) @@ -52,19 +54,25 @@ enum shape_id_fl_type { // are found in the fields_obj found on the rclass struct SHAPE_ID_LAYOUT_RCLASS = RBIMPL_SHAPE_ID_FL(3), - // Means this object is an RData or RTypedData and IVs are found in the - // fields_obj found on the RData/RTypedData struct - SHAPE_ID_LAYOUT_RDATA = RBIMPL_SHAPE_ID_FL(4), + // Means this object is an extened RObject or a RTypedData and IVs are found in the + // fields_obj found on the RObject/RTypedData struct at offset `sizeof(VALUE) * 2`. + SHAPE_ID_LAYOUT_EXTENDED = RBIMPL_SHAPE_ID_FL(4), + SHAPE_ID_LAYOUT_RDATA = SHAPE_ID_LAYOUT_EXTENDED, // Means this is a complicated object: boxable classes, structs, objects // that store IVs on the geniv table - SHAPE_ID_LAYOUT_OTHER = SHAPE_ID_LAYOUT_RCLASS | SHAPE_ID_LAYOUT_RDATA, + SHAPE_ID_LAYOUT_OTHER = SHAPE_ID_LAYOUT_RCLASS | SHAPE_ID_LAYOUT_EXTENDED, SHAPE_ID_LAYOUT_MASK = SHAPE_ID_LAYOUT_OTHER, SHAPE_ID_FL_NON_CANONICAL_MASK = SHAPE_ID_FL_FROZEN | SHAPE_ID_FL_HAS_OBJECT_ID, SHAPE_ID_FLAGS_MASK = SHAPE_ID_CAPACITY_MASK | SHAPE_ID_FL_NON_CANONICAL_MASK | SHAPE_ID_FL_COMPLEX | SHAPE_ID_LAYOUT_MASK, + // These parts of the shape id are specific to the object. + // Typically, when replicating a shape transition from an object to + // its IMEMO/fields, these bits should be stripped. + // All other bits are shared between an IMEMO/fields and its owner. + SHAPE_ID_FL_PRIVATE_MASK = SHAPE_ID_LAYOUT_MASK|SHAPE_ID_CAPACITY_MASK, #undef RBIMPL_SHAPE_ID_FL }; @@ -201,16 +209,27 @@ RBASIC_SET_FULL_SHAPE_ID(VALUE obj, shape_id_t shape_id) RUBY_ASSERT(rb_shape_verify_consistency(obj, shape_id)); } +static inline shape_id_t rb_shape_transition_layout(shape_id_t, shape_id_t); + +static inline void +RBASIC_SET_SHAPE_ID_WITH_LAYOUT(VALUE obj, shape_id_t target_shape_id, shape_id_t layout) +{ + RUBY_ASSERT((layout & SHAPE_ID_LAYOUT_MASK) == layout); + shape_id_t current_shape_id = RBASIC_SHAPE_ID(obj); + current_shape_id = rb_shape_transition_layout(current_shape_id, layout); + current_shape_id = (current_shape_id & SHAPE_ID_FL_PRIVATE_MASK) | (target_shape_id & ~SHAPE_ID_FL_PRIVATE_MASK); + RBASIC_SET_FULL_SHAPE_ID(obj, current_shape_id); +} + static inline void RBASIC_SET_SHAPE_ID(VALUE obj, shape_id_t shape_id) { RUBY_ASSERT(!RB_SPECIAL_CONST_P(obj)); - shape_id = ( - (shape_id & ~(SHAPE_ID_CAPACITY_MASK|SHAPE_ID_LAYOUT_MASK)) | - (RBASIC_SHAPE_ID(obj) & (SHAPE_ID_CAPACITY_MASK|SHAPE_ID_LAYOUT_MASK)) - ); - RBASIC_SET_FULL_SHAPE_ID(obj, shape_id); + RBASIC_SET_FULL_SHAPE_ID(obj, ( + (shape_id & ~SHAPE_ID_FL_PRIVATE_MASK) | + (RBASIC_SHAPE_ID(obj) & SHAPE_ID_FL_PRIVATE_MASK) + )); } static inline shape_id_t @@ -380,17 +399,7 @@ ROBJECT_FIELDS_HASH(VALUE obj) RUBY_ASSERT(rb_obj_shape_complex_p(obj)); RUBY_ASSERT(FL_TEST_RAW(obj, ROBJECT_HEAP)); - return ROBJECT(obj)->as.hash; -} - -static inline void -ROBJECT_SET_FIELDS_HASH(VALUE obj, st_table *tbl) -{ - RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); - RUBY_ASSERT(rb_obj_shape_complex_p(obj)); - RUBY_ASSERT(FL_TEST_RAW(obj, ROBJECT_HEAP)); - - ROBJECT(obj)->as.hash = tbl; + return rb_imemo_fields_complex_tbl(ROBJECT(obj)->as.extended); } static inline uint32_t @@ -539,6 +548,12 @@ rb_shape_transition_slot_size(shape_id_t shape_id, size_t slot_size) return rb_shape_transition_capacity(shape_id, rb_shape_capacity_for_slot_size(slot_size)); } +static inline shape_id_t +rb_shape_transition_layout(shape_id_t shape_id, shape_id_t layout) +{ + return (shape_id & (~SHAPE_ID_LAYOUT_MASK)) | layout; +} + shape_id_t rb_shape_transition_object_id(shape_id_t shape_id); static inline shape_id_t diff --git a/spec/ruby/library/objectspace/memsize_of_spec.rb b/spec/ruby/library/objectspace/memsize_of_spec.rb index 9c8aea4e84ea0a..a99d69d17032f3 100644 --- a/spec/ruby/library/objectspace/memsize_of_spec.rb +++ b/spec/ruby/library/objectspace/memsize_of_spec.rb @@ -23,12 +23,20 @@ end it "is larger if the Object has more instance variables" do - obj = Object.new - before = ObjectSpace.memsize_of(obj) - 100.times do |i| - obj.instance_variable_set(:"@foo#{i}", nil) + before = ObjectSpace.memsize_of(Object.new) + + klass = Class.new do + set_ivar = 100.times.map { |i| "@foo#{i} = nil" } + class_eval(<<~RUBY, __FILE__, __LINE__ + 1) + def initialize + #{set_ivar.join("; ")} + end + RUBY end - after = ObjectSpace.memsize_of(obj) + + klass.new # in case the runtime needs warmup + + after = ObjectSpace.memsize_of(klass.new) after.should > before end end diff --git a/test/objspace/test_objspace.rb b/test/objspace/test_objspace.rb index 84cd88659ee2f0..403e98e6e1608d 100644 --- a/test/objspace/test_objspace.rb +++ b/test/objspace/test_objspace.rb @@ -895,6 +895,11 @@ def test_name_error_message bar rescue => err _, m = ObjectSpace.reachable_objects_from(err) + + # The very first NameError allocated by a process is extended + if ObjectSpace::InternalObjectWrapper === m # T_IMEMO/fields_obj + m, _ = ObjectSpace.reachable_objects_from(m) + end end assert_equal(m, m.clone) end diff --git a/test/ruby/test_shapes.rb b/test/ruby/test_shapes.rb index cbf4eebfa37a8a..8dc52eee54bf04 100644 --- a/test/ruby/test_shapes.rb +++ b/test/ruby/test_shapes.rb @@ -1106,8 +1106,8 @@ def test_shape_layout klass.instance_variable_set(:@a, 123) assert_equal :rclass, RubyVM::Shape.of(klass).layout - assert_equal :rdata, RubyVM::Shape.of(Thread.current).layout - assert_equal :rdata, RubyVM::Shape.of(lambda {}).layout + assert_equal :extended_or_rdata, RubyVM::Shape.of(Thread.current).layout + assert_equal :extended_or_rdata, RubyVM::Shape.of(lambda {}).layout assert_equal :other, RubyVM::Shape.of(Struct.new(:x).new(1)).layout assert_equal :other, RubyVM::Shape.of([]).layout diff --git a/variable.c b/variable.c index 994feb7a202d8b..bcd8c32ea2246b 100644 --- a/variable.c +++ b/variable.c @@ -1274,6 +1274,22 @@ rb_obj_fields_generic_uncached(VALUE obj) return fields_obj; } +static bool +obj_use_generic_fields_tbl_p(VALUE obj) +{ + switch (BUILTIN_TYPE(obj)) { + case T_OBJECT: + case T_CLASS: + case T_MODULE: + case T_DATA: + return false; + case T_STRUCT: + return !FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS); + default: + return true; + } +} + VALUE rb_obj_fields(VALUE obj, ID field_name) { @@ -1283,6 +1299,9 @@ rb_obj_fields(VALUE obj, ID field_name) VALUE fields_obj = 0; if (rb_obj_shape_has_fields(obj)) { switch (BUILTIN_TYPE(obj)) { + case T_OBJECT: + fields_obj = ROBJECT_FIELDS_OBJ(obj); + break; case T_DATA: fields_obj = RTYPEDDATA(obj)->fields_obj; break; @@ -1360,10 +1379,15 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie } RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields)); - RUBY_ASSERT(!original_fields_obj || IMEMO_TYPE_P(original_fields_obj, imemo_fields)); + RUBY_ASSERT(!original_fields_obj || IMEMO_TYPE_P(original_fields_obj, imemo_fields) || RB_TYPE_P(original_fields_obj, T_OBJECT)); + int type = BUILTIN_TYPE(obj); if (fields_obj != original_fields_obj) { - switch (BUILTIN_TYPE(obj)) { + switch (type) { + case T_OBJECT: + RUBY_ASSERT(obj != fields_obj); + ROBJECT_SET_EXTENDED(obj, fields_obj); + break; case T_DATA: RB_OBJ_WRITE(obj, &RTYPEDDATA(obj)->fields_obj, fields_obj); break; @@ -1389,19 +1413,32 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie } } - if (original_fields_obj) { + if (original_fields_obj && original_fields_obj != obj) { // Clear root shape to avoid triggering cleanup such as free_object_id. rb_imemo_fields_clear(original_fields_obj); } } - RBASIC_SET_SHAPE_ID(obj, RBASIC_SHAPE_ID(fields_obj)); + if (type == T_OBJECT) { + RBASIC_SET_SHAPE_ID_WITH_LAYOUT(obj, RBASIC_SHAPE_ID(fields_obj), SHAPE_ID_LAYOUT_EXTENDED); + } + else { + RBASIC_SET_SHAPE_ID(obj, RBASIC_SHAPE_ID(fields_obj)); + } } void rb_obj_replace_fields(VALUE obj, VALUE fields_obj) { - RB_VM_LOCKING() { + if (obj_use_generic_fields_tbl_p(obj)) { + // We'll first lookup the generic fields table and then insert + // into it, so lock once for both operations. + RB_VM_LOCKING() { + VALUE original_fields_obj = rb_obj_fields_no_ractor_check(obj); + rb_obj_set_fields(obj, fields_obj, 0, original_fields_obj); + } + } + else { VALUE original_fields_obj = rb_obj_fields_no_ractor_check(obj); rb_obj_set_fields(obj, fields_obj, 0, original_fields_obj); } @@ -1421,7 +1458,7 @@ rb_obj_field_get(VALUE obj, shape_id_t target_shape_id) fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj); break; case T_OBJECT: - fields_obj = obj; + fields_obj = ROBJECT_FIELDS_OBJ(obj); break; case T_IMEMO: RUBY_ASSERT(IMEMO_TYPE_P(obj, imemo_fields)); @@ -1472,7 +1509,7 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef) fields_obj = obj; break; case T_OBJECT: - fields_obj = obj; + fields_obj = ROBJECT_FIELDS_OBJ(obj); break; default: fields_obj = rb_obj_fields(obj, id); @@ -1573,38 +1610,17 @@ static shape_id_t obj_transition_complex(VALUE obj, st_table *table) { RUBY_ASSERT(!rb_obj_shape_complex_p(obj)); + RUBY_ASSERT(!RB_TYPE_P(obj, T_IMEMO)); + shape_id_t shape_id = rb_obj_shape_transition_complex(obj); + VALUE fields_obj = rb_imemo_fields_new_complex_tbl(obj, shape_id, table, RB_OBJ_SHAREABLE_P(obj)); - switch (BUILTIN_TYPE(obj)) { - case T_OBJECT: - { - VALUE *old_fields = NULL; - uint32_t old_fields_len = 0; - if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { - old_fields = ROBJECT_FIELDS(obj); - old_fields_len = ROBJECT_FIELDS_CAPACITY(obj); - } - else { - FL_SET_RAW(obj, ROBJECT_HEAP); - } - RBASIC_SET_SHAPE_ID(obj, shape_id); - ROBJECT_SET_FIELDS_HASH(obj, table); - if (old_fields) { - SIZED_FREE_N(old_fields, old_fields_len); - } - } - break; - case T_CLASS: - case T_MODULE: - case T_IMEMO: - UNREACHABLE; - break; - default: - { - VALUE fields_obj = rb_imemo_fields_new_complex_tbl(obj, shape_id, table, RB_OBJ_SHAREABLE_P(obj)); - rb_obj_replace_fields(obj, fields_obj); - } - } + rb_obj_set_fields(obj, fields_obj, 0, 0); + RBASIC_SET_SHAPE_ID(obj, shape_id); + + RUBY_ASSERT(FL_TEST_RAW(fields_obj, ROBJECT_HEAP)); + RUBY_ASSERT(rb_obj_shape_complex_p(obj)); + RUBY_ASSERT(rb_obj_shape_complex_p(fields_obj)); return shape_id; } @@ -1658,7 +1674,7 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) } break; case T_OBJECT: - fields_obj = obj; + fields_obj = ROBJECT_FIELDS_OBJ(obj); break; default: { fields_obj = rb_obj_fields(obj, id); @@ -1681,12 +1697,7 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) if (UNLIKELY(rb_shape_complex_p(next_shape_id))) { if (UNLIKELY(!rb_shape_complex_p(old_shape_id))) { - if (type == T_OBJECT) { - rb_evict_fields_to_hash(obj); - } - else { - fields_obj = imemo_fields_complex_from_obj(obj, fields_obj, next_shape_id); - } + fields_obj = imemo_fields_complex_from_obj(obj, fields_obj, next_shape_id); } st_data_t key = id; if (!st_delete(rb_imemo_fields_complex_tbl(fields_obj), &key, (st_data_t *)&val)) { @@ -1712,18 +1723,17 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) MEMMOVE(&fields[removed_index], &fields[removed_index + 1], VALUE, trailing_fields); RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id); - if (FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP)) { - if (rb_obj_embedded_size(new_fields_count) <= rb_gc_obj_slot_size(fields_obj)) { - // Re-embed objects when instances become small enough - // This is necessary because YJIT assumes that objects with the same shape - // have the same embeddedness for efficiency (avoid extra checks) - FL_UNSET_RAW(fields_obj, ROBJECT_HEAP); - MEMCPY(rb_imemo_fields_ptr(fields_obj), fields, VALUE, new_fields_count); - SIZED_FREE_N(fields, RSHAPE_CAPACITY(old_shape_id)); - } - else if (RSHAPE_CAPACITY(old_shape_id) != RSHAPE_CAPACITY(next_shape_id)) { - IMEMO_OBJ_FIELDS(fields_obj)->as.external.ptr = ruby_xrealloc_sized(fields, RSHAPE_CAPACITY(next_shape_id) * sizeof(VALUE), RSHAPE_CAPACITY(old_shape_id) * sizeof(VALUE)); + if (type == T_OBJECT && obj != fields_obj && new_fields_count == rb_shape_embedded_capacity(RBASIC_SHAPE_ID(obj))) { + // Re-embed objects when instances become small enough + // This is necessary because YJIT assumes that objects with the same shape + // have the same embeddedness for efficiency (avoid extra checks) + // Note: shapes have changed significantly since, we could not do this anymore. + VALUE *embedded_fields = ROBJECT_EMBEDDED_FIELDS(obj); + MEMCPY(embedded_fields, fields, VALUE, new_fields_count); + for (attr_index_t i = 0; i < new_fields_count; i++) { + RB_OBJ_WRITTEN(obj, Qundef, embedded_fields[i]); } + fields_obj = 0; } } else { @@ -1732,10 +1742,15 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) } } - RBASIC_SET_SHAPE_ID(obj, next_shape_id); if (fields_obj != original_fields_obj) { switch (type) { case T_OBJECT: + if (!fields_obj) { + FL_UNSET_RAW(obj, ROBJECT_HEAP); + } + else if (fields_obj != obj) { + ROBJECT_SET_EXTENDED(obj, fields_obj); + } break; case T_CLASS: case T_MODULE: @@ -1747,6 +1762,18 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) } } + if (type == T_OBJECT) { + if (!fields_obj || fields_obj == obj) { + RBASIC_SET_SHAPE_ID_WITH_LAYOUT(obj, next_shape_id, SHAPE_ID_LAYOUT_ROBJECT); + } + else { + RBASIC_SET_SHAPE_ID_WITH_LAYOUT(obj, next_shape_id, SHAPE_ID_LAYOUT_EXTENDED); + } + } + else { + RBASIC_SET_SHAPE_ID(obj, next_shape_id); + } + return val; } @@ -1764,9 +1791,9 @@ rb_obj_init_complex(VALUE obj, st_table *table) RUBY_ASSERT(RSHAPE_LEN(RBASIC_SHAPE_ID(obj)) == 0); if (rb_obj_shape_complex_p(obj)) { - st_table *old_table = ROBJECT_FIELDS_HASH(obj); - ROBJECT_SET_FIELDS_HASH(obj, table); - if (old_table) st_free_table(old_table); + shape_id_t shape_id = rb_obj_shape_transition_complex(obj); + VALUE fields_obj = rb_imemo_fields_new_complex_tbl(obj, shape_id, table, false); + ROBJECT_SET_EXTENDED(obj, fields_obj); } else { obj_transition_complex(obj, table); @@ -1847,8 +1874,7 @@ imemo_fields_set(VALUE owner, VALUE fields_obj, shape_id_t target_shape_id, ID f } else { attr_index_t index = RSHAPE_INDEX(target_shape_id); - - if (concurrent || index >= RSHAPE_CAPACITY(current_shape_id)) { + if (concurrent || index >= rb_shape_embedded_capacity(current_shape_id)) { return imemo_fields_copy_append(owner, original_fields_obj, current_shape_id, target_shape_id, val); } @@ -1904,23 +1930,6 @@ generic_ivar_set(VALUE obj, ID id, VALUE val) return generic_field_set(obj, target_shape_id, id, val); } -void -rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t new_capacity) -{ - RUBY_ASSERT(!rb_obj_shape_complex_p(obj)); - - if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { - SIZED_REALLOC_N(ROBJECT(obj)->as.heap.fields, VALUE, new_capacity, current_len); - } - else { - VALUE *ptr = ROBJECT_FIELDS(obj); - VALUE *newptr = ALLOC_N(VALUE, new_capacity); - MEMCPY(newptr, ptr, VALUE, current_len); - FL_SET_RAW(obj, ROBJECT_HEAP); - ROBJECT(obj)->as.heap.fields = newptr; - } -} - static int rb_obj_copy_ivs_to_hash_table_i(ID key, VALUE val, st_data_t arg) { @@ -1952,31 +1961,54 @@ obj_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val) current_shape_id = rb_evict_fields_to_hash(obj); } - if (RSHAPE_LEN(target_shape_id) > RSHAPE_LEN(current_shape_id)) { - RBASIC_SET_SHAPE_ID(obj, target_shape_id); - } + // may be T_OBJECT or imemo_fields + VALUE fields_obj = ROBJECT_FIELDS_OBJ(obj); + + RUBY_ASSERT(rb_obj_shape_complex_p(obj)); + RUBY_ASSERT(rb_obj_shape_complex_p(fields_obj)); if (!field_name) { field_name = RSHAPE_EDGE_NAME(target_shape_id); RUBY_ASSERT(field_name); } - st_insert(ROBJECT_FIELDS_HASH(obj), (st_data_t)field_name, (st_data_t)val); - RB_OBJ_WRITTEN(obj, Qundef, val); + st_insert(rb_imemo_fields_complex_tbl(fields_obj), (st_data_t)field_name, (st_data_t)val); + RB_OBJ_WRITTEN(fields_obj, Qundef, val); + + RBASIC_SET_SHAPE_ID(obj, target_shape_id); + if (obj != fields_obj) { + RBASIC_SET_SHAPE_ID(fields_obj, target_shape_id); + } return ATTR_INDEX_NOT_SET; } else { attr_index_t index = RSHAPE_INDEX(target_shape_id); - if (index >= RSHAPE_LEN(current_shape_id)) { - if (UNLIKELY(index >= RSHAPE_CAPACITY(current_shape_id))) { - rb_ensure_iv_list_size(obj, RSHAPE_CAPACITY(current_shape_id), RSHAPE_CAPACITY(target_shape_id)); - } + // may be T_OBJECT or imemo_fields + VALUE fields_obj = ROBJECT_FIELDS_OBJ(obj); + + if (index < RSHAPE_LEN(current_shape_id)) { + // Replace existing value; + RB_OBJ_WRITE(fields_obj, &rb_imemo_fields_ptr(fields_obj)[index], val); + return index; + } + + RUBY_ASSERT(index == RSHAPE_LEN(current_shape_id)); + + if (UNLIKELY(index >= RSHAPE_CAPACITY(current_shape_id))) { + fields_obj = imemo_fields_copy_append(obj, fields_obj, current_shape_id, target_shape_id, val); + ROBJECT_SET_EXTENDED(obj, fields_obj); + RBASIC_SET_FULL_SHAPE_ID(obj, rb_shape_transition_layout(target_shape_id, SHAPE_ID_LAYOUT_EXTENDED)); + } + else { + RB_OBJ_WRITE(fields_obj, &rb_imemo_fields_ptr(fields_obj)[index], val); RBASIC_SET_SHAPE_ID(obj, target_shape_id); } - RB_OBJ_WRITE(obj, &ROBJECT_FIELDS(obj)[index], val); + if (obj != fields_obj) { + RBASIC_SET_SHAPE_ID(fields_obj, target_shape_id); + } return index; } diff --git a/vm_eval.c b/vm_eval.c index 84bb40ba8034ac..049131b0914d7b 100644 --- a/vm_eval.c +++ b/vm_eval.c @@ -1080,6 +1080,7 @@ VALUE rb_funcallv(VALUE recv, ID mid, int argc, const VALUE *argv) { VM_ASSERT(ruby_thread_has_gvl_p()); + VM_ASSERT(!RB_TYPE_P(recv, T_IMEMO)); return rb_funcallv_scope(recv, mid, argc, argv, CALL_FCALL); } diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 06ad97691523a0..1abf7dd122aefa 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1216,7 +1216,7 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call switch (BUILTIN_TYPE(obj)) { case T_OBJECT: - fields_obj = obj; + fields_obj = ROBJECT_FIELDS_OBJ(obj); break; case T_CLASS: case T_MODULE: @@ -1463,9 +1463,13 @@ vm_setivar(VALUE obj, VALUE val, rb_setivar_cache cache) break; } - RB_OBJ_WRITE(obj, &ROBJECT_FIELDS(obj)[cache.index], val); + VALUE fields_obj = ROBJECT_FIELDS_OBJ(obj); + RB_OBJ_WRITE(fields_obj, &rb_imemo_fields_ptr(fields_obj)[cache.index], val); if (shape_id != dest_shape_id) { RBASIC_SET_SHAPE_ID(obj, dest_shape_id); + if (fields_obj != obj) { + RBASIC_SET_SHAPE_ID(fields_obj, dest_shape_id); + } } RB_DEBUG_COUNTER_INC(ivar_set_ic_hit); diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 1f8b320c4d6749..0a9a0a796eb7a5 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -3015,20 +3015,20 @@ fn gen_get_ivar( } Some(ivar_index) => { let ivar_opnd = if receiver_t_object { - if comptime_receiver.embedded_p() { - // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h + let offs = ROBJECT_OFFSET_AS_ARY as i32 + (ivar_index * SIZEOF_VALUE) as i32; + // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h + if comptime_receiver.embedded_p() { // Load the variable - let offs = ROBJECT_OFFSET_AS_ARY as i32 + (ivar_index * SIZEOF_VALUE) as i32; Opnd::mem(64, recv, offs) - } else { + } else { // Compile time value is *not* embedded. - // Get a pointer to the extended table + // Get the T_IMEMO/fields let tbl_opnd = asm.load(Opnd::mem(64, recv, ROBJECT_OFFSET_AS_HEAP_FIELDS as i32)); - // Read the ivar from the extended table - Opnd::mem(64, tbl_opnd, (SIZEOF_VALUE * ivar_index) as i32) + // Read the ivar from the T_IMEMO/fields + Opnd::mem(64, tbl_opnd, offs) } } else { asm_comment!(asm, "call rb_ivar_get_at()"); @@ -3080,6 +3080,33 @@ fn gen_getinstancevariable( ) } +fn gen_trigger_wb( + asm: &mut Assembler, + recv: Opnd, + write_val: Opnd) +{ + asm.spill_regs(); // for ccall (unconditionally spill them for RegMappings consistency) + let skip_wb = asm.new_label("skip_wb"); + // If the value we're writing is an immediate, we don't need to WB + asm.test(write_val, (RUBY_IMMEDIATE_MASK as u64).into()); + asm.jnz(skip_wb); + + // If the value we're writing is nil or false, we don't need to WB + asm.cmp(write_val, Qnil.into()); + asm.jbe(skip_wb); + + asm_comment!(asm, "write barrier"); + asm.ccall( + rb_gc_writebarrier as *const u8, + vec![ + recv, + write_val, + ] + ); + + asm.write_label(skip_wb); +} + // Generate an IV write. // This function doesn't deal with writing the shape, or expanding an object // to use an IV buffer if necessary. That is the callers responsibility @@ -3089,30 +3116,44 @@ fn gen_write_iv( recv: Opnd, ivar_index: usize, set_value: Opnd, - extension_needed: bool) + extension_needed: bool, + skip_wb: bool) { // Compile time self is embedded and the ivar index lands within the object let embed_test_result = comptime_receiver.embedded_p() && !extension_needed; + let offs = ROBJECT_OFFSET_AS_ARY as i32 + (ivar_index * SIZEOF_VALUE) as i32; + if embed_test_result { // Find the IV offset - let offs = ROBJECT_OFFSET_AS_ARY as i32 + (ivar_index * SIZEOF_VALUE) as i32; let ivar_opnd = Opnd::mem(64, recv, offs); // Write the IV asm_comment!(asm, "write IV"); asm.mov(ivar_opnd, set_value); + + // If we know the stack value is an immediate, there's no need to + // generate WB code. + if !skip_wb { + gen_trigger_wb(asm, recv, set_value); + } } else { // Compile time value is *not* embedded. - // Get a pointer to the extended table + // Get a pointer to the extended T_IMEMO/fields let tbl_opnd = asm.load(Opnd::mem(64, recv, ROBJECT_OFFSET_AS_HEAP_FIELDS as i32)); // Write the ivar in to the extended table - let ivar_opnd = Opnd::mem(64, tbl_opnd, (SIZEOF_VALUE * ivar_index) as i32); + let ivar_opnd = Opnd::mem(64, tbl_opnd, offs); asm_comment!(asm, "write IV"); asm.mov(ivar_opnd, set_value); + + // If we know the stack value is an immediate, there's no need to + // generate WB code. + if !skip_wb { + gen_trigger_wb(asm, tbl_opnd, set_value); + } } } @@ -3183,7 +3224,7 @@ fn gen_set_ivar( // The current shape doesn't contain this iv, we need to transition to another shape. let mut new_shape_complex = false; - let new_shape = if !shape_complex && receiver_t_object && ivar_index.is_none() { + if !shape_complex && receiver_t_object && ivar_index.is_none() { let current_shape_id = comptime_receiver.shape_id_of(); // We don't need to check about imemo_fields here because we're definitely looking at a T_OBJECT. let klass = unsafe { rb_obj_class(comptime_receiver) }; @@ -3218,7 +3259,7 @@ fn gen_set_ivar( // If the receiver isn't a T_OBJECT, then just write out the IV write as a function call. // too-complex shapes can't use index access, so we use rb_ivar_get for them too. - if !receiver_t_object || shape_complex || new_shape_complex || megamorphic { + if !receiver_t_object || shape_complex || new_shape_complex || megamorphic || ivar_index.is_none() { // The function could raise FrozenError. // Note that this modifies REG_SP, which is why we do it first jit_prepare_non_leaf_call(jit, asm); @@ -3252,7 +3293,7 @@ fn gen_set_ivar( } } else { // Get the receiver - let mut recv = asm.load(if let StackOpnd(index) = recv_opnd { + let recv = asm.load(if let StackOpnd(index) = recv_opnd { asm.stack_opnd(index as i32) } else { Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SELF) @@ -3278,44 +3319,8 @@ fn gen_set_ivar( let write_val; match ivar_index { - // If we don't have an instance variable index, then we need to - // transition out of the current shape. None => { - let (new_shape_id, needs_extension, ivar_index) = new_shape.unwrap(); - if let Some((current_capacity, new_capacity)) = needs_extension { - // Generate the C call so that runtime code will increase - // the capacity and set the buffer. - asm_comment!(asm, "call rb_ensure_iv_list_size"); - - // It allocates so can trigger GC, which takes the VM lock - // so could yield to a different ractor. - jit_prepare_call_with_gc(jit, asm); - asm.ccall(rb_ensure_iv_list_size as *const u8, - vec![ - recv, - Opnd::UImm(current_capacity.into()), - Opnd::UImm(new_capacity.into()) - ] - ); - - // Load the receiver again after the function call - recv = asm.load(if let StackOpnd(index) = recv_opnd { - asm.stack_opnd(index as i32) - } else { - Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SELF) - }); - } - - write_val = asm.stack_opnd(0); - gen_write_iv(asm, comptime_receiver, recv, ivar_index, write_val, needs_extension.is_some()); - - asm_comment!(asm, "write shape"); - - let shape_id_offset = unsafe { rb_shape_id_offset() }; - let shape_opnd = Opnd::mem(SHAPE_ID_NUM_BITS as u8, recv, shape_id_offset); - - // Store the new shape - asm.store(shape_opnd, Opnd::UImm(new_shape_id as u64)); + panic!("ivar_index None should have been delegated to rb_vm_set_ivar_id") }, Some(ivar_index) => { @@ -3325,34 +3330,9 @@ fn gen_set_ivar( // made the transition already, then there's no reason to // update the shape on the object. Just set the IV. write_val = asm.stack_opnd(0); - gen_write_iv(asm, comptime_receiver, recv, ivar_index, write_val, false); + gen_write_iv(asm, comptime_receiver, recv, ivar_index, write_val, false, stack_type.is_imm()); }, } - - // If we know the stack value is an immediate, there's no need to - // generate WB code. - if !stack_type.is_imm() { - asm.spill_regs(); // for ccall (unconditionally spill them for RegMappings consistency) - let skip_wb = asm.new_label("skip_wb"); - // If the value we're writing is an immediate, we don't need to WB - asm.test(write_val, (RUBY_IMMEDIATE_MASK as u64).into()); - asm.jnz(skip_wb); - - // If the value we're writing is nil or false, we don't need to WB - asm.cmp(write_val, Qnil.into()); - asm.jbe(skip_wb); - - asm_comment!(asm, "write barrier"); - asm.ccall( - rb_gc_writebarrier as *const u8, - vec![ - recv, - write_val, - ] - ); - - asm.write_label(skip_wb); - } } let write_val = asm.stack_pop(1); // Keep write_val on stack during ccall for GC diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 2a9646dfed21bf..47c62c4a7f3f63 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -1126,7 +1126,6 @@ extern "C" { pub fn rb_ivar_get_at_no_ractor_check(obj: VALUE, index: attr_index_t) -> VALUE; pub fn rb_gvar_get(arg1: ID) -> VALUE; pub fn rb_gvar_set(arg1: ID, arg2: VALUE) -> VALUE; - pub fn rb_ensure_iv_list_size(obj: VALUE, current_len: u32, newsize: u32); pub fn rb_vm_barrier(); pub fn rb_str_byte_substr(str_: VALUE, beg: VALUE, len: VALUE) -> VALUE; pub fn rb_str_substr_two_fixnums( diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index ad7d4556786799..9024971d1ac61d 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -288,7 +288,7 @@ impl ShapeId { match self.0 & SHAPE_ID_LAYOUT_MASK { SHAPE_ID_LAYOUT_ROBJECT => ShapeLayout::RObject, SHAPE_ID_LAYOUT_RCLASS => ShapeLayout::RClass, - SHAPE_ID_LAYOUT_RDATA => ShapeLayout::RData, + SHAPE_ID_LAYOUT_EXTENDED => ShapeLayout::RData, SHAPE_ID_LAYOUT_OTHER => ShapeLayout::Other, layout => unreachable!("unknown shape layout bits: {layout:#x}"), } diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index ff254f433ec2c2..b552e8323b3ec5 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -1513,11 +1513,13 @@ pub const SHAPE_ID_FL_FROZEN: shape_id_fl_type = 134217728; pub const SHAPE_ID_FL_HAS_OBJECT_ID: shape_id_fl_type = 268435456; pub const SHAPE_ID_LAYOUT_ROBJECT: shape_id_fl_type = 0; pub const SHAPE_ID_LAYOUT_RCLASS: shape_id_fl_type = 536870912; +pub const SHAPE_ID_LAYOUT_EXTENDED: shape_id_fl_type = 1073741824; pub const SHAPE_ID_LAYOUT_RDATA: shape_id_fl_type = 1073741824; pub const SHAPE_ID_LAYOUT_OTHER: shape_id_fl_type = 1610612736; pub const SHAPE_ID_LAYOUT_MASK: shape_id_fl_type = 1610612736; pub const SHAPE_ID_FL_NON_CANONICAL_MASK: shape_id_fl_type = 402653184; pub const SHAPE_ID_FLAGS_MASK: shape_id_fl_type = 2146959360; +pub const SHAPE_ID_FL_PRIVATE_MASK: shape_id_fl_type = 1677197312; pub type shape_id_fl_type = u32; pub const CONST_DEPRECATED: rb_const_flag_t = 256; pub const CONST_VISIBILITY_MASK: rb_const_flag_t = 255; @@ -2192,7 +2194,6 @@ unsafe extern "C" { pub fn rb_ivar_get_at_no_ractor_check(obj: VALUE, index: attr_index_t) -> VALUE; pub fn rb_gvar_get(arg1: ID) -> VALUE; pub fn rb_gvar_set(arg1: ID, arg2: VALUE) -> VALUE; - pub fn rb_ensure_iv_list_size(obj: VALUE, current_len: u32, newsize: u32); pub fn rb_vm_barrier(); pub fn rb_str_byte_substr(str_: VALUE, beg: VALUE, len: VALUE) -> VALUE; pub fn rb_str_substr_two_fixnums( diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 7225fcdc3800bb..5fd2369537dd7e 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -5344,13 +5344,6 @@ impl Function { elidable: true }) } - fn load_ivar_heap(&mut self, block: BlockId, recv: InsnId, id: ID, ivar_index: attr_index_t) -> InsnId { - // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h - let ptr = self.load_field(block, recv, FieldName::as_heap, ROBJECT_OFFSET_AS_HEAP_FIELDS, types::CPtr); - let offset = SIZEOF_VALUE_I32 * ivar_index as i32; - self.load_field(block, ptr, id.into(), offset, types::BasicObject) - } - fn load_ivar_embedded(&mut self, block: BlockId, recv: InsnId, id: ID, ivar_index: attr_index_t) -> InsnId { // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h let offset = ROBJECT_OFFSET_AS_ARY @@ -5390,11 +5383,7 @@ impl Function { self.load_ivar_embedded(block, fields_obj, id, ivar_index) }, ShapeLayout::RObject => { - if recv_type.flags().is_embedded() { - self.load_ivar_embedded(block, self_val, id, ivar_index) - } else { - self.load_ivar_heap(block, self_val, id, ivar_index) - } + self.load_ivar_embedded(block, self_val, id, ivar_index) }, ShapeLayout::Other => { // Non-T_OBJECT, non-class/module, non-typed-data: fall back to C call @@ -5444,10 +5433,20 @@ impl Function { // Instance variable writes on immediate values raise. return Err(Counter::setivar_fallback_immediate); } - if !profiled_type.flags().is_t_object() { - // Check if the receiver is a T_OBJECT - return Err(Counter::setivar_fallback_not_t_object); + + match profiled_type.shape().layout() { + ShapeLayout::RObject => { + // OK + } + ShapeLayout::RData => { + // FIXME: we side exit for now as we're missing SHAPE_ID_FL_PRIVATE_MASK handling. + return Err(Counter::setivar_fallback_not_t_object); + } + _ => { + return Err(Counter::setivar_fallback_not_t_object); + } } + assert!(profiled_type.shape().is_valid()); if profiled_type.shape().is_frozen() { // Can't set ivars on frozen objects @@ -5490,22 +5489,35 @@ impl Function { } fn emit_optimized_setivar(&mut self, block: BlockId, self_val: InsnId, id: ID, val: InsnId, spec: SetIvarSpec) { - // Current shape contains this ivar - let (ivar_storage, offset) = if spec.profiled_type.flags().is_embedded() { - // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h - let offset = ROBJECT_OFFSET_AS_ARY + (SIZEOF_VALUE * spec.ivar_index.to_usize()) as i32; - (self_val, offset) - } else { - let as_heap = self.load_field(block, self_val, FieldName::as_heap, ROBJECT_OFFSET_AS_HEAP_FIELDS, types::CPtr); - let offset = SIZEOF_VALUE_I32 * spec.ivar_index as i32; - (as_heap, offset) + let offset = ROBJECT_OFFSET_AS_ARY + (SIZEOF_VALUE * spec.ivar_index.to_usize()) as i32; + + // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h + let (ivar_storage, embedded) = match spec.profiled_type.shape().layout() { + ShapeLayout::RObject => { // AKA embedded + (self_val, true) + }, + ShapeLayout::RData => { // AKA extended + let fields = self.load_field(block, self_val, FieldName::as_heap, ROBJECT_OFFSET_AS_HEAP_FIELDS, types::BasicObject); + (fields, false) + }, + ShapeLayout::Other | ShapeLayout::RClass => { + panic!("This is a T_OBJECT only path (for now)") + } }; + self.push_insn(block, Insn::StoreField { recv: ivar_storage, id: id.into(), offset, val, num_bits: types::BasicObject.num_bits() }); - self.push_insn(block, Insn::WriteBarrier { recv: self_val, val }); + self.push_insn(block, Insn::WriteBarrier { recv: ivar_storage, val }); if spec.next_shape != spec.profiled_type.shape() { // Write the new shape ID let shape_id = self.push_insn(block, Insn::Const { val: Const::CShape(spec.next_shape) }); let shape_id_offset = unsafe { rb_shape_id_offset() }; + + if !embedded { + // FIXME: We need to strip the SHAPE_ID_FL_PRIVATE_MASK from the shape for `ivar_storage`. + // see `RBASIC_SET_SHAPE_ID`. + // This path is currently dead code, see the FIXME in `prepare_optimized_setivar` + self.push_insn(block, Insn::StoreField { recv: ivar_storage, id: FieldName::shape_id, offset: shape_id_offset, val: shape_id, num_bits: types::CShape.num_bits() }); + } self.push_insn(block, Insn::StoreField { recv: self_val, id: FieldName::shape_id, offset: shape_id_offset, val: shape_id, num_bits: types::CShape.num_bits() }); } } diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 8023368039055f..b75bd87f178346 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -9492,8 +9492,8 @@ mod hir_opt_tests { Jump bb4(v17) bb6(): v19:CShape[0x1003] = GuardBitEquals v12, CShape(0x1003) recompile - v21:CPtr = LoadField v11, :as_heap@0x1004 - v22:BasicObject = LoadField v21, :@foo@0x1005 + v21:RubyValue = LoadField v11, :fields_obj@0x1004 + v22:BasicObject = LoadField v21, :@foo@0x1004 Jump bb4(v22) bb4(v13:BasicObject): v25:Fixnum[1] = Const Value(1) @@ -9555,12 +9555,12 @@ mod hir_opt_tests { v15:CBool = IsBitEqual v12, v14 CondBranch v15, bb5(), bb6() bb5(): - v17:CPtr = LoadField v11, :as_heap@0x1002 - v18:BasicObject = LoadField v17, :@foo@0x1003 + v17:RubyValue = LoadField v11, :fields_obj@0x1002 + v18:BasicObject = LoadField v17, :@foo@0x1002 Jump bb4(v18) bb6(): - v20:CShape[0x1004] = GuardBitEquals v12, CShape(0x1004) recompile - v22:BasicObject = LoadField v11, :@foo@0x1005 + v20:CShape[0x1003] = GuardBitEquals v12, CShape(0x1003) recompile + v22:BasicObject = LoadField v11, :@foo@0x1004 Jump bb4(v22) bb4(v13:BasicObject): v25:Fixnum[1] = Const Value(1) From eed5795a04627b846180f1e617d3724e711928bd Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 10 Jul 2026 13:44:22 +0900 Subject: [PATCH 09/13] [ruby/rubygems] Don't reassign Gem::TSort when old RubyGems already defines it RubyGems older than 3.6 has no rubygems/vendored_tsort, and its own Gem::TSort is often already loaded through rubygems/request_set by the time Bundler runs, e.g. from Gem.activate_bin_path in binstubs. The unconditional reassignment printed an already-initialized-constant warning to stderr, breaking specs that assert empty stderr against system RubyGems. The old vendored Gem::TSort is API-compatible, so keep it when present. https://github.com/ruby/rubygems/commit/a42ded1d99 Co-Authored-By: Claude Fable 5 --- lib/bundler/vendored_tsort.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/bundler/vendored_tsort.rb b/lib/bundler/vendored_tsort.rb index 8d8ab26ecef91c..e49492994d8fb3 100644 --- a/lib/bundler/vendored_tsort.rb +++ b/lib/bundler/vendored_tsort.rb @@ -4,5 +4,8 @@ require "rubygems/vendored_tsort" rescue LoadError require "tsort" - Gem::TSort = TSort + # RubyGems older than 3.6 has no rubygems/vendored_tsort, but may have + # already loaded its own API-compatible Gem::TSort through + # rubygems/request_set, e.g. from Gem.activate_bin_path in binstubs. + Gem::TSort = TSort unless defined?(Gem::TSort) end From d48f442dc188522d778edfad926b23127d95b4dd Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 10 Jul 2026 13:59:28 +0900 Subject: [PATCH 10/13] [ruby/rubygems] Restore system RubyGems coverage in CI with explicit RGV=system Since RGV started defaulting to ".", the system-rubygems-bundler job has been silently testing the repo's RubyGems instead of the one shipped with each Ruby, because switch_rubygems is injected into every spec-spawned process. Add an explicit RGV=system mode and set it on the job's test step. The harness itself keeps running on the checked out RubyGems even in this mode. The flattened lib/ contains both Bundler and RubyGems, so it is unavoidably on the harness load path and would get mixed into an older RubyGems otherwise. Only processes spawned by specs boot system RubyGems untouched, which is the configuration under test. https://github.com/ruby/rubygems/commit/40bb5bcd38 Co-Authored-By: Claude Fable 5 --- spec/bundler/support/helpers.rb | 8 +++++++- spec/bundler/support/switch_rubygems.rb | 10 +++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/spec/bundler/support/helpers.rb b/spec/bundler/support/helpers.rb index b0d4b5008ba742..7059742ec88387 100644 --- a/spec/bundler/support/helpers.rb +++ b/spec/bundler/support/helpers.rb @@ -185,7 +185,13 @@ def gembin(cmd, options = {}) def sys_exec(cmd, options = {}, &block) env = options[:env] || {} - env["RUBYOPT"] = opt_add(opt_add("-r#{spec_dir}/support/switch_rubygems.rb", env["RUBYOPT"]), ENV["RUBYOPT"]) + if ENV["RGV"] == "system" + # Spawned processes must boot system RubyGems untouched, so don't make + # them switch, and drop the load path override the harness runs with. + env["RUBYOPT"] = opt_add(env["RUBYOPT"] || "", opt_remove("-I#{source_lib_dir}", ENV["RUBYOPT"])) + else + env["RUBYOPT"] = opt_add(opt_add("-r#{spec_dir}/support/switch_rubygems.rb", env["RUBYOPT"]), ENV["RUBYOPT"]) + end options[:env] = env sh(cmd, options, &block) diff --git a/spec/bundler/support/switch_rubygems.rb b/spec/bundler/support/switch_rubygems.rb index 640b9f83b7d116..ac8ba1f03971a4 100644 --- a/spec/bundler/support/switch_rubygems.rb +++ b/spec/bundler/support/switch_rubygems.rb @@ -2,4 +2,12 @@ require_relative "rubygems_version_manager" ENV["RGV"] ||= "." -RubygemsVersionManager.new(ENV["RGV"]).switch + +# RGV=system runs specs against system RubyGems: processes spawned by specs +# boot it untouched (see `Spec::Helpers#sys_exec`). The test harness itself +# still runs on the checked out RubyGems, because the repo's lib/ dir is +# unavoidably on its load path and would get mixed into an older RubyGems. +source = ENV["RGV"] +source = "." if source == "system" + +RubygemsVersionManager.new(source).switch From e8219853ad185d34aaa87450033b2e737d13ed8f Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 10 Jul 2026 14:13:37 +0900 Subject: [PATCH 11/13] [ruby/rubygems] Fall back to rubygems/tsort instead of activating the tsort default gem RubyGems 3.4 and 3.5 ship vendored tsort under its pre-3.6 name rubygems/tsort, so prefer that when rubygems/vendored_tsort is missing. Requiring the real tsort activated the tsort default gem, breaking the guarantee that bundler/setup activates no gems. This mirrors the fallback chain already used by vendored_net_http. https://github.com/ruby/rubygems/commit/7300743bfe Co-Authored-By: Claude Fable 5 --- lib/bundler/vendored_tsort.rb | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/bundler/vendored_tsort.rb b/lib/bundler/vendored_tsort.rb index e49492994d8fb3..d7d680344efd6e 100644 --- a/lib/bundler/vendored_tsort.rb +++ b/lib/bundler/vendored_tsort.rb @@ -1,11 +1,21 @@ # frozen_string_literal: true -begin - require "rubygems/vendored_tsort" -rescue LoadError - require "tsort" - # RubyGems older than 3.6 has no rubygems/vendored_tsort, but may have - # already loaded its own API-compatible Gem::TSort through - # rubygems/request_set, e.g. from Gem.activate_bin_path in binstubs. - Gem::TSort = TSort unless defined?(Gem::TSort) +# The defined? guard avoids reopening Gem::TSort when an old RubyGems has +# already loaded its own copy, e.g. through rubygems/request_set from +# Gem.activate_bin_path in binstubs. +# +unless defined?(Gem::TSort) + begin + require "rubygems/vendored_tsort" + rescue LoadError + begin + # RubyGems 3.4 and 3.5 ship the same file under its pre-3.6 name. + # Requiring the real tsort here instead would activate the tsort + # default gem, and `bundler/setup` must not activate any gems. + require "rubygems/tsort" + rescue LoadError + require "tsort" + Gem::TSort = TSort + end + end end From a5f2cc69bfcf5aaec9e0feadcbabcc0ec5888ab8 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 10 Jul 2026 15:06:21 +0900 Subject: [PATCH 12/13] [ruby/rubygems] Fix bundle install with no_install_plugin on RubyGems older than 4.1 Gem::Installer#remove_stale_plugins only exists since RubyGems 4.1, so installing with no_install_plugin set on an older system RubyGems raised NameError. Reimplement it in Bundler's installer, like generate_plugins already is. https://github.com/ruby/rubygems/commit/d3037ba378 Co-Authored-By: Claude Fable 5 --- lib/bundler/rubygems_gem_installer.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/bundler/rubygems_gem_installer.rb b/lib/bundler/rubygems_gem_installer.rb index c6313ddf8d0eb7..d8c50556c531c8 100644 --- a/lib/bundler/rubygems_gem_installer.rb +++ b/lib/bundler/rubygems_gem_installer.rb @@ -94,6 +94,15 @@ def generate_plugins end end + # Reimplemented from RubyGems, since RubyGems older than 4.1 doesn't + # provide it + def remove_stale_plugins + return unless spec.plugins.empty? + + ensure_writable_dir @plugins_dir + remove_plugins_for(spec, @plugins_dir) + end + def warn_skipped_extensions return if spec.extensions.empty? From 605b28ecb1ae290018be76a5b5a845647d5cbb50 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 10 Jul 2026 15:06:33 +0900 Subject: [PATCH 13/13] [ruby/rubygems] Make version-sensitive specs check the RubyGems version under test Under RGV=system the harness runs on the checked out RubyGems while spec-spawned processes run system RubyGems, so rubygems-tagged filters and expectations interpolating Gem::VERSION were checking the wrong version. Capture the system RubyGems version before switching and use it for the rubygems: exclusion filter, version-dependent expectations, and the global gem cache location. The cached git gemspec comparison now serializes with the RubyGems that wrote the file, since Specification#to_ruby output differs across versions. https://github.com/ruby/rubygems/commit/62fa95fe35 Co-Authored-By: Claude Fable 5 --- spec/bundler/commands/exec_spec.rb | 4 ++-- spec/bundler/commands/install_spec.rb | 4 ++-- spec/bundler/install/gemfile/git_spec.rb | 7 +++++-- spec/bundler/install/gems/resolving_spec.rb | 2 +- spec/bundler/install/global_cache_spec.rb | 7 ++++--- spec/bundler/runtime/self_management_spec.rb | 8 +++----- spec/bundler/support/filters.rb | 5 ++++- spec/bundler/support/helpers.rb | 7 +++++++ spec/bundler/support/switch_rubygems.rb | 7 ++++++- 9 files changed, 34 insertions(+), 17 deletions(-) diff --git a/spec/bundler/commands/exec_spec.rb b/spec/bundler/commands/exec_spec.rb index d744fc616bf238..e7a294b42932fa 100644 --- a/spec/bundler/commands/exec_spec.rb +++ b/spec/bundler/commands/exec_spec.rb @@ -75,13 +75,13 @@ skip "https://github.com/ruby/rubygems/issues/3351" if mswin? install_gemfile "source \"https://gem.repo1\"; gem \"myrack\"" bundle "exec #{gem_cmd} --version" - expect(out).to eq(Gem::VERSION) + expect(out).to eq(exercised_rubygems_version.to_s) end it "works when exec'ing to rubygems through sh -c" do install_gemfile "source \"https://gem.repo1\"; gem \"myrack\"" bundle "exec sh -c '#{gem_cmd} --version'" - expect(out).to eq(Gem::VERSION) + expect(out).to eq(exercised_rubygems_version.to_s) end it "works when exec'ing back to bundler to run a remote resolve" do diff --git a/spec/bundler/commands/install_spec.rb b/spec/bundler/commands/install_spec.rb index f8a134f231089e..1e0e3a7c244b42 100644 --- a/spec/bundler/commands/install_spec.rb +++ b/spec/bundler/commands/install_spec.rb @@ -1394,7 +1394,7 @@ def run expect(gem_make_out).not_to include("make -j8") end - it "uses 3 slots from the available pool when running the compilation of an extension" do + it "uses 3 slots from the available pool when running the compilation of an extension", rubygems: ">= 4.1.0.dev" do ENV.delete("MAKEFLAGS") install_gemfile(<<~G, env: { "BUNDLE_JOBS" => "8" }) @@ -1407,7 +1407,7 @@ def run expect(gem_make_out).to include("make -j3") end - it "consumes 3 slots from the pool when BUNDLE_JOBS isn't set" do + it "consumes 3 slots from the pool when BUNDLE_JOBS isn't set", rubygems: ">= 4.1.0.dev" do ENV.delete("MAKEFLAGS") install_gemfile(<<~G) diff --git a/spec/bundler/install/gemfile/git_spec.rb b/spec/bundler/install/gemfile/git_spec.rb index ee4c91a4f2b4a6..5fa740f4e1c68b 100644 --- a/spec/bundler/install/gemfile/git_spec.rb +++ b/spec/bundler/install/gemfile/git_spec.rb @@ -114,9 +114,12 @@ sha = git.ref_for("main", 11) spec_file = default_bundle_path("bundler/gems/foo-1.0-#{sha}/foo.gemspec") expect(spec_file).to exist - ruby_code = Gem::Specification.load(spec_file.to_s).to_ruby + # Serialize with the RubyGems that wrote the file, since `#to_ruby` + # output differs across RubyGems versions + ruby "print Gem::Specification.load(#{spec_file.to_s.dump}).to_ruby" + ruby_code = out file_code = File.read(spec_file) - expect(file_code).to eq(ruby_code) + expect(file_code.strip).to eq(ruby_code) end it "does not update the git source implicitly" do diff --git a/spec/bundler/install/gems/resolving_spec.rb b/spec/bundler/install/gems/resolving_spec.rb index 111d361aab9c4f..178f68e30bb676 100644 --- a/spec/bundler/install/gems/resolving_spec.rb +++ b/spec/bundler/install/gems/resolving_spec.rb @@ -726,7 +726,7 @@ Because every version of require_rubygems depends on RubyGems > 9000 and Gemfile depends on require_rubygems >= 0, RubyGems > 9000 is required. - So, because current RubyGems version is = #{Gem::VERSION}, + So, because current RubyGems version is = #{exercised_rubygems_version}, version solving has failed. E expect(err).to end_with(nice_error) diff --git a/spec/bundler/install/global_cache_spec.rb b/spec/bundler/install/global_cache_spec.rb index 259e8d57ba2ebc..5e5494dbe9d349 100644 --- a/spec/bundler/install/global_cache_spec.rb +++ b/spec/bundler/install/global_cache_spec.rb @@ -9,9 +9,10 @@ let(:source2) { "http://gemserver.example.org" } def cache_base - # Use the unified global gem cache path if available (from RubyGems), - # otherwise fall back to the Bundler-specific cache location - if Gem.respond_to?(:global_gem_cache_path) + # Use the unified global gem cache path if the RubyGems under test + # provides it, otherwise fall back to the Bundler-specific cache + # location that Bundler uses on RubyGems older than 4.0 + if exercised_rubygems_version >= Gem::Version.new("4.0.0.a") Pathname.new(Gem.global_gem_cache_path) else home(".bundle", "cache", "gems") diff --git a/spec/bundler/runtime/self_management_spec.rb b/spec/bundler/runtime/self_management_spec.rb index 176c2a3121774d..970a3869367fd0 100644 --- a/spec/bundler/runtime/self_management_spec.rb +++ b/spec/bundler/runtime/self_management_spec.rb @@ -171,11 +171,9 @@ expect(out).to eq(previous_minor) end - it "requires the right bundler version from the config and run bundle CLI without re-exec" do - unless Bundler.rubygems.provides?(">= 4.1.0.dev") - skip "This spec can only run when Gem::BundlerVersionFinder.bundler_versions reads bundler configs" - end - + # Can only run when Gem::BundlerVersionFinder.bundler_versions reads + # bundler configs + it "requires the right bundler version from the config and run bundle CLI without re-exec", rubygems: ">= 4.1.0.dev" do lockfile_bundled_with(current_version) bundle_config "version #{previous_minor}" diff --git a/spec/bundler/support/filters.rb b/spec/bundler/support/filters.rb index 2be25b4a78a6fe..67a32354d081b9 100644 --- a/spec/bundler/support/filters.rb +++ b/spec/bundler/support/filters.rb @@ -23,7 +23,10 @@ def inspect RSpec.configure do |config| config.filter_run_excluding realworld: true - config.filter_run_excluding rubygems: RequirementChecker.against(Gem.rubygems_version) + # Version-gated specs care about the RubyGems that spec-spawned processes + # run, which under RGV=system is older than the one loaded in this process. + exercised_rubygems_version = Gem::Version.new(ENV["BUNDLER_SPEC_SYSTEM_RUBYGEMS_VERSION"] || Gem::VERSION) + config.filter_run_excluding rubygems: RequirementChecker.against(exercised_rubygems_version) config.filter_run_excluding git: RequirementChecker.against(git_version) config.filter_run_excluding ruby_repo: !ENV["GEM_COMMAND"].nil? config.filter_run_excluding no_color_tty: Gem.win_platform? || !ENV["GITHUB_ACTION"].nil? diff --git a/spec/bundler/support/helpers.rb b/spec/bundler/support/helpers.rb index 7059742ec88387..8798cca57bb546 100644 --- a/spec/bundler/support/helpers.rb +++ b/spec/bundler/support/helpers.rb @@ -183,6 +183,13 @@ def gembin(cmd, options = {}) sys_exec(cmd.to_s, options) end + # The RubyGems version that processes spawned by specs run. Under + # RGV=system it's the system RubyGems, which is older than the one loaded + # in the current process. + def exercised_rubygems_version + @exercised_rubygems_version ||= Gem::Version.new(ENV["BUNDLER_SPEC_SYSTEM_RUBYGEMS_VERSION"] || Gem::VERSION) + end + def sys_exec(cmd, options = {}, &block) env = options[:env] || {} if ENV["RGV"] == "system" diff --git a/spec/bundler/support/switch_rubygems.rb b/spec/bundler/support/switch_rubygems.rb index ac8ba1f03971a4..0958a1e827753c 100644 --- a/spec/bundler/support/switch_rubygems.rb +++ b/spec/bundler/support/switch_rubygems.rb @@ -7,7 +7,12 @@ # boot it untouched (see `Spec::Helpers#sys_exec`). The test harness itself # still runs on the checked out RubyGems, because the repo's lib/ dir is # unavoidably on its load path and would get mixed into an older RubyGems. +# The system RubyGems version is captured here, while it's still the one +# loaded, so version-gated specs can check the version actually under test. source = ENV["RGV"] -source = "." if source == "system" +if source == "system" + ENV["BUNDLER_SPEC_SYSTEM_RUBYGEMS_VERSION"] ||= Gem::VERSION + source = "." +end RubygemsVersionManager.new(source).switch