[pull] master from ruby:master - #1317
Merged
Merged
Conversation
(ruby/prism#4202) * This does not scale well with more find implementations. * RubyVM is CRuby-specific and experimental so not something usually exposed in public APIs. * Tests can use the specific class directly, which is more reliable. ruby/prism@411f741593
timer_thread_register_waiting kept the timed waiters on a single list sorted by deadline, inserting with a linear scan. A server whose connections share one timeout value -- every keep-alive loop -- makes each insert walk the whole list: with 8000 waiting connections the scan burns half the CPU (43.71% + 8.30% in thread_sched_wait_events) and M:N falls behind 1:1. Bucket timed waiters into a hierarchical timing wheel instead (Varghese & Lauck, "Hashed and Hierarchical Timing Wheels: Data Structures for the Efficient Implementation of a Timer Facility", SOSP '87): 4 levels of 64 slots, 1ms slots at the finest level and 64x coarser per level, ~4.6h of direct range; farther deadlines park on the coarsest level's farthest slot and re-sort on arrival. A not-yet-due waiter found while draining a slot is re-bucketed onto a finer level, so each waiter moves at most once per level. Deadlines are floored to a slot, so the wheel never fires late, and the re-bucketing keeps sleep precision. Insertion becomes O(1) arithmetic. Removal -- the most frequent operation, since most timed fd waits are cancelled by their fd becoming ready -- stays O(1): the cached next-expiry is a lower bound that only insertion has to maintain (waking early is harmless, so removal does no bookkeeping at all). The timer thread is now woken only when an insertion shortens that bound, instead of on every insertion. Untimed (fd-only) waiters move to their own plain list. The wheel and its operations are compiled only with USE_MN_THREADS; the slot width can be widened with -DTIMER_WHEEL_TICK_MS=n for experiments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TestException#test_detailed_message_under_gc_compact_stress calls
Exception#detailed_message inside EnvUtil.under_gc_compact_stress. The
first error display of a process lazily requires the decoration gems
(error_highlight, did_you_mean, syntax_suggest -- error.c's
require_decoration_gems), so the whole library load happened inside the
block, where GC.stress makes every allocation a full mark+compact.
The cost of each of those collections is proportional to the live heap,
so the load does not just take a while, it degrades with the age of the
worker. Measured here (x86_64, idle, gems enabled):
live slots block
14k 13.7s
114k 61.8s
414k 222.9s
A test-all worker that has already run other files sits far above that,
and the parallel runner kills a worker that has not responded for 1200s
(worker_timeout) with SIGSEGV -- which is what CI reported:
worker 63824=ruby/test_exception does not respond; SEGV is sent
1) Timeout:
TestException#test_detailed_message_under_gc_compact_stress
rb_crash_63824.txt:
lib/did_you_mean/spell_checker.rb: [BUG] Segmentation fault at 0x000003e90000f94a
test/ruby/test_exception.rb:1484:in 'detailed_message'
lib/did_you_mean.rb:4:in '<top (required)>'
lib/did_you_mean.rb:4:in 'require_relative'
(The reported "address" is not a fault address: for a signal sent by
another process siginfo's si_addr aliases si_pid/si_uid, so it reads
back as (uid << 32) | sender_pid -- 0x3e9 is uid 1001, the runner user.)
Whether this test took milliseconds or ran past the timeout depended on
whether an earlier file in the same worker had already loaded
did_you_mean, which made it look like a random hang. It only bites
where the decoration gems are enabled: `make test-all` passes
--disable-gems through RUN_OPTS, but the ZJIT/YJIT jobs override
RUN_OPTS, so gems are on there.
Do the load before the block; this test is about detailed_message, not
about require. With gems enabled, the block goes from 28.7s to 0.22s;
the --disable-gems path is unchanged (0.17s).
CI: https://github.com/ruby/ruby/actions/runs/31860169748/job/94952042861
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
2 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 : )