[pull] master from ruby:master - #1311
Merged
Merged
Conversation
This patch allows the OS to free page and arena memory. This only happens in single objspace mode at the end of a major GC. There is still an empty_pages list per objspace, and this stays resident as before. Only pages previously munmapped pre-rlgc are madvised(DONTNEED)'d. We call these discarded pages. Once an arena (2MB) has all of its 32 pages (each 64K) discarded, we munmap the arena. At least 1 arena is kept mapped at all times. The following fields are added to GC.stat for the default GC: page_pool_arenas // number of mapped arenas page_pool_arenas_freed // number of freed (unmapped) arenas page_pool_total_pages // total pages in all arenas (arena_count * 32 currently) page_pool_discarded_pages // number of madvise(DONTNEED) pages (they can still be mapped) Future work: * Reduce the amount of empty pages per objspace or get rid of objspace->empty_pages altogether and keep a certain amount of resident pages in the pool for all objspaces. * Allow reclaiming memory when there are multiple Ractors.
ractor_sched_enq only signalled ractor.sched.cond. The signal reaches a parked shared native thread, and a running one revisits the queue in ractor_sched_deq (under the same lock) before it can wait, so as long as any shared native thread exists the entry is served. With none -- each one dedicated to a blocking region or retired -- the signal is lost, and if the timer thread is also in its untimed sleep (timeslice_wait_inf), nothing serves the enqueued ractor: its runnable thread never runs. native_thread_check_and_create_shared, which would repair the shortage, runs on the timer thread's timeout branch, which is never reached again. Wake the timer thread under the same lock its timeout decision is made under, from the two transitions that can strand an entry: an enqueue that finds snt_cnt == 0, and the blocking-region entry that dedicates the last shared native thread away while the queue is non-empty. Both are off the ordinary path, and timer_thread_wakeup_locked writes to the communication pipe only when the timer thread sleeps untimed, so the common case costs nothing. [Bug #21504] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
native_thread_create_shared called thread_sched_to_ready before trying to widen the shared pool, so when pthread_create failed the creator's failure path (marking the thread killed, removing it from the living set, rb_ractor_cancel_creation) ran against a thread that an already parked shared native thread could pick up and run to its end concurrently -- including rb_ractor_postmortem_free of the very rb_thread_t the creator is still writing to. Observed under RLIMIT_NPROC exhaustion as ractor.c: rb_ractor_cancel_creation: r->status_ == ractor_blocking ractor.c: ractor_check_blocking: cr->threads.cnt >= cr->threads.blocking_cnt + 1 i.e. the living-set removal ran twice. Widen the pool first and make the thread ready only after that succeeded; the failure path then handles a thread that never became runnable, which is what it assumes. A never-started coroutine's stack and context are already freed by rb_threadptr_sched_free. Also roll snt_cnt back when native_thread_create0 fails, and free the allocated rb_native_thread. The count stayed raised before, so native_thread_check_and_create_shared concluded forever that the pool was wide enough and never tried again: permanent starvation instead of a retry on the next timer tick. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
waitfd_to_waiting_flag maps RB_WAITFD_PRI to no thread_sched_waiting_* event, so the M:N scheduler registered nothing for it yet parked the thread: a timeout-less IO#wait(IO::PRIORITY) on an M:N thread slept forever (a VM_ASSERT in thread_io_wait_events on debug builds). Exclude it from thread_io_mn_schedulable; poll(2) on the blocking path handles POLLPRI. The test lives in test/ruby/test_ractor.rb rather than bootstraptest because it needs the socket extension, which btest runs without. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The M:N wait set th->status to THREAD_STOPPED_FOREVER around the park and to THREAD_RUNNABLE after it, unconditionally. A dedicated thread's native_cond_sleep leaves status alone: sleep_hrtime sets THREAD_STOPPED, re-sleeps while it stays so (SLEEP_SPURIOUS_CHECK), and only a waker sets THREAD_RUNNABLE. Rewriting it to THREAD_RUNNABLE made every spurious wakeup end the sleep: an interrupt deferred by Thread.handle_interrupt cut sleep(n) short on an M:N thread. Touch status only when the thread entered as THREAD_RUNNABLE (an io wait), which shows "sleep" while parked as a dedicated thread's blocking region does; a sleeper's status now belongs to its caller on both kinds of native thread. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rb_nogvl resolved RUBY_UBF_IO / RUBY_UBF_PROCESS to ubf_select before looking at RB_NOGVL_UBF_ASYNC_SAFE, so a caller combining the two had ubf_select invoked from the signal handler path (rb_thread_wakeup_timer_thread), where its ubf_list_lock and ractor scheduler lock can self-deadlock. No in-tree caller does this, but the API accepted it. Honour the flag only for a caller-provided ubf; the sentinels fall back to the ordinary deferred invocation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In the forked child: - Do not pthread_mutex_destroy timer_th.waiting_lock. glibc returns EBUSY (which rb_native_mutex_destroy turns into rb_bug_errno) for a mutex that another ractor's M:N thread held at the fork moment. Reinitializing it, as the code already does, is the whole story. - Reinitialize nt_machine_stack_lock, whose holder at the fork moment does not exist in the child; the next machine-stack alloc or free would deadlock on it. - Forget timer_th.event_fd on the kqueue backend instead of leaving the stale number around. A kqueue is not inherited across fork, so it must not be closed either: the number may name a reused fd by then. - Reset dnt_cnt in thread_sched_atfork next to snt_cnt: only the forking thread's native thread survives. Its consumer is only a debug log, but the log lied after fork. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A retiring shared thread -- ractor_sched_deq giving itself back, or a thread rejoining a full pool (nt->retiring) -- just returned from nt_start, leaking its rb_native_thread: the struct, the coroutine context, the condvars and the sigaltstack. This was the old "how to free nt and nt->altstack?" TODO, and retiring made it recurring: every pool shrink leaked one. Both exits drop the nt from the counts before leaving the loop, so nothing can reference it; the exiting thread frees it itself, disarming its sigaltstack registration first so a late signal cannot run on the freed block. Verified with SNT_IDLE_RETIRE=0 (retire on the first idle wakeup): 168 retire/free pairs in a bursty ractor workload and a clean btest run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
None of these is reachable with default settings; each is a mine for a configuration or platform that steps on it. - timer_thread_set_timeout: a sleeper's deadline further away than INT_MAX ms truncated to a negative timeout, i.e. an untimed epoll_wait, and the sleeper was never woken. Clamp to INT_MAX. - nt_thread_stack_size: the size sum went through int, so stack sizes totalling over 2GB went negative (stack_count 0, and nt_alloc_stack retries mapping 512MB chunks forever). Compute in size_t. Also page-align the VM stack and machine stack areas: the configured sizes align only to 4KB, and on 16KB/64KB-page systems the guard page and the MAP_FIXED machine stack must land on page boundaries. - nt_alloc_stack: an mprotect/mmap failure had already consumed the slot (uninitialized_stack_count--); restore it. - nt_free_stack: prev_free_chunk == NULL cannot double as the "not on the free list" test -- the list's tail also has it NULL, and re-pushing the tail self-cycled the list. Track membership in its own flag. - ruby_stack_overflowed_p: the __APPLE__ branch runs when th == NULL yet read th->nt->thread_id; ask pthread_self() instead. - rb_thread_create_timer_thread: rb_bug on pthread_create failure instead of silently running without a timer thread. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- verify_waiting_list compared w->data.timeout with itself, so the sorted-order assert was vacuously true and the waiting list's O(n) insert has been unverified even on debug builds. - timer_thread_register_waiting repeated the rel && *rel > 0 check that the branch right above it had just done. - Remove THREAD_BLOCKING_YIELD: no user, and it calls thread_sched_to_waiting with a signature that predates the yield_immediately parameter, so any new user fails to compile. - native_thread_check_and_create_shared took ractor.sched.lock through the raw mutex API, bypassing the lock-owner bookkeeping every other taker goes through (ractor_sched_lock). - The stack chunk layout comment claimed "0th page is Redzone"; the header has occupied the head pages since 407dd02. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thread_io_wait() sent every fd wait with a timeout to the blocking region (ppoll on a dedicated native thread), even under M:N. A pattern like IO#wait_readable(0.5) -- WEBrick's keepalive loop, or any flag-polling wait -- therefore cost a native thread handoff per call: ~0.7ms to reacquire the running slot after each ppoll under load, serializing servers to a fraction of the 1:1 throughput. Register such waits with the timer thread instead, as fd+timeout combined waiting. That registration path had no caller until now, and its expiry path timer_thread_deq_wakeup() left the waiter on its fd's waiter list with the fd still armed; unregister it there the same way the cancel path does. A zero timeout keeps using ppoll: it is a plain probe and needs no parking. WEBrick c=256: MN 2.3k -> 6.0k rps (hello, now above 1:1's 5.7k), 0.37k -> 1.5k rps (50 memcached accesses per request). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…is enabled It still needs to take the VM lock occasionally to protect certain global data structures during marking but it doesn't have to take the lock the entire time. The problem with a long-held VM lock is that it increases the chance that other Ractors will block on it (and increases the amount of time they will be blocked). Note: It still takes the VM lock around the sweep of every page but I'll work on that in a separate PR.
```
/github/workspace/src/ext/digest/blake3/blake3_dispatch.c:108:5: error: '_Atomic' is a C11 extension [-Werror,-Wc11-extensions]
108 | ATOMIC_INT g_cpu_features = UNDEFINED;
| ^
/github/workspace/src/ext/digest/blake3/blake3_dispatch.c:34:20: note: expanded from macro 'ATOMIC_INT'
34 | #define ATOMIC_INT _Atomic int
| ^
1 error generated.
```
ruby/digest@505c2d159f
The distributed files do not include `extconf.h` generated by `create_header`. ruby/digest@d3bcdccba3
Mainly, to dodge warnings on i686:
../src/zjit.h: In function ‘ZJIT_STACK_MAP_BASE_PTR_STACK_SIZE’:
../src/zjit.h:83:18: warning: right shift count >= width of type [-Wshift-count-overflow]
83 | return entry >> ZJIT_STACK_MAP_BASE_PTR_SIZE_SHIFT;
| ^~
Since frozen ar_table can occupy smaller slots, shrinking the table on freeze potentially allow the Hash to be demoted to an even smaller slot.
page_pool_reclaim() advises (madvise) cold page bodies only when the OS page size is smaller than HEAP_PAGE_SIZE, because the first OS page of a body holds the in-body freelist link and the arena tag and must be spared. On systems whose page size is >= HEAP_PAGE_SIZE (64KiB pages on aarch64, e.g. the oci-aarch64 CI machine) Step A therefore never increments page_pool.advised_count -- but Step B still subtracted PAGE_POOL_ARENA_BODIES (32) from it for every munmap'd arena, so the counter went negative. advised_count is an int reported through GC.stat as page_pool_discarded_pages with SIZET2NUM(), so a negative value is converted to a huge unsigned value and GC.stat has to allocate a Bignum for it. That allocation happens after GC.stat has already read the slot counters, so any pair of "read stats, then read them again another way" observes one extra live object, which is exactly what these two tests do (both fail by exactly 1 in the same direction). Skip the Step B adjustment when the platform cannot advise at all. When it can, every body of an unmapped arena is on its cold freelist and was just advised by Step A, so subtracting 32 stays exact. Verified by simulating a 64KiB page size (forcing page_pool.os_page_size = HEAP_PAGE_SIZE) on x86_64: before the fix GC.stat[:page_pool_discarded_pages] reads 18446744073709551456 after arenas are unmapped and the test_stat comparison is off by 1; after the fix it reads 0 and test/ruby/test_gc.rb passes both with and without the simulation. Error: 1) Failure: TestGc#test_stat_heap_constraints [test/ruby/test_gc.rb:298]: <357949> expected but was <357950>. 2) Failure: TestGc#test_stat [test/ruby/test_gc.rb:178]: <356712> expected but was <356711>. (5 consecutive master@oci-aarch64 runs since 9ebb977) CI: https://ci.rvm.jp/results/master@oci-aarch64/6449795 Log: https://ci.rvm.jp/logfiles/brlog.master.20260814-161514 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same as test_settracefunc.rb. This should fix flaky CI failures like https://github.com/ruby/ruby/actions/runs/31826726042/job/94852494689
This should avoid flaky failures such as <https://github.com/ruby/ruby/actions/runs/31831706566/job/94868685348>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )