From 513a6d475be6df487a4c7af75755e26be3c69957 Mon Sep 17 00:00:00 2001 From: XrXr Date: Wed, 8 Jul 2026 12:00:59 -0400 Subject: [PATCH 01/13] ZJIT: Delete test-only vm_stack_canary definition We always link with the miniruby now so there is no need for this dance. --- zjit/src/cruby.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index cc5bc35f8c8d10..ad7d4556786799 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -396,17 +396,10 @@ pub fn for_each_iseq(mut callback: F) { } /// Return a poison value to be set above the stack top to verify leafness. -#[cfg(not(test))] pub fn vm_stack_canary() -> u64 { unsafe { rb_vm_stack_canary() }.as_u64() } -/// Avoid linking the C function in `cargo test` -#[cfg(test)] -pub fn vm_stack_canary() -> u64 { - 0 -} - /// Opaque execution-context type from vm_core.h #[repr(C)] pub struct rb_execution_context_struct { From 6acb0fb49e3b9073aada5156a9f319fe8ac5cdde Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Wed, 8 Jul 2026 15:05:04 +0000 Subject: [PATCH 02/13] thread_pthread.c: cancel a Ractor's ready-queue entry when served directly A runnable thread whose Ractor was enqueued to the global ready queue can be served without a dequeue: a thread waking up in wait_running_turn hands its native thread over by a direct transfer, and the hot-thread path steals `running` back outright. Both leave the Ractor's grq entry behind, orphaned. The entry itself is harmless (the dequeuing nt sees a served Ractor and skips), but the accounting breaks: the next legitimate enqueue double-lists the one grq_node embedded in the sched, corrupting the queue (observed on master as pthread_mutex_lock EINVAL / crashes; about 1/20 runs of a Ractor x Thread x IO churn under a saturated shared-nt pool). Restore the invariant "enqueued <=> runnable and unserved" by cancelling the entry at both direct-service points (ractor_sched_cancel_enq). The grq_node is kept self-linked whenever it is not enqueued, so membership is read off the node itself: under the per-Ractor sched lock a self-linked read is decisive (enqueuers hold that lock too), and the common direct switch -- whose transition never enqueued -- pays no lock at all; a linked read can race only with a dequeue, so recheck under the grq lock before ccan_list_del_init. ractor_sched_enq now unconditionally rb_bugs on a still-linked node: the double-list is queue corruption either way, and a VM_CHECK_MODE-only assert cannot catch this race in practice (CHECK builds perturb the timing; the previous VM_CHECK-only scan never fired while plain builds reproduced the corruption). Co-Authored-By: Claude Fable 5 --- thread_pthread.c | 52 +++++++++++++++++++++++++++++++++++++++++------- thread_pthread.h | 4 ++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/thread_pthread.c b/thread_pthread.c index e1f3f7e668d9ba..c1d37508463369 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -331,6 +331,7 @@ static void timer_thread_wakeup(void); static void timer_thread_wakeup_locked(rb_vm_t *vm); static void timer_thread_wakeup_force(void); static void thread_sched_switch(rb_thread_t *cth, rb_thread_t *next_th); +static void ractor_sched_cancel_enq(rb_vm_t *vm, struct rb_thread_sched *sched); static void coroutine_transfer0(struct coroutine_context *transfer_from, struct coroutine_context *transfer_to, bool to_dead); @@ -854,6 +855,12 @@ thread_sched_wait_running_turn(struct rb_thread_sched *sched, rb_thread_t *th, b if (th->has_dedicated_nt && th == sched->runnable_hot_th && (sched->running == NULL || sched->running->has_dedicated_nt)) { RUBY_DEBUG_LOG("(nt) stealing: hot-th:%u. running:%u", rb_th_serial(th), rb_th_serial(sched->running)); + // th serves itself on its own nt, displacing the enqueued + // running thread back to the readyq: cancel the entry that was + // posted for it (a later dequeue would find this Ractor served + // and its next enqueue would double-list the node) + ractor_sched_cancel_enq(th->vm, sched); + // If there is a thread set to run, move it back to the front of the readyq if (sched->running != NULL) { rb_thread_t *running = sched->running; @@ -1230,6 +1237,7 @@ rb_thread_sched_init(struct rb_thread_sched *sched, bool atfork) ccan_list_head_init(&sched->readyq); sched->readyq_cnt = 0; + ccan_list_node_init(&sched->grq_node); // self-linked = not enqueued #if USE_MN_THREADS if (!atfork) sched->enable_mn_threads = true; // MN is enabled on Ractors @@ -1272,6 +1280,10 @@ thread_sched_switch0(struct coroutine_context *current_cont, rb_thread_t *next_t RUBY_DEBUG_LOG("next_th:%u", rb_th_serial(next_th)); + // this direct transfer serves next_th without a dequeue; cancel its + // Ractor's outstanding grq entry (no-op when nothing is enqueued) + ractor_sched_cancel_enq(next_th->vm, TH_SCHED(next_th)); + ruby_thread_set_native(next_th); native_thread_assign(nt, next_th); @@ -1307,6 +1319,29 @@ grq_size(rb_vm_t *vm, rb_ractor_t *cr) } #endif +// A direct service of a runnable thread (direct transfer or the hot-thread +// steal) bypasses the grq; cancel the Ractor's outstanding entry so that +// "enqueued <=> runnable and unserved" keeps holding. The caller holds the +// per-Ractor sched lock, so no concurrent enqueue can relink the node: a +// self-linked read needs no lock (the common case -- direct switches whose +// transition never enqueued). A linked read can race only with a dequeue, +// hence the recheck under the grq lock. +static void +ractor_sched_cancel_enq(rb_vm_t *vm, struct rb_thread_sched *sched) +{ + if (sched->grq_node.next != &sched->grq_node) { + ractor_sched_lock(vm, NULL); + { + if (sched->grq_node.next != &sched->grq_node) { + ccan_list_del_init(&sched->grq_node); + VM_ASSERT(vm->ractor.sched.grq_cnt > 0); + vm->ractor.sched.grq_cnt--; + } + } + ractor_sched_unlock(vm, NULL); + } +} + static void ractor_sched_enq(rb_vm_t *vm, rb_ractor_t *r) { @@ -1318,14 +1353,16 @@ ractor_sched_enq(rb_vm_t *vm, rb_ractor_t *r) ractor_sched_lock(vm, cr); { -#if VM_CHECK_MODE > 0 - // check if grq contains r - rb_ractor_t *tr; - ccan_list_for_each(&vm->ractor.sched.grq, tr, threads.sched.grq_node) { - VM_ASSERT(r != tr); + // Precondition: not already enqueued (the grq_node is self-linked). + // This holds because every service of a runnable-but-unserved thread + // either dequeues the entry (the nt scheduling loop) or cancels it + // (direct transfers / the hot-thread steal; see + // ractor_sched_cancel_enq) -- re-adding a linked node would corrupt + // the queue, so check unconditionally (a CHECK-mode-only assert + // would miss it: the race needs timing that CHECK builds perturb). + if (sched->grq_node.next != &sched->grq_node) { + rb_bug("ractor_sched_enq: already enqueued"); } -#endif - ccan_list_add_tail(&vm->ractor.sched.grq, &sched->grq_node); vm->ractor.sched.grq_cnt++; VM_ASSERT(grq_size(vm, cr) == vm->ractor.sched.grq_cnt); @@ -1389,6 +1426,7 @@ ractor_sched_deq(rb_vm_t *vm, rb_ractor_t *cr) VM_ASSERT(rb_current_execution_context(false) == NULL); if (r) { + ccan_list_node_init(&r->threads.sched.grq_node); // back to self-linked VM_ASSERT(vm->ractor.sched.grq_cnt > 0); vm->ractor.sched.grq_cnt--; RUBY_DEBUG_LOG("r:%d grq_cnt:%u", (int)rb_ractor_id(r), vm->ractor.sched.grq_cnt); diff --git a/thread_pthread.h b/thread_pthread.h index 9214385ba23c9a..6e39797c328ea7 100644 --- a/thread_pthread.h +++ b/thread_pthread.h @@ -141,6 +141,10 @@ struct rb_thread_sched { struct ccan_list_head readyq; int readyq_cnt; // ractor scheduling + // When not linked in vm->ractor.sched.grq, this node is kept + // self-linked (ccan_list_node_init), so "linked?" can be read off the + // node itself: enqueuers assert it, and direct transfers cancel an + // outstanding entry (see ractor_sched_cancel_enq). struct ccan_list_node grq_node; }; From 31b41ac5fbd3d6247a9830026852d539536e5121 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Wed, 8 Jul 2026 08:34:21 +0000 Subject: [PATCH 03/13] MN threads: a dying coroutine thread always returns to its native thread Redesign the teardown of M:N (coroutine) threads around the natural call structure nt_start -> co_start -> thread_start_func_2: when the thread body returns, its epilogue (coroutine_thread_terminated) finishes all rb_thread_t / rb_ractor_t business while th is still valid, and co_start then makes exactly one final transfer back to the native thread's own context (nt_start's loop), where the nt reclaims the dead coroutine's context. The resume itself proves the final transfer's register save completed, so the reclaim needs no list, no handshake and no extra locking. This deletes the zombie-threads machinery (VM-global list, sched.finished publish, the GC mark/reap pass) and closes its UAF window: publishing `finished` before the terminal transfer let the GC free the Thread wrapper -- whose coroutine context the transfer still saves into -- mid-teardown. * struct rb_thread_context bundles a coroutine thread's execution-owned resources: the coroutine_context (kept first, so th->sched.context points into the block) plus its machine stack, the stashed final-transfer target (nt), and a dead flag. co_start's epilogue sets the flag and transfers via the stashed nt, touching tctx alone -- th may already be collected by then. The nt loop passes every coroutine it resumes from to thread_sched_reclaim_from(), which frees dead ones (stack back to the pool). A live coroutine cannot be resumed elsewhere, die and be freed while the nt loop still inspects it: a live yield arrives with the sched lock handed off, and no other nt can schedule that thread again until the nt loop's own unlock. * coroutine_thread_terminated (run from thread_start_func_2, where the upstream non-coroutine teardown also runs, while th is still valid and listed) does: wake the next thread as upstream's thread_sched_to_dead does; stash th->nt into tctx and detach th->sched.context; leave the living set; clear the current ec; and, for a shared-nt non-last thread, enqueue the Ractor's next runnable thread (not earlier: waking it from to_dead would let another thread of the same Ractor execute while this teardown still runs). Everything that reads th happens before the living-set removal, after which the GC may collect th (and, for a Ractor's main thread, the Ractor). * interrupt_lock is now destroyed in thread_free instead of during teardown (thread_cleanup_func): while th is in its Ractor's living set, anyone -- terminate_all on the Ractor's terminating main thread, Thread#kill / #raise -- may lock it, and destroying it mid-teardown leaves a window where a concurrent interrupter locks a destroyed mutex (EINVAL; the upstream non-coroutine path has the same, smaller, window). At thread_free time the wrapper is unreachable AND off the living set (listed threads are marked), so no interrupter can exist. The atfork cleanup reinitializes the copied lock, which may have been held at the instant of fork. * The Thread wrapper's dfree only reclaims the context of a thread that never started; a terminated one cleared th->sched.context in its epilogue. VM shutdown needs no extra synchronization: past the living-set removal the epilogue touches only its own context block, the TLS and nt->nt_context, none of which VM destruct frees (SNT native threads and the machine-stack pool are intentionally not torn down at exit, as before). Verified: bootstraptest 2050 PASS; test_thread / test_ractor / test_fiber / test_gc / test_gc_compact / thread_queue 282 tests 0F0E; Ractor x Thread x IO teardown churn 150/150 + 60/60 (exercises DNT promotion, terminate_all racing dying threads, and the double-enqueue fixed in the previous commit); fork with live Ractors + child GC 20/20; ASAN clean including unreferenced Thread/Ractor collection (the old zombie window) and mixed stress; RGENGC_CHECK_MODE=2 btest 2050 + churn; TSan warning profile identical to master (the new functions appear in no race report); YJIT move-under-GC.stress; kill/join races; Thread object retention across Ractor death; exit-while-winding. Co-Authored-By: Claude Fable 5 --- ractor.c | 2 - thread.c | 23 +++++++-- thread_none.c | 18 +++---- thread_pthread.c | 122 +++++++++++++++++++++++++++++--------------- thread_pthread.h | 13 +++-- thread_pthread_mn.c | 116 ++++++++++++++++++++++++++++------------- thread_win32.c | 18 +++---- vm.c | 18 +++++-- vm_core.h | 6 +-- 9 files changed, 223 insertions(+), 113 deletions(-) diff --git a/ractor.c b/ractor.c index cb603f40b93389..fc6694c45c7f84 100644 --- a/ractor.c +++ b/ractor.c @@ -757,7 +757,6 @@ ractor_check_blocking(rb_ractor_t *cr, unsigned int remained_thread_cnt, const c } } -void rb_threadptr_remove(rb_thread_t *th); void rb_ractor_living_threads_remove(rb_ractor_t *cr, rb_thread_t *th) @@ -766,7 +765,6 @@ rb_ractor_living_threads_remove(rb_ractor_t *cr, rb_thread_t *th) RUBY_DEBUG_LOG("r->threads.cnt:%d--", cr->threads.cnt); ractor_check_blocking(cr, cr->threads.cnt - 1, __FILE__, __LINE__); - rb_threadptr_remove(th); if (cr->threads.cnt == 1) { vm_remove_ractor(th->vm, cr); diff --git a/thread.c b/thread.c index 2e75c2081e2ab4..72839418fc5074 100644 --- a/thread.c +++ b/thread.c @@ -535,10 +535,19 @@ thread_cleanup_func(void *th_ptr, int atfork) if (atfork) { native_thread_destroy_atfork(th->nt); th->nt = NULL; + // The copied interrupt_lock may have been held at the moment of + // fork (interrupters run concurrently); reinitialize it so that + // thread_free's destroy is well-defined in the child. + rb_native_mutex_initialize(&th->interrupt_lock); return; } - rb_native_mutex_destroy(&th->interrupt_lock); + // interrupt_lock is destroyed in thread_free: while th is in its + // Ractor's living set, anyone (terminate_all on the Ractor's main + // thread, Thread#kill/#raise) may lock it -- and the living set keeps + // the Thread object marked, so it cannot reach thread_free while + // listed. Destroying it anywhere during teardown leaves a window where + // a concurrent interrupter locks a destroyed mutex (EINVAL). } void @@ -807,6 +816,16 @@ thread_start_func_2(rb_thread_t *th, VALUE *stack_start) thread_cleanup_func(th, FALSE); VM_ASSERT(th->ec->vm_stack == NULL); +#if defined(USE_MN_THREADS) && USE_MN_THREADS + if (th_has_coroutine(th)) { + // Run the coroutine thread's epilogue here, while th is still valid; + // co_start then only makes the final transfer (see + // coroutine_thread_terminated in thread_pthread_mn.c). + coroutine_thread_terminated(th); + return 0; + } +#endif + if (th->invoke_type == thread_invoke_type_ractor_proc) { // after rb_ractor_living_threads_remove() // GC will happen anytime and this ractor can be collected (and destroy GVL). @@ -908,8 +927,6 @@ thread_create_core(VALUE thval, struct thread_create_params *params) RBASIC_CLEAR_CLASS(th->pending_interrupt_mask_stack); } - rb_native_mutex_initialize(&th->interrupt_lock); - RUBY_DEBUG_LOG("r:%u th:%u", rb_ractor_id(th->ractor), rb_th_serial(th)); rb_ractor_living_threads_insert(th->ractor, th); diff --git a/thread_none.c b/thread_none.c index cb844148e1ff27..1a94559b7e04c9 100644 --- a/thread_none.c +++ b/thread_none.c @@ -311,17 +311,7 @@ rb_ractor_sched_barrier_join(rb_vm_t *vm, rb_ractor_t *cr) // do nothing } -void -rb_threadptr_remove(rb_thread_t *th) -{ - // do nothing -} -void -rb_thread_sched_mark_zombies(rb_vm_t *vm) -{ - // do nothing -} bool rb_thread_lock_native_thread(void) @@ -342,3 +332,11 @@ rb_thread_malloc_stack_set(rb_thread_t *th, void *stack, size_t stack_size) } #endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */ + +void +rb_thread_sched_wait_winding(rb_vm_t *vm) +{ + // no coroutine (M:N) threads on this implementation: nothing winds down + // after leaving the living set (see thread_pthread.c) + (void)vm; +} diff --git a/thread_pthread.c b/thread_pthread.c index c1d37508463369..129f1f150d33e4 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -332,7 +332,23 @@ static void timer_thread_wakeup_locked(rb_vm_t *vm); static void timer_thread_wakeup_force(void); static void thread_sched_switch(rb_thread_t *cth, rb_thread_t *next_th); static void ractor_sched_cancel_enq(rb_vm_t *vm, struct rb_thread_sched *sched); -static void coroutine_transfer0(struct coroutine_context *transfer_from, +#if USE_MN_THREADS +// A coroutine thread's execution context: the coroutine_context (first, so +// th->sched.context points at the whole block) plus what is needed to free +// it without the rb_thread_t. Owned by the execution: the dying thread marks +// it dead before its final transfer, and whichever context RESUMES from that +// transfer frees it -- the resume itself proves the transfer's register save +// into this block has completed, so no further synchronization is needed. +struct rb_thread_context { + struct coroutine_context co; // must be first + void *stack; // the coroutine machine stack (pool stack) + struct rb_native_thread *nt; // final transfer target, stashed at termination + bool dead; +}; + +static bool thread_sched_reclaim_from(struct coroutine_context *returning_from); +#endif +static struct coroutine_context *coroutine_transfer0(struct coroutine_context *transfer_from, struct coroutine_context *transfer_to, bool to_dead); #define thread_sched_dump(s) thread_sched_dump_(__FILE__, __LINE__, s) @@ -1053,6 +1069,10 @@ thread_sched_to_dead_common(struct rb_thread_sched *sched, rb_thread_t *th) RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED, th); + // A dying coroutine thread (will_switch=true here) does NOT wake the + // next thread now: it is still winding down (co_start's epilogue), and + // the same Ractor must not have two threads executing at once. The + // epilogue enqueues the Ractor after its last rb_ractor_t access. thread_sched_wakeup_next_thread(sched, th, !th_has_dedicated_nt(th)); RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_EXITED, th); @@ -1244,7 +1264,7 @@ rb_thread_sched_init(struct rb_thread_sched *sched, bool atfork) #endif } -static void +static struct coroutine_context * coroutine_transfer0(struct coroutine_context *transfer_from, struct coroutine_context *transfer_to, bool to_dead) { #ifdef RUBY_ASAN_ENABLED @@ -1259,7 +1279,6 @@ coroutine_transfer0(struct coroutine_context *transfer_from, struct coroutine_co __tsan_switch_to_fiber(transfer_to->tsan_fiber, 0); #endif - RBIMPL_ATTR_MAYBE_UNUSED() struct coroutine_context *returning_from = coroutine_transfer(transfer_from, transfer_to); /* if to_dead was passed, the caller is promising that this coroutine is finished and it should @@ -1270,9 +1289,10 @@ coroutine_transfer0(struct coroutine_context *transfer_from, struct coroutine_co (const void**)&returning_from->stack_base, &returning_from->stack_size); #endif + return returning_from; } -static void +static struct coroutine_context * thread_sched_switch0(struct coroutine_context *current_cont, rb_thread_t *next_th, struct rb_native_thread *nt, bool to_dead) { VM_ASSERT(!nt->dedicated); @@ -1287,7 +1307,7 @@ thread_sched_switch0(struct coroutine_context *current_cont, rb_thread_t *next_t ruby_thread_set_native(next_th); native_thread_assign(nt, next_th); - coroutine_transfer0(current_cont, next_th->sched.context, to_dead); + return coroutine_transfer0(current_cont, next_th->sched.context, to_dead); } static void @@ -1319,6 +1339,18 @@ grq_size(rb_vm_t *vm, rb_ractor_t *cr) } #endif +// ruby_vm_destruct: wait until no native thread is between a coroutine +// epilogue and its reclaim -- past that point the reclaim frees through the +// (about to be destroyed) objspace and reads the (about to be unset) VM. +// Runs without the VM lock, which the epilogue needs to progress. +void +rb_thread_sched_wait_winding(rb_vm_t *vm) +{ + while (RUBY_ATOMIC_LOAD(vm->ractor.sched.winding_cnt) > 0) { + native_thread_yield(); + } +} + // A direct service of a runnable thread (direct transfer or the hot-thread // steal) bypasses the grq; cancel the Ractor's outstanding entry so that // "enqueued <=> runnable and unserved" keeps holding. The caller holds the @@ -2390,19 +2422,34 @@ nt_start(void *ptr) if (r) { struct rb_thread_sched *sched = &r->threads.sched; + bool locked = true; + thread_sched_lock(sched, NULL); { rb_thread_t *next_th = sched->running; if (next_th && next_th->nt == NULL) { RUBY_DEBUG_LOG("nt:%d next_th:%d", (int)nt->serial, (int)next_th->serial); +#if USE_MN_THREADS + struct coroutine_context *from = thread_sched_switch0(nt->nt_context, next_th, nt, false); + if (thread_sched_reclaim_from(from)) { + // A terminal transfer: the dying thread released + // the sched lock before transferring (its Ractor + // may be gone by now); we resumed with NO lock + // held and must not touch the sched again. + locked = false; + } +#else thread_sched_switch0(nt->nt_context, next_th, nt, false); +#endif } else { RUBY_DEBUG_LOG("no schedulable threads -- next_th:%p", next_th); } } - thread_sched_unlock(sched, NULL); + if (locked) { + thread_sched_unlock(sched, NULL); + } } else { // timeout -> deleted. @@ -2423,26 +2470,30 @@ static int native_thread_create_shared(rb_thread_t *th); #if USE_MN_THREADS static void nt_free_stack(void *mstack); -#endif -void -rb_threadptr_remove(rb_thread_t *th) + +// Reclaim the coroutine context the nt scheduling loop just resumed from, +// if its thread terminated. A dying thread always returns to its native +// thread's own context (co_start's epilogue), so this is the only place +// terminal transfers arrive -- and our running here proves the transfer's +// register save into the block has completed. Returns true for a terminal +// transfer, whose dying thread RELEASED the sched lock before transferring. +static bool +thread_sched_reclaim_from(struct coroutine_context *returning_from) { -#if USE_MN_THREADS - if (th->sched.malloc_stack) { - // dedicated - return; - } - else { - rb_vm_t *vm = th->vm; - th->sched.finished = false; + struct rb_thread_context *tctx = (struct rb_thread_context *)returning_from; - RB_VM_LOCKING() { - ccan_list_add(&vm->ractor.sched.zombie_threads, &th->sched.node.zombie_threads); - } + if (tctx != NULL && tctx->dead) { + nt_free_stack(tctx->stack); + 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 + RUBY_ATOMIC_DEC(GET_VM()->ractor.sched.winding_cnt); + return true; } -#endif + return false; } +#endif void rb_threadptr_sched_free(rb_thread_t *th) @@ -2453,14 +2504,16 @@ rb_threadptr_sched_free(rb_thread_t *th) SIZED_FREE_N((VALUE *)th->sched.context_stack, th->sched.context_stack_size); native_thread_destroy(th->nt); } - else { - nt_free_stack(th->sched.context_stack); + else if (th->sched.context != NULL) { + // a coroutine thread that never reached its epilogue (never started); + // a terminated one is reclaimed by whoever resumed from its final + // transfer (thread_sched_reclaim_from), and cleared this pointer. + struct rb_thread_context *tctx = (struct rb_thread_context *)th->sched.context; + nt_free_stack(tctx->stack); + SIZED_FREE(tctx); + th->sched.context = NULL; // TODO: how to free nt and nt->altstack? } - - SIZED_FREE(th->sched.context); - th->sched.context = NULL; - // VM_ASSERT(th->sched.context == NULL); #else SIZED_FREE_N((VALUE *)th->sched.context_stack, th->sched.context_stack_size); native_thread_destroy(th->nt); @@ -2469,21 +2522,6 @@ rb_threadptr_sched_free(rb_thread_t *th) th->nt = NULL; } -void -rb_thread_sched_mark_zombies(rb_vm_t *vm) -{ - if (!ccan_list_empty(&vm->ractor.sched.zombie_threads)) { - rb_thread_t *zombie_th, *next_zombie_th; - ccan_list_for_each_safe(&vm->ractor.sched.zombie_threads, zombie_th, next_zombie_th, sched.node.zombie_threads) { - if (zombie_th->sched.finished) { - ccan_list_del_init(&zombie_th->sched.node.zombie_threads); - } - else { - rb_gc_mark(zombie_th->self); - } - } - } -} static int native_thread_create(rb_thread_t *th) diff --git a/thread_pthread.h b/thread_pthread.h index 6e39797c328ea7..cbc2d5ee4abf0a 100644 --- a/thread_pthread.h +++ b/thread_pthread.h @@ -68,14 +68,11 @@ struct rb_thread_sched_item { // locked by vm->ractor.sched.lock struct ccan_list_node running_threads; - // connected to vm->ractor.sched.zombie_threads - struct ccan_list_node zombie_threads; } node; struct rb_thread_sched_waiting waiting_reason; uint32_t event_serial; - bool finished; bool malloc_stack; void *context_stack; size_t context_stack_size; @@ -148,6 +145,16 @@ struct rb_thread_sched { struct ccan_list_node grq_node; }; +struct rb_thread_context; + +// A coroutine (M:N) thread's teardown runs coroutine_thread_terminated +// instead of the dedicated-thread path in thread_start_func_2; see the +// comments there and in thread_pthread_mn.c. th->sched.context is cleared in +// that epilogue, so this also reads as "did not tear down yet". +// (Only meaningful when USE_MN_THREADS -- gate uses accordingly; the macro +// itself is a plain pointer test and always compiles.) +#define th_has_coroutine(th) ((th)->sched.context != NULL) + #ifdef RB_THREAD_LOCAL_SPECIFIER NOINLINE(void rb_current_ec_set(struct rb_execution_context_struct *)); diff --git a/thread_pthread_mn.c b/thread_pthread_mn.c index c9e6649832bc8c..2efde13702e420 100644 --- a/thread_pthread_mn.c +++ b/thread_pthread_mn.c @@ -447,6 +447,73 @@ 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. +static void +coroutine_thread_terminated(rb_thread_t *th) +{ + struct rb_thread_context *tctx = (struct rb_thread_context *)th->sched.context; + struct rb_thread_sched *sched = TH_SCHED(th); + rb_ractor_t *r = th->ractor; + bool last = (th->invoke_type == thread_invoke_type_ractor_proc); + bool is_dnt = th_has_dedicated_nt(th); + + // VM destruct tears down the heap and then unsets ruby_current_vm_ptr; + // this native thread still frees the dead context afterwards (the nt + // loop's reclaim uses ruby_xfree and the stack-pool geometry via + // GET_VM()). Make destruct wait until the reclaim finished. (Observed: + // an assert_separately child exiting right after a Ractor finished + // crashed at GET_VM()->default_params, offset 0x2600, on two arches.) + RUBY_ATOMIC_INC(th->vm->ractor.sched.winding_cnt); + + rb_thread_t *wake_th; + + thread_sched_lock(sched, th); + { + // designate the successor (running = next from readyq, or NULL); for + // a dedicated nt (will_switch false) this also enqueues the Ractor. + 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. + wake_th = is_dnt ? NULL : sched->running; + + tctx->nt = th->nt; // stash the final transfer target for co_start + native_thread_assign(NULL, th); + th->sched.context = NULL; // the wrapper's dfree must not reclaim tctx + } + 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) { + rb_current_ec_set(NULL); // TLS only; r may be collectable already + } + else { + rb_ractor_set_current_ec(r, NULL); // r alive: it has other threads + + if (wake_th && wake_th->nt == NULL) { + // enqueue the successor designated above -- exactly once per + // "runnable but unserved" period, by its designator. + thread_sched_lock(sched, NULL); + ractor_sched_enq(wake_th->vm, r); + thread_sched_unlock(sched, NULL); + } + } +} + #ifdef __APPLE__ # define co_start ruby_coroutine_start #else @@ -475,38 +542,14 @@ co_start(struct coroutine_context *from, struct coroutine_context *self) RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_RESUMED, th); call_thread_start_func_2(th); } - thread_sched_lock(sched, NULL); - - RUBY_DEBUG_LOG("terminated th:%d", (int)th->serial); - - // Thread is terminated - - struct rb_native_thread *nt = th->nt; - bool is_dnt = th_has_dedicated_nt(th); - native_thread_assign(NULL, th); - rb_ractor_set_current_ec(th->ractor, NULL); - - if (is_dnt) { - // SNT became DNT while running. Just return to the nt_context - - th->sched.finished = true; - coroutine_transfer0(self, nt->nt_context, true); - } - else { - rb_thread_t *next_th = sched->running; - - if (next_th && !next_th->nt) { - // switch to the next thread - thread_sched_set_unlocked(sched, NULL); - th->sched.finished = true; - thread_sched_switch0(th->sched.context, next_th, nt, true); - } - else { - // switch to the next Ractor - th->sched.finished = true; - coroutine_transfer0(self, nt->nt_context, true); - } - } + // Thread is terminated. coroutine_thread_terminated (run from + // thread_start_func_2 while th was still valid) already left the living + // set and stashed the transfer target, so th / its Ractor may already be + // collected. Only tctx is touched here: mark it dead and transfer back to + // this native thread's own context, where the nt loop reclaims tctx. + struct rb_thread_context *tctx = (struct rb_thread_context *)self; + tctx->dead = true; + coroutine_transfer0(&tctx->co, tctx->nt->nt_context, true); rb_bug("unreachable"); } @@ -533,9 +576,12 @@ native_thread_create_shared(rb_thread_t *th) th->sched.context_stack = machine_stack; th->sched.context_stack_size = machine_stack_size; - th->sched.context = ruby_xmalloc(sizeof(struct coroutine_context)); - coroutine_initialize(th->sched.context, co_start, machine_stack, machine_stack_size); - th->sched.context->argument = th; + struct rb_thread_context *tctx = ruby_xmalloc(sizeof(struct rb_thread_context)); + tctx->stack = machine_stack; + tctx->dead = false; + th->sched.context = &tctx->co; + coroutine_initialize(&tctx->co, co_start, machine_stack, machine_stack_size); + tctx->co.argument = th; RUBY_DEBUG_LOG("th:%u vm_stack:%p machine_stack:%p", rb_th_serial(th), vm_stack, machine_stack); thread_sched_to_ready(TH_SCHED(th), th); diff --git a/thread_win32.c b/thread_win32.c index a2ce3b9d155afd..5fc73b91b2e664 100644 --- a/thread_win32.c +++ b/thread_win32.c @@ -910,17 +910,7 @@ rb_threadptr_sched_free(rb_thread_t *th) ruby_xfree(th->sched.vm_stack); } -void -rb_threadptr_remove(rb_thread_t *th) -{ - // do nothing -} -void -rb_thread_sched_mark_zombies(rb_vm_t *vm) -{ - // do nothing -} static bool vm_barrier_finish_p(rb_vm_t *vm) @@ -1031,3 +1021,11 @@ rb_thread_malloc_stack_set(rb_thread_t *th, void *stack, size_t stack_size) } #endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */ + +void +rb_thread_sched_wait_winding(rb_vm_t *vm) +{ + // no coroutine (M:N) threads on this implementation: nothing winds down + // after leaving the living set (see thread_pthread.c) + (void)vm; +} diff --git a/vm.c b/vm.c index 82f595dc62d4dd..85a22d40048261 100644 --- a/vm.c +++ b/vm.c @@ -3339,7 +3339,6 @@ vm_mark_negative_cme(VALUE val, void *dmy) return ID_TABLE_CONTINUE; } -void rb_thread_sched_mark_zombies(rb_vm_t *vm); void rb_vm_mark(void *ptr) @@ -3395,8 +3394,6 @@ rb_vm_mark(void *ptr) } } } - - rb_thread_sched_mark_zombies(vm); } RUBY_MARK_LEAVE("vm"); @@ -3418,6 +3415,14 @@ void rb_objspace_free_objects(void *objspace); int ruby_vm_destruct(rb_vm_t *vm) { + { + // wait for native threads still winding down a dead coroutine: their + // reclaim frees through the objspace this function is about to + // destroy (see coroutine_thread_terminated) + void rb_thread_sched_wait_winding(rb_vm_t *vm); + rb_thread_sched_wait_winding(vm); + } + RUBY_FREE_ENTER("vm"); ruby_vm_during_cleanup = true; @@ -3878,6 +3883,9 @@ thread_free(void *ptr) RUBY_FREE_ENTER("thread"); rb_threadptr_sched_free(th); + // destroyed here rather than during teardown: nothing can interrupt a + // thread that is unreachable and off its Ractor's living set + rb_native_mutex_destroy(&th->interrupt_lock); if (th->locking_mutex != Qfalse) { rb_bug("thread_free: locking_mutex must be NULL (%p:%p)", (void *)th, (void *)th->locking_mutex); @@ -3993,6 +4001,10 @@ th_init(rb_thread_t *th, VALUE self, rb_vm_t *vm) th->self = self; ccan_list_head_init(&th->interrupt_exec_tasks); + // initialized here (not at thread creation) so that every Thread object + // -- including allocated-but-never-started ones -- owns a valid mutex: + // thread_free destroys it unconditionally + rb_native_mutex_initialize(&th->interrupt_lock); rb_threadptr_root_fiber_setup(th); diff --git a/vm_core.h b/vm_core.h index 04bf0f81fbf442..c72933ae21f1e5 100644 --- a/vm_core.h +++ b/vm_core.h @@ -724,6 +724,7 @@ typedef struct rb_vm_struct { unsigned int max_cpu; struct ccan_list_head grq; // // Global Ready Queue + rb_atomic_t winding_cnt; // native threads between a coroutine epilogue and its reclaim; ruby_vm_destruct waits for 0 unsigned int grq_cnt; // running threads @@ -732,8 +733,6 @@ typedef struct rb_vm_struct { // threads which switch context by timeslice struct ccan_list_head timeslice_threads; - struct ccan_list_head zombie_threads; - // true if timeslice timer is not enable bool timeslice_wait_inf; @@ -2006,9 +2005,6 @@ rb_vm_living_threads_init(rb_vm_t *vm) { ccan_list_head_init(&vm->workqueue); ccan_list_head_init(&vm->ractor.set); -#ifdef RUBY_THREAD_PTHREAD_H - ccan_list_head_init(&vm->ractor.sched.zombie_threads); -#endif } typedef int rb_backtrace_iter_func(void *, VALUE, int, VALUE); From 880cc0a274702ad26a8bd8114b246ee9e8babba2 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Wed, 8 Jul 2026 09:31:21 +0000 Subject: [PATCH 04/13] thread_pthread.c: reset grq_cnt at fork thread_sched_atfork empties the global ready queue with ccan_list_head_init but leaves vm->ractor.sched.grq_cnt at whatever the parent had, so a child forked while any Ractor was enqueued keeps a stale non-zero count for an empty list. VM_CHECK_MODE builds then fail "grq_size(vm, cr) == vm->ractor.sched.grq_cnt" in ractor_sched_deq (reproducible on master with RUBY_MN_THREADS=1: fork in a loop that also creates Ractors); on regular builds the counter only feeds assertions and debug logs, so the corruption stays silent. Co-Authored-By: Claude Fable 5 --- thread_pthread.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/thread_pthread.c b/thread_pthread.c index 129f1f150d33e4..4912711506b277 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1742,6 +1742,11 @@ thread_sched_atfork(struct rb_thread_sched *sched) rb_native_cond_initialize(&vm->ractor.sched.barrier_release_cond); ccan_list_head_init(&vm->ractor.sched.grq); + vm->ractor.sched.grq_cnt = 0; // the list was just emptied; reset the count with it + // 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. + vm->ractor.sched.winding_cnt = 0; ccan_list_head_init(&vm->ractor.sched.timeslice_threads); ccan_list_head_init(&vm->ractor.sched.running_threads); From 841ea9d5024e1aeb4be4d03734214ad22e0d7607 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Tue, 7 Jul 2026 20:27:04 +0100 Subject: [PATCH 05/13] Inline GC fastpath in ZJIT for newhash instruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This extends the ZJIT GC fast path to the newhash instruction for empty hashes. This was verified in a stripped down benchmarking environment, using the following micro-benchmark in ruby-bench: ``` def run(max) i = 0 while i < max h = {} i += 1 end end require_relative '../harness/loader.rb' run_benchmark(50) do 50.times do run(500_000) end end ``` This patch resulted in a ~30% speedup. ``` master: ruby 4.1.0dev (2026-07-07T14:04:07Z master c51b1596c0) +ZJIT +PRISM [x86_64-linux] experiment: ruby 4.1.0dev (2026-07-07T19:27:04Z mvh-zjit-inline-fa.. 60730f1aa1) +ZJIT +PRISM [x86_64-linux] ----- ------------ --------------- ------------------ ----------------- bench master (ms) experiment (ms) experiment 1st itr master/experiment hash 682.3 ± 0.7% 518.0 ± 1.0% 1.082 1.317 ----- ------------ --------------- ------------------ ----------------- Legend: - experiment 1st itr: ratio of master/experiment time for the first benchmarking iteration. - master/experiment: ratio of master/experiment time. Higher is better for experiment. Above 1 represents a speedup. ``` --- hash.c | 23 ++++++++++++++++++++++- test/ruby/test_zjit.rb | 21 +++++++++++++++++++++ zjit.h | 2 ++ zjit/src/codegen.rs | 14 +++++++++++++- zjit/src/codegen/gc_fastpath.rs | 4 ++-- zjit/src/cruby.rs | 3 +++ 6 files changed, 63 insertions(+), 4 deletions(-) diff --git a/hash.c b/hash.c index 41d5a1dfe61bdd..4faa59686476f2 100644 --- a/hash.c +++ b/hash.c @@ -49,6 +49,7 @@ #include "ruby/ractor.h" #include "vm_sync.h" #include "builtin.h" +#include "zjit.h" /* Flags of RHash * @@ -1440,10 +1441,16 @@ compact_after_delete(VALUE hash) } } +static inline size_t +hash_slot_size(bool st) +{ + return sizeof(struct RHash) + (st ? sizeof(st_table) : sizeof(ar_table)); +} + static VALUE hash_alloc_flags(VALUE klass, VALUE flags, VALUE ifnone, bool st) { - const size_t size = sizeof(struct RHash) + (st ? sizeof(st_table) : sizeof(ar_table)); + const size_t size = hash_slot_size(st); VALUE hash = rb_newobj_of(klass, T_HASH | flags, size); return rb_hash_set_ifnone(hash, ifnone); } @@ -1455,6 +1462,20 @@ hash_alloc(VALUE klass) return hash_alloc_flags(klass, 0, Qnil, sizeof(st_table) > sizeof(ar_table)); } +#if USE_ZJIT +size_t +rb_zjit_hash_new_size(void) +{ + return hash_slot_size(sizeof(st_table) > sizeof(ar_table)); +} + +size_t +rb_zjit_offset_rhash_ifnone(void) +{ + return offsetof(struct RHash, ifnone); +} +#endif + static VALUE empty_hash_alloc(VALUE klass) { diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 18baf3832abb72..57b120ff7f0bf6 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -404,6 +404,27 @@ def allocate_array } end + def test_regression_gc_stress_new_hash_fast_path + assert_compiles ':ok', %q{ + def make = {} + + begin + GC.stress = true + 10.times { make } + h = make + raise "wrong class" unless h.instance_of?(Hash) + raise "wrong size" unless h.size == 0 + raise "wrong default" unless h.default.nil? + h[:a] = 1 + h[:b] = 2 + raise "not writable" unless h == { a: 1, b: 2 } + :ok + ensure + GC.stress = false + end + } + end + def test_exit_tracing # Smoke test: --zjit-trace-exits writes a Fuchsia trace (.fxt) file to /tmp assert_compiles('true', <<~RUBY, extra_args: ['--zjit-trace-exits']) diff --git a/zjit.h b/zjit.h index b4e399ba2a1f51..2a3a4e2b01c1a3 100644 --- a/zjit.h +++ b/zjit.h @@ -77,6 +77,8 @@ void rb_zjit_invalidate_no_singleton_class(VALUE klass); void rb_zjit_invalidate_root_box(void); void rb_zjit_jit_frame_update_references(zjit_jit_frame_t *jit_frame); void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp); +size_t rb_zjit_hash_new_size(void); +size_t rb_zjit_offset_rhash_ifnone(void); // Special value for cfp->jit_return that means "this is a C method frame, use // rb_zjit_c_frame as the JITFrame". We don't control the native stack layout diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index a6239074a02ee9..4fb28a0d9ee4a8 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -2269,7 +2269,19 @@ fn gen_new_hash( ) -> lir::Opnd { if elements.is_empty() { gen_prepare_leaf_call_with_gc(asm, state); - asm_ccall!(asm, rb_hash_new,) + + let alloc_size = unsafe { rb_zjit_hash_new_size() }; + let flags = RUBY_T_HASH as u64; + let klass = unsafe { rb_cHash }; + let ifnone_offset: i32 = unsafe { rb_zjit_offset_rhash_ifnone() } + .try_into() + .expect("RHash ifnone offset fits in i32"); + + let hash = gc_fastpath::gc_fast_path_new_obj(jit, asm, alloc_size, flags, klass, |asm| { + asm_ccall!(asm, rb_hash_new,) + }); + asm.store(Opnd::mem(VALUE_BITS, hash, ifnone_offset), Qnil.into()); + hash } else { gen_prepare_non_leaf_call(jit, asm, state); diff --git a/zjit/src/codegen/gc_fastpath.rs b/zjit/src/codegen/gc_fastpath.rs index ad109440d14d8b..6d63f2d2f2fbfa 100644 --- a/zjit/src/codegen/gc_fastpath.rs +++ b/zjit/src/codegen/gc_fastpath.rs @@ -193,7 +193,7 @@ fn emit_default_new_obj_fastpath( asm.store(Opnd::mem(64, gc_cache, cursor_offset), new_cursor); asm.store( Opnd::mem(VALUE_BITS, cursor, RUBY_OFFSET_RBASIC_FLAGS), - fastpath.flags.into(), + Opnd::UImm(fastpath.flags.0 as u64), ); asm.store( Opnd::mem(VALUE_BITS, cursor, RUBY_OFFSET_RBASIC_KLASS), @@ -309,7 +309,7 @@ fn emit_mmtk_new_obj_fastpath( asm.store(Opnd::mem(VALUE_BITS, aligned, 0), Opnd::UImm(payload_size)); asm.store( Opnd::mem(VALUE_BITS, obj, RUBY_OFFSET_RBASIC_FLAGS), - fastpath.flags.into(), + Opnd::UImm(fastpath.flags.0 as u64), ); asm.store( Opnd::mem(VALUE_BITS, obj, RUBY_OFFSET_RBASIC_KLASS), diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index ad7d4556786799..24ba1a5c52f860 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -124,6 +124,9 @@ unsafe extern "C" { pub fn rb_zjit_offset_ractor_newobj_cache() -> usize; + pub fn rb_zjit_hash_new_size() -> usize; + pub fn rb_zjit_offset_rhash_ifnone() -> usize; + // Floats within range will be encoded without creating objects in the heap. // (Range is 0x3000000000000001 to 0x4fffffffffffffff (1.7272337110188893E-77 to 2.3158417847463237E+77). pub fn rb_float_new(d: f64) -> VALUE; From 251a19f69ed18ba3e21fa4fde6354f36a03814cc Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Tue, 7 Jul 2026 21:13:20 +0100 Subject: [PATCH 06/13] Fix depends --- depend | 1 + 1 file changed, 1 insertion(+) diff --git a/depend b/depend index b375948348d28a..980de8863a3d7b 100644 --- a/depend +++ b/depend @@ -6731,6 +6731,7 @@ hash.$(OBJEXT): {$(VPATH)}vm_core.h hash.$(OBJEXT): {$(VPATH)}vm_debug.h hash.$(OBJEXT): {$(VPATH)}vm_opts.h hash.$(OBJEXT): {$(VPATH)}vm_sync.h +hash.$(OBJEXT): {$(VPATH)}zjit.h imemo.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h imemo.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h imemo.$(OBJEXT): $(CCAN_DIR)/list/list.h From 578a3813274613d17403f0452e70086e2fe5a582 Mon Sep 17 00:00:00 2001 From: eightbitraptor Date: Tue, 7 Jul 2026 21:20:22 +0100 Subject: [PATCH 07/13] Follow recommended Enum message style Co-authored-by: Tavian Barnes --- zjit/src/codegen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 4fb28a0d9ee4a8..d409e95cb80d07 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -2275,7 +2275,7 @@ fn gen_new_hash( let klass = unsafe { rb_cHash }; let ifnone_offset: i32 = unsafe { rb_zjit_offset_rhash_ifnone() } .try_into() - .expect("RHash ifnone offset fits in i32"); + .expect("RHash ifnone offset should fit in i32"); let hash = gc_fastpath::gc_fast_path_new_obj(jit, asm, alloc_size, flags, klass, |asm| { asm_ccall!(asm, rb_hash_new,) From ea2e8bb1110a090f038f4168f7b435bf37b54b4e Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Tue, 7 Jul 2026 23:05:22 +0100 Subject: [PATCH 08/13] Add a comment explaining the redundant store --- zjit/src/codegen.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index d409e95cb80d07..1feac5d07cc8aa 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -2280,6 +2280,9 @@ fn gen_new_hash( let hash = gc_fastpath::gc_fast_path_new_obj(jit, asm, alloc_size, flags, klass, |asm| { asm_ccall!(asm, rb_hash_new,) }); + // TODO: this runs on the slow path too, where rb_hash_new already set + // ifnone. A fast-path-only init hook in gc_fast_path_new_obj would avoid + // the redundant store and be reusable for other types. asm.store(Opnd::mem(VALUE_BITS, hash, ifnone_offset), Qnil.into()); hash } else { From 51384f4bf9c32f5d2fc057d9dc06b5d7b0240bf4 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Wed, 8 Jul 2026 16:00:28 +0100 Subject: [PATCH 09/13] Generate rb_zjit_hash_new_size binding with bindgen --- zjit/bindgen/src/main.rs | 1 + zjit/src/cruby.rs | 1 - zjit/src/cruby_bindings.inc.rs | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 4bad82b4e1e9c8..9a7b4a15644d07 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -112,6 +112,7 @@ fn main() { .allowlist_function("rb_protect") .allowlist_function("rb_zjit_profile_disable") .allowlist_function("rb_zjit_insn_to_bare_insn") + .allowlist_function("rb_zjit_hash_new_size") // For crashing .allowlist_function("rb_bug") diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 24ba1a5c52f860..5621f516e581ab 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -124,7 +124,6 @@ unsafe extern "C" { pub fn rb_zjit_offset_ractor_newobj_cache() -> usize; - pub fn rb_zjit_hash_new_size() -> usize; pub fn rb_zjit_offset_rhash_ifnone() -> usize; // Floats within range will be encoded without creating objects in the heap. diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 3a321e04062360..de75402a0b3879 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2240,6 +2240,7 @@ unsafe extern "C" { pub fn rb_iseqw_to_iseq(iseqw: VALUE) -> *const rb_iseq_t; pub fn rb_iseq_label(iseq: *const rb_iseq_t) -> VALUE; pub fn rb_iseq_defined_string(type_: defined_type) -> VALUE; + pub fn rb_zjit_hash_new_size() -> usize; pub fn rb_profile_frames( start: ::std::os::raw::c_int, limit: ::std::os::raw::c_int, From ff8e91304841502b9b1c2d06f8d87f9ee8de6d3f Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Wed, 8 Jul 2026 16:02:01 +0100 Subject: [PATCH 10/13] Inline RHash ifnone offset into jit_bindgen_constants --- depend | 2 ++ hash.c | 6 ------ jit.c | 4 ++++ yjit/src/cruby_bindings.inc.rs | 1 + zjit.h | 1 - zjit/src/codegen.rs | 5 +---- zjit/src/cruby.rs | 2 -- zjit/src/cruby_bindings.inc.rs | 1 + 8 files changed, 9 insertions(+), 13 deletions(-) diff --git a/depend b/depend index 980de8863a3d7b..bfde95272b0d95 100644 --- a/depend +++ b/depend @@ -7849,6 +7849,7 @@ jit.$(OBJEXT): $(top_srcdir)/internal/compile.h jit.$(OBJEXT): $(top_srcdir)/internal/compilers.h jit.$(OBJEXT): $(top_srcdir)/internal/fixnum.h jit.$(OBJEXT): $(top_srcdir)/internal/gc.h +jit.$(OBJEXT): $(top_srcdir)/internal/hash.h jit.$(OBJEXT): $(top_srcdir)/internal/imemo.h jit.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h jit.$(OBJEXT): $(top_srcdir)/internal/serial.h @@ -8014,6 +8015,7 @@ jit.$(OBJEXT): {$(VPATH)}internal/has/declspec_attribute.h jit.$(OBJEXT): {$(VPATH)}internal/has/extension.h jit.$(OBJEXT): {$(VPATH)}internal/has/feature.h jit.$(OBJEXT): {$(VPATH)}internal/has/warning.h +jit.$(OBJEXT): {$(VPATH)}internal/hash.h jit.$(OBJEXT): {$(VPATH)}internal/intern/array.h jit.$(OBJEXT): {$(VPATH)}internal/intern/bignum.h jit.$(OBJEXT): {$(VPATH)}internal/intern/class.h diff --git a/hash.c b/hash.c index 4faa59686476f2..b863f217bcae99 100644 --- a/hash.c +++ b/hash.c @@ -1468,12 +1468,6 @@ rb_zjit_hash_new_size(void) { return hash_slot_size(sizeof(st_table) > sizeof(ar_table)); } - -size_t -rb_zjit_offset_rhash_ifnone(void) -{ - return offsetof(struct RHash, ifnone); -} #endif static VALUE diff --git a/jit.c b/jit.c index 12351ee1533aba..5def0f89b14ec7 100644 --- a/jit.c +++ b/jit.c @@ -16,6 +16,7 @@ #include "internal/gc.h" #include "vm_sync.h" #include "internal/fixnum.h" +#include "internal/hash.h" #include "internal/string.h" #include "internal/class.h" #include "internal/imemo.h" @@ -37,6 +38,9 @@ enum jit_bindgen_constants { // Field offset for fields_obj in T_DATA TDATA_OFFSET_FIELDS_OBJ = offsetof(struct RTypedData, fields_obj), + // Field offset for the RHash struct + RUBY_OFFSET_RHASH_IFNONE = offsetof(struct RHash, ifnone), + // Field offsets for the RString struct RUBY_OFFSET_RSTRING_LEN = offsetof(struct RString, len), diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 336d5cd559c795..29149bbb044836 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -988,6 +988,7 @@ pub const ROBJECT_OFFSET_AS_HEAP_FIELDS: jit_bindgen_constants = 16; pub const ROBJECT_OFFSET_AS_ARY: jit_bindgen_constants = 16; pub const RCLASS_OFFSET_PRIME_FIELDS_OBJ: jit_bindgen_constants = 40; pub const TDATA_OFFSET_FIELDS_OBJ: jit_bindgen_constants = 16; +pub const RUBY_OFFSET_RHASH_IFNONE: jit_bindgen_constants = 16; pub const RUBY_OFFSET_RSTRING_LEN: jit_bindgen_constants = 16; pub const RB_SHAPE_FLAG_SHIFT: jit_bindgen_constants = 32; pub const RUBY_OFFSET_EC_CFP: jit_bindgen_constants = 16; diff --git a/zjit.h b/zjit.h index 2a3a4e2b01c1a3..34ebdc095c6be9 100644 --- a/zjit.h +++ b/zjit.h @@ -78,7 +78,6 @@ void rb_zjit_invalidate_root_box(void); void rb_zjit_jit_frame_update_references(zjit_jit_frame_t *jit_frame); void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp); size_t rb_zjit_hash_new_size(void); -size_t rb_zjit_offset_rhash_ifnone(void); // Special value for cfp->jit_return that means "this is a C method frame, use // rb_zjit_c_frame as the JITFrame". We don't control the native stack layout diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 1feac5d07cc8aa..6a610e2c4235e4 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -2273,9 +2273,6 @@ fn gen_new_hash( let alloc_size = unsafe { rb_zjit_hash_new_size() }; let flags = RUBY_T_HASH as u64; let klass = unsafe { rb_cHash }; - let ifnone_offset: i32 = unsafe { rb_zjit_offset_rhash_ifnone() } - .try_into() - .expect("RHash ifnone offset should fit in i32"); let hash = gc_fastpath::gc_fast_path_new_obj(jit, asm, alloc_size, flags, klass, |asm| { asm_ccall!(asm, rb_hash_new,) @@ -2283,7 +2280,7 @@ fn gen_new_hash( // TODO: this runs on the slow path too, where rb_hash_new already set // ifnone. A fast-path-only init hook in gc_fast_path_new_obj would avoid // the redundant store and be reusable for other types. - asm.store(Opnd::mem(VALUE_BITS, hash, ifnone_offset), Qnil.into()); + asm.store(Opnd::mem(VALUE_BITS, hash, RUBY_OFFSET_RHASH_IFNONE), Qnil.into()); hash } else { gen_prepare_non_leaf_call(jit, asm, state); diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 5621f516e581ab..ad7d4556786799 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -124,8 +124,6 @@ unsafe extern "C" { pub fn rb_zjit_offset_ractor_newobj_cache() -> usize; - pub fn rb_zjit_offset_rhash_ifnone() -> usize; - // Floats within range will be encoded without creating objects in the heap. // (Range is 0x3000000000000001 to 0x4fffffffffffffff (1.7272337110188893E-77 to 2.3158417847463237E+77). pub fn rb_float_new(d: f64) -> VALUE; diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index de75402a0b3879..c7747b58010b56 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -1956,6 +1956,7 @@ pub const ROBJECT_OFFSET_AS_HEAP_FIELDS: jit_bindgen_constants = 16; pub const ROBJECT_OFFSET_AS_ARY: jit_bindgen_constants = 16; pub const RCLASS_OFFSET_PRIME_FIELDS_OBJ: jit_bindgen_constants = 40; pub const TDATA_OFFSET_FIELDS_OBJ: jit_bindgen_constants = 16; +pub const RUBY_OFFSET_RHASH_IFNONE: jit_bindgen_constants = 16; pub const RUBY_OFFSET_RSTRING_LEN: jit_bindgen_constants = 16; pub const RB_SHAPE_FLAG_SHIFT: jit_bindgen_constants = 32; pub const RUBY_OFFSET_EC_CFP: jit_bindgen_constants = 16; From 569ac0f998c34eb1fa7eb3aa9b770f3b355f3169 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Wed, 8 Jul 2026 19:26:21 +0100 Subject: [PATCH 11/13] Use VALUE::as_u64().into() for flags in the alloc fast path --- zjit/src/codegen/gc_fastpath.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zjit/src/codegen/gc_fastpath.rs b/zjit/src/codegen/gc_fastpath.rs index 6d63f2d2f2fbfa..105c33e7cbbb27 100644 --- a/zjit/src/codegen/gc_fastpath.rs +++ b/zjit/src/codegen/gc_fastpath.rs @@ -193,7 +193,7 @@ fn emit_default_new_obj_fastpath( asm.store(Opnd::mem(64, gc_cache, cursor_offset), new_cursor); asm.store( Opnd::mem(VALUE_BITS, cursor, RUBY_OFFSET_RBASIC_FLAGS), - Opnd::UImm(fastpath.flags.0 as u64), + fastpath.flags.as_u64().into(), ); asm.store( Opnd::mem(VALUE_BITS, cursor, RUBY_OFFSET_RBASIC_KLASS), @@ -309,7 +309,7 @@ fn emit_mmtk_new_obj_fastpath( asm.store(Opnd::mem(VALUE_BITS, aligned, 0), Opnd::UImm(payload_size)); asm.store( Opnd::mem(VALUE_BITS, obj, RUBY_OFFSET_RBASIC_FLAGS), - Opnd::UImm(fastpath.flags.0 as u64), + fastpath.flags.as_u64().into(), ); asm.store( Opnd::mem(VALUE_BITS, obj, RUBY_OFFSET_RBASIC_KLASS), From f1ce5fc24c6b37fde4fb41282d01f3d99d6ce4a1 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Wed, 8 Jul 2026 21:01:40 +0100 Subject: [PATCH 12/13] Move empty-hash GC-stress test to codegen_tests.rs --- test/ruby/test_zjit.rb | 21 --------------------- zjit/src/codegen_tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 57b120ff7f0bf6..18baf3832abb72 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -404,27 +404,6 @@ def allocate_array } end - def test_regression_gc_stress_new_hash_fast_path - assert_compiles ':ok', %q{ - def make = {} - - begin - GC.stress = true - 10.times { make } - h = make - raise "wrong class" unless h.instance_of?(Hash) - raise "wrong size" unless h.size == 0 - raise "wrong default" unless h.default.nil? - h[:a] = 1 - h[:b] = 2 - raise "not writable" unless h == { a: 1, b: 2 } - :ok - ensure - GC.stress = false - end - } - end - def test_exit_tracing # Smoke test: --zjit-trace-exits writes a Fuchsia trace (.fxt) file to /tmp assert_compiles('true', <<~RUBY, extra_args: ['--zjit-trace-exits']) diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 705691577601b4..efd158bfe3eeb4 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -2897,6 +2897,29 @@ fn test_new_hash_empty() { assert_snapshot!(assert_compiles("test"), @"{}"); } +// Exercises the empty-hash GC fast path under GC pressure. Guards against +// baking object flags as a GC-managed VALUE: T_HASH (8) has no immediate-mask +// bits set, so misclassifying it as a heap object records a bogus GC offset +// and crashes during marking. +#[test] +fn test_new_hash_empty_gc_stress() { + eval(" + def make = {} + "); + assert_contains_opcode("make", YARVINSN_newhash); + assert_snapshot!(assert_compiles(r#" + begin + GC.stress = true + make + h = make + h[:a] = 1 + [h.class, h.size, h.default, h] + ensure + GC.stress = false + end + "#), @"[Hash, 1, nil, {a: 1}]"); +} + #[test] fn test_new_hash_nonempty() { eval(r#" From 583668d5ed12331fd1b1361d6cad3f2378ddda2b Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 8 Jul 2026 19:54:49 +0900 Subject: [PATCH 13/13] Remove gc_bump_pointer_heap It was leftover and we don't actually need it. --- gc/default/default.c | 138 ++++++++++++-------------------- gc/default/zjit_fastpath.h | 2 +- gc/gc_impl.h | 8 -- zjit/bindgen/src/main.rs | 1 - zjit/src/codegen/gc_fastpath.rs | 6 +- zjit/src/cruby_bindings.inc.rs | 11 +-- 6 files changed, 54 insertions(+), 112 deletions(-) diff --git a/gc/default/default.c b/gc/default/default.c index 431d039e205439..7d8220c6b52af7 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -241,15 +241,17 @@ static RB_THREAD_LOCAL_SPECIFIER int malloc_increase_local; #endif typedef struct ractor_newobj_heap_cache { + uintptr_t cursor; + uintptr_t cursor_end; struct free_region *next_region; struct heap_page *using_page; uintptr_t region_end; + size_t allocated_objects_count; } rb_ractor_newobj_heap_cache_t; typedef struct ractor_newobj_cache { size_t incremental_mark_step_allocated_slots; rb_ractor_newobj_heap_cache_t heap_caches[HEAP_COUNT]; - struct gc_bump_pointer_heap bump_heaps[]; } rb_ractor_newobj_cache_t; typedef struct { @@ -1764,31 +1766,11 @@ calloc1(size_t n) return calloc(1, n); } -static void -gc_ractor_jit_cursor_sync(void *c, void *data) -{ - rb_ractor_newobj_cache_t *gc_cache = c; - bool tracing = (bool)(uintptr_t)data; - - for (size_t heap_idx = 0; heap_idx < HEAP_COUNT; heap_idx++) { - struct gc_bump_pointer_heap *bump = &gc_cache->bump_heaps[heap_idx]; - bump->jit_cursor_end = tracing ? bump->cursor : bump->cursor_end; - } -} - void rb_gc_impl_set_event_hook(void *objspace_ptr, const rb_event_flag_t event) { rb_objspace_t *objspace = objspace_ptr; - rb_event_flag_t old_events = objspace->hook_events; objspace->hook_events = event & RUBY_INTERNAL_EVENT_OBJSPACE_MASK; - - bool was_tracing = old_events & RUBY_INTERNAL_EVENT_NEWOBJ; - bool now_tracing = objspace->hook_events & RUBY_INTERNAL_EVENT_NEWOBJ; - if (was_tracing != now_tracing) { - rb_gc_ractor_newobj_cache_foreach(gc_ractor_jit_cursor_sync, - (void *)(uintptr_t)now_tracing); - } } unsigned long long @@ -2498,14 +2480,11 @@ rb_gc_impl_size_allocatable_p(size_t size) } static inline void -gc_bump_flush_alloc_count(struct gc_bump_pointer_heap *bump, rb_heap_t *heap) +gc_bump_flush_alloc_count(rb_ractor_newobj_heap_cache_t *heap_cache, rb_heap_t *heap) { - if (bump->slot_size == 0) return; - - size_t n = (bump->cursor - bump->region_start) / bump->slot_size; - if (n > 0) { - RUBY_ATOMIC_SIZE_ADD(heap->total_allocated_objects, n); - bump->region_start = bump->cursor; + if (heap_cache->allocated_objects_count > 0) { + RUBY_ATOMIC_SIZE_ADD(heap->total_allocated_objects, heap_cache->allocated_objects_count); + heap_cache->allocated_objects_count = 0; } } @@ -2513,31 +2492,29 @@ static void ractor_cache_flush_count(rb_objspace_t *objspace, rb_ractor_newobj_cache_t *gc_cache) { for (int heap_idx = 0; heap_idx < HEAP_COUNT; heap_idx++) { - gc_bump_flush_alloc_count(&gc_cache->bump_heaps[heap_idx], &heaps[heap_idx]); + gc_bump_flush_alloc_count(&gc_cache->heap_caches[heap_idx], &heaps[heap_idx]); } } static inline void -ractor_cache_open_window(rb_objspace_t *objspace, struct gc_bump_pointer_heap *bump, - rb_ractor_newobj_heap_cache_t *heap_cache) +ractor_cache_open_window(rb_objspace_t *objspace, rb_ractor_newobj_heap_cache_t *heap_cache, + size_t heap_idx) { uintptr_t end = heap_cache->region_end; if (RB_UNLIKELY(is_incremental_marking(objspace))) { - uintptr_t window_end = bump->cursor + INCREMENTAL_MARK_STEP_ALLOCATIONS * bump->slot_size; + uintptr_t window_end = heap_cache->cursor + INCREMENTAL_MARK_STEP_ALLOCATIONS * pool_slot_sizes[heap_idx]; if (window_end < end) end = window_end; } - bump->cursor_end = end; - bump->jit_cursor_end = - RB_UNLIKELY(objspace->hook_events & RUBY_INTERNAL_EVENT_NEWOBJ) ? bump->cursor : end; + heap_cache->cursor_end = end; } static inline bool -ractor_cache_advance_region(rb_objspace_t *objspace, struct gc_bump_pointer_heap *bump, - rb_ractor_newobj_heap_cache_t *heap_cache, rb_heap_t *heap) +ractor_cache_advance_region(rb_objspace_t *objspace, rb_ractor_newobj_heap_cache_t *heap_cache, + size_t heap_idx) { - gc_bump_flush_alloc_count(bump, heap); + gc_bump_flush_alloc_count(heap_cache, &heaps[heap_idx]); struct free_region *region = heap_cache->next_region; if (region == NULL) { @@ -2546,13 +2523,12 @@ ractor_cache_advance_region(rb_objspace_t *objspace, struct gc_bump_pointer_heap rb_asan_unpoison_object((VALUE)region, false); GC_ASSERT(RB_TYPE_P((VALUE)region, T_NONE)); - bump->cursor = (uintptr_t)region; - bump->region_start = (uintptr_t)region; + heap_cache->cursor = (uintptr_t)region; heap_cache->region_end = region->end; heap_cache->next_region = region->next; rb_asan_poison_object((VALUE)region); - ractor_cache_open_window(objspace, bump, heap_cache); + ractor_cache_open_window(objspace, heap_cache, heap_idx); return true; } @@ -2561,24 +2537,25 @@ static inline VALUE ractor_cache_allocate_slot(rb_objspace_t *objspace, rb_ractor_newobj_cache_t *gc_cache, size_t heap_idx) { - struct gc_bump_pointer_heap *bump = &gc_cache->bump_heaps[heap_idx]; + rb_ractor_newobj_heap_cache_t *heap_cache = &gc_cache->heap_caches[heap_idx]; + size_t slot_size = pool_slot_sizes[heap_idx]; - uintptr_t cursor = bump->cursor; - if (RB_UNLIKELY(cursor + bump->slot_size > bump->cursor_end)) { + uintptr_t cursor = heap_cache->cursor; + if (RB_UNLIKELY(cursor + slot_size > heap_cache->cursor_end)) { if (RB_UNLIKELY(is_incremental_marking(objspace))) { return Qfalse; } - rb_ractor_newobj_heap_cache_t *heap_cache = &gc_cache->heap_caches[heap_idx]; - if (!ractor_cache_advance_region(objspace, bump, heap_cache, &heaps[heap_idx])) { + if (!ractor_cache_advance_region(objspace, heap_cache, heap_idx)) { return Qfalse; } - cursor = bump->cursor; + cursor = heap_cache->cursor; } VALUE obj = (VALUE)cursor; rb_asan_unpoison_object(obj, true); - bump->cursor = cursor + bump->slot_size; + heap_cache->cursor = cursor + slot_size; + heap_cache->allocated_objects_count++; #if RGENGC_CHECK_MODE GC_ASSERT(rb_gc_impl_obj_slot_size(obj) == heap_slot_size(heap_idx)); @@ -2612,10 +2589,9 @@ ractor_cache_set_page(rb_objspace_t *objspace, rb_ractor_newobj_cache_t *gc_cach { gc_report(3, objspace, "ractor_set_cache: Using page %p\n", (void *)page->body); - struct gc_bump_pointer_heap *bump = &gc_cache->bump_heaps[heap_idx]; rb_ractor_newobj_heap_cache_t *heap_cache = &gc_cache->heap_caches[heap_idx]; - GC_ASSERT(bump->cursor + bump->slot_size > bump->cursor_end); + GC_ASSERT(heap_cache->cursor + pool_slot_sizes[heap_idx] > heap_cache->cursor_end); GC_ASSERT(heap_cache->next_region == NULL); GC_ASSERT(page->free_slots != 0); GC_ASSERT(page->free_region != NULL); @@ -2625,13 +2601,12 @@ ractor_cache_set_page(rb_objspace_t *objspace, rb_ractor_newobj_cache_t *gc_cach struct free_region *region = page->free_region; rb_asan_unpoison_object((VALUE)region, false); GC_ASSERT(RB_TYPE_P((VALUE)region, T_NONE)); - bump->cursor = (uintptr_t)region; - bump->region_start = (uintptr_t)region; + heap_cache->cursor = (uintptr_t)region; heap_cache->region_end = region->end; heap_cache->next_region = region->next; rb_asan_poison_object((VALUE)region); - ractor_cache_open_window(objspace, bump, heap_cache); + ractor_cache_open_window(objspace, heap_cache, heap_idx); page->free_slots = 0; page->free_region = NULL; @@ -2684,12 +2659,12 @@ rb_gc_impl_zjit_new_obj_fastpath(void *objspace_ptr, size_t alloc_size, VALUE fl } if (slot_size == 0) return false; - size_t bump_base = offsetof(rb_ractor_newobj_cache_t, bump_heaps) + - heap_idx * sizeof(struct gc_bump_pointer_heap); + size_t base = offsetof(rb_ractor_newobj_cache_t, heap_caches) + + heap_idx * sizeof(rb_ractor_newobj_heap_cache_t); struct rb_gc_zjit_default_new_obj_fastpath default_fastpath = { - bump_base + offsetof(struct gc_bump_pointer_heap, cursor), - bump_base + offsetof(struct gc_bump_pointer_heap, jit_cursor_end), + base + offsetof(rb_ractor_newobj_heap_cache_t, cursor), + base + offsetof(rb_ractor_newobj_heap_cache_t, cursor_end), slot_size, flags, klass @@ -2710,7 +2685,6 @@ NOINLINE(static VALUE newobj_bump_pointer_miss(rb_objspace_t *objspace, rb_racto static VALUE newobj_bump_pointer_miss(rb_objspace_t *objspace, rb_ractor_newobj_cache_t *gc_cache, size_t heap_idx, bool vm_locked) { - struct gc_bump_pointer_heap *bump = &gc_cache->bump_heaps[heap_idx]; rb_ractor_newobj_cache_t *cache = gc_cache; rb_ractor_newobj_heap_cache_t *heap_cache = &cache->heap_caches[heap_idx]; rb_heap_t *heap = &heaps[heap_idx]; @@ -2743,25 +2717,22 @@ newobj_bump_pointer_miss(rb_objspace_t *objspace, rb_ractor_newobj_cache_t *gc_c } if (is_incremental_marking(objspace)) { - if (bump->slot_size > 0) { - cache->incremental_mark_step_allocated_slots += - (bump->cursor - bump->region_start) / bump->slot_size; - } - gc_bump_flush_alloc_count(bump, heap); + cache->incremental_mark_step_allocated_slots += heap_cache->allocated_objects_count; + gc_bump_flush_alloc_count(heap_cache, heap); if (cache->incremental_mark_step_allocated_slots >= INCREMENTAL_MARK_STEP_ALLOCATIONS) { gc_continue(objspace, heap); cache->incremental_mark_step_allocated_slots = 0; } - if (bump->cursor + bump->slot_size <= heap_cache->region_end) { - ractor_cache_open_window(objspace, bump, heap_cache); + if (heap_cache->cursor + pool_slot_sizes[heap_idx] <= heap_cache->region_end) { + ractor_cache_open_window(objspace, heap_cache, heap_idx); obj = ractor_cache_allocate_slot(objspace, gc_cache, heap_idx); } } if (obj == Qfalse) { - if (ractor_cache_advance_region(objspace, bump, heap_cache, heap)) { + if (ractor_cache_advance_region(objspace, heap_cache, heap_idx)) { obj = ractor_cache_allocate_slot(objspace, gc_cache, heap_idx); } } @@ -2774,8 +2745,7 @@ newobj_bump_pointer_miss(rb_objspace_t *objspace, rb_ractor_newobj_cache_t *gc_c } if (RB_UNLIKELY(ruby_gc_stressful)) { - bump->cursor_end = bump->cursor; - bump->jit_cursor_end = bump->cursor; + heap_cache->cursor_end = heap_cache->cursor; } } @@ -4112,13 +4082,12 @@ gc_mode_transition(rb_objspace_t *objspace, enum gc_mode mode) } static void -heap_page_flush_cache_regions(struct heap_page *page, struct gc_bump_pointer_heap *bump, - rb_ractor_newobj_heap_cache_t *heap_cache) +heap_page_flush_cache_regions(struct heap_page *page, rb_ractor_newobj_heap_cache_t *heap_cache) { struct free_region *chain = heap_cache->next_region; - if (bump->cursor < heap_cache->region_end) { - VALUE start = (VALUE)bump->cursor; + if (heap_cache->cursor < heap_cache->region_end) { + VALUE start = (VALUE)heap_cache->cursor; rb_asan_unpoison_object(start, false); struct free_region *remnant = (struct free_region *)start; remnant->flags = 0; @@ -4186,26 +4155,23 @@ gc_ractor_newobj_cache_clear(void *c, void *data) newobj_cache->incremental_mark_step_allocated_slots = 0; for (size_t heap_idx = 0; heap_idx < HEAP_COUNT; heap_idx++) { - struct gc_bump_pointer_heap *bump = &gc_cache->bump_heaps[heap_idx]; rb_ractor_newobj_heap_cache_t *cache = &newobj_cache->heap_caches[heap_idx]; rb_heap_t *heap = &heaps[heap_idx]; - gc_bump_flush_alloc_count(bump, heap); + gc_bump_flush_alloc_count(cache, heap); struct heap_page *page = cache->using_page; - RUBY_DEBUG_LOG("ractor using_page:%p cursor:%p", (void *)page, (void *)bump->cursor); + RUBY_DEBUG_LOG("ractor using_page:%p cursor:%p", (void *)page, (void *)cache->cursor); if (page) { - heap_page_flush_cache_regions(page, bump, cache); + heap_page_flush_cache_regions(page, cache); } cache->using_page = NULL; cache->next_region = NULL; cache->region_end = 0; - bump->cursor = 0; - bump->cursor_end = 0; - bump->jit_cursor_end = 0; - bump->region_start = 0; + cache->cursor = 0; + cache->cursor_end = 0; } } @@ -4215,9 +4181,8 @@ gc_ractor_newobj_cache_exhaust(void *c, void *data) rb_ractor_newobj_cache_t *gc_cache = c; for (size_t heap_idx = 0; heap_idx < HEAP_COUNT; heap_idx++) { - struct gc_bump_pointer_heap *bump = &gc_cache->bump_heaps[heap_idx]; - bump->cursor_end = bump->cursor; - bump->jit_cursor_end = bump->cursor; + rb_ractor_newobj_heap_cache_t *heap_cache = &gc_cache->heap_caches[heap_idx]; + heap_cache->cursor_end = heap_cache->cursor; } } @@ -6826,13 +6791,8 @@ rb_gc_impl_ractor_cache_alloc(void *objspace_ptr, void *ractor) objspace->live_ractor_cache_count++; - rb_ractor_newobj_cache_t *gc_cache = - calloc1(sizeof(rb_ractor_newobj_cache_t) + - sizeof(struct gc_bump_pointer_heap) * (HEAP_COUNT + 1)); + rb_ractor_newobj_cache_t *gc_cache = calloc1(sizeof(rb_ractor_newobj_cache_t)); - for (size_t i = 0; i < HEAP_COUNT; i++) { - gc_cache->bump_heaps[i].slot_size = pool_slot_sizes[i]; - } return gc_cache; } diff --git a/gc/default/zjit_fastpath.h b/gc/default/zjit_fastpath.h index 08dd00928f50f3..3b72a89fc38a3f 100644 --- a/gc/default/zjit_fastpath.h +++ b/gc/default/zjit_fastpath.h @@ -9,7 +9,7 @@ struct rb_gc_zjit_default_new_obj_fastpath { size_t cursor_offset; - size_t jit_cursor_end_offset; + size_t cursor_end_offset; size_t slot_size; VALUE flags; VALUE klass; diff --git a/gc/gc_impl.h b/gc/gc_impl.h index 5aa8fee1503a79..95f7a37267658f 100644 --- a/gc/gc_impl.h +++ b/gc/gc_impl.h @@ -13,14 +13,6 @@ #include #include -struct gc_bump_pointer_heap { - uintptr_t cursor; - uintptr_t cursor_end; - uintptr_t jit_cursor_end; - uintptr_t region_start; - size_t slot_size; -}; - enum rb_gc_zjit_fastpath_kind { RB_GC_ZJIT_FASTPATH_DEFAULT = 1, RB_GC_ZJIT_FASTPATH_MMTK = 2, diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 9a7b4a15644d07..801663a81f1c8e 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -91,7 +91,6 @@ fn main() { .allowlist_type("RBasic") .allowlist_type("RArray") - .allowlist_type("gc_bump_pointer_heap") .allowlist_type("rb_gc_zjit_fastpath_kind") .allowlist_type("rb_gc_zjit_fastpath") .allowlist_type("rb_gc_zjit_fastpath_data") diff --git a/zjit/src/codegen/gc_fastpath.rs b/zjit/src/codegen/gc_fastpath.rs index 105c33e7cbbb27..b4ce600e279d5c 100644 --- a/zjit/src/codegen/gc_fastpath.rs +++ b/zjit/src/codegen/gc_fastpath.rs @@ -12,7 +12,7 @@ use super::JITState; #[derive(Clone, Copy)] struct RbGcZjitDefaultNewObjFastpath { cursor_offset: usize, - jit_cursor_end_offset: usize, + cursor_end_offset: usize, slot_size: usize, flags: VALUE, klass: VALUE, @@ -173,7 +173,7 @@ fn emit_default_new_obj_fastpath( miss: &Target, ) -> Option { let cursor_offset: i32 = fastpath.cursor_offset.try_into().ok()?; - let jit_cursor_end_offset: i32 = fastpath.jit_cursor_end_offset.try_into().ok()?; + let cursor_end_offset: i32 = fastpath.cursor_end_offset.try_into().ok()?; let slot_size: u64 = fastpath.slot_size.try_into().ok()?; let thread = asm.load(Opnd::mem(64, EC, RUBY_OFFSET_EC_THREAD_PTR as i32)); @@ -184,7 +184,7 @@ fn emit_default_new_obj_fastpath( let gc_cache = asm.load(Opnd::mem(64, ractor, ractor_newobj_cache_offset)); let cursor = asm.load(Opnd::mem(64, gc_cache, cursor_offset)); - let cursor_end = asm.load(Opnd::mem(64, gc_cache, jit_cursor_end_offset)); + let cursor_end = asm.load(Opnd::mem(64, gc_cache, cursor_end_offset)); let new_cursor = asm.add(cursor, Opnd::UImm(slot_size)); asm.cmp(cursor_end, new_cursor); diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index c7747b58010b56..a43038e665ce2b 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -1968,15 +1968,6 @@ pub type jit_bindgen_constants = i32; pub const rb_invalid_shape_id: shape_id_t = 524287; pub type rb_iseq_param_keyword_struct = rb_iseq_constant_body_rb_iseq_parameters_rb_iseq_param_keyword; -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct gc_bump_pointer_heap { - pub cursor: usize, - pub cursor_end: usize, - pub jit_cursor_end: usize, - pub region_start: usize, - pub slot_size: usize, -} pub const RB_GC_ZJIT_FASTPATH_DEFAULT: rb_gc_zjit_fastpath_kind = 1; pub const RB_GC_ZJIT_FASTPATH_MMTK: rb_gc_zjit_fastpath_kind = 2; pub type rb_gc_zjit_fastpath_kind = u32; @@ -1994,7 +1985,7 @@ pub struct rb_gc_zjit_fastpath { #[repr(C)] pub struct rb_gc_zjit_default_new_obj_fastpath { pub cursor_offset: usize, - pub jit_cursor_end_offset: usize, + pub cursor_end_offset: usize, pub slot_size: usize, pub flags: VALUE, pub klass: VALUE,