Skip to content

Commit ec337a8

Browse files
luke-gruberparacycle
authored andcommitted
Wake timer to create new SNT when needed for dedicated task (ruby#16009)
When removing a thread from `running_threads`, if we're on a shared native thread and we're running a dedicated task, we need to wake the timer thread so it can create a new SNT if necessary. We only do this if it's waiting forever without the 10ms quantum timeout for now, because max 10ms of wait is considered "good enough". In the future, perhaps we can force the timer thread to wake if this becomes an issue (`timer_thread_wakeup_force`). Fixes [Bug #21504]
1 parent c795ee4 commit ec337a8

3 files changed

Lines changed: 73 additions & 20 deletions

File tree

test/ruby/test_ractor.rb

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,45 @@ def test_max_cpu_1
201201
RUBY
202202
end
203203

204+
def test_timer_thread_create_snt_for_dedicated_task
205+
omit "timer thread works differently" if windows?
206+
omit "test relies on this as a best-effort safety mechanism" unless defined?(Process::WNOHANG)
207+
assert_separately([{ "RUBY_MAX_CPU" => "2" }], <<~'RUBY', timeout: 30)
208+
$VERBOSE = nil
209+
CHILD_PID = 0
210+
211+
rs = []
212+
2.times do |i|
213+
rs << Ractor.new(i) do |j|
214+
if j == 0
215+
pid = spawn("sleep 60", close_others: true)
216+
Object.const_set(:CHILD_PID, pid)
217+
Process.waitpid(pid) # block forever (dedicated task)
218+
else
219+
while CHILD_PID == 0
220+
sleep 1 # make sure first ractor blocks forever first (this is what we're testing)
221+
end
222+
1_000.times do
223+
[nil] * 100
224+
end
225+
end
226+
end
227+
end
228+
229+
rs.last.join
230+
begin
231+
result = Process.waitpid(CHILD_PID, Process::WNOHANG)
232+
rescue Errno::ECHILD, Errno::ESRCH
233+
# If it's somehow not a child (not running?), don't send it a signal
234+
else
235+
if result.nil?
236+
Process.kill('KILL', CHILD_PID) rescue nil
237+
end
238+
end
239+
rs.first.join # reap
240+
RUBY
241+
end
242+
204243
def test_symbol_proc_is_shareable
205244
pr = :symbol.to_proc
206245
assert_make_shareable(pr)

thread_pthread.c

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ static const void *const condattr_monotonic = NULL;
9090
#endif
9191
#endif
9292

93+
#ifndef MINIMUM_SNT
94+
// make at least MINIMUM_SNT snts for debug.
95+
#define MINIMUM_SNT 0
96+
#endif
97+
9398
#ifdef HAVE_SCHED_YIELD
9499
#define native_thread_yield() (void)sched_yield()
95100
#else
@@ -554,6 +559,20 @@ ractor_sched_timeslice_threads_contain_p(rb_vm_t *vm, rb_thread_t *th)
554559

555560
static void ractor_sched_barrier_join_signal_locked(rb_vm_t *vm);
556561

562+
static bool
563+
need_more_shared_native_threads_p(rb_vm_t *vm)
564+
{
565+
ASSERT_ractor_sched_locked(vm, NULL);
566+
567+
unsigned int schedulable_ractor_cnt = vm->ractor.cnt;
568+
unsigned int snt_cnt = vm->ractor.sched.snt_cnt;
569+
RUBY_ASSERT(schedulable_ractor_cnt >= 1);
570+
if (!vm->ractor.main_ractor->threads.sched.enable_mn_threads) {
571+
schedulable_ractor_cnt--; // do not need snt for main ractor
572+
}
573+
return snt_cnt < MINIMUM_SNT || (snt_cnt < schedulable_ractor_cnt && snt_cnt < vm->ractor.sched.max_cpu);
574+
}
575+
557576
// setup timeslice signals by the timer thread.
558577
static void
559578
thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *cr, rb_vm_t *vm,
@@ -564,6 +583,7 @@ thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *c
564583
#endif
565584

566585
rb_thread_t *del_timeslice_th;
586+
bool wakeup_timer_thread = false;
567587

568588
if (del_th && sched->is_running_timeslice) {
569589
del_timeslice_th = del_th;
@@ -592,6 +612,12 @@ thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *c
592612
ractor_sched_barrier_join_signal_locked(vm);
593613
}
594614
sched->is_running = false;
615+
616+
// If we need more SNTs, the timer thread should be awake and monitoring the situation so it can correct it.
617+
if (!del_th->has_dedicated_nt && del_th->nt->dedicated > 0 && (sched->running != NULL || vm->ractor.sched.grq_cnt > 0) &&
618+
need_more_shared_native_threads_p(vm)) {
619+
wakeup_timer_thread = true;
620+
}
595621
}
596622

597623
if (add_th) {
@@ -616,7 +642,7 @@ thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *c
616642
ccan_list_add(&vm->ractor.sched.timeslice_threads, &add_timeslice_th->sched.node.timeslice_threads);
617643
sched->is_running_timeslice = true;
618644
if (was_empty) {
619-
timer_thread_wakeup_locked(vm);
645+
wakeup_timer_thread = true;
620646
}
621647
}
622648

@@ -625,6 +651,10 @@ thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *c
625651
ccan_list_del_init(&del_timeslice_th->sched.node.timeslice_threads);
626652
}
627653

654+
if (wakeup_timer_thread) {
655+
timer_thread_wakeup_locked(vm);
656+
}
657+
628658
VM_ASSERT(ractor_sched_running_threads_size(vm) == vm->ractor.sched.running_cnt);
629659
VM_ASSERT(ractor_sched_timeslice_threads_size(vm) <= vm->ractor.sched.running_cnt);
630660
}
@@ -1264,11 +1294,6 @@ ractor_sched_enq(rb_vm_t *vm, rb_ractor_t *r)
12641294
#define SNT_KEEP_SECONDS 0
12651295
#endif
12661296

1267-
#ifndef MINIMUM_SNT
1268-
// make at least MINIMUM_SNT snts for debug.
1269-
#define MINIMUM_SNT 0
1270-
#endif
1271-
12721297
static rb_ractor_t *
12731298
ractor_sched_deq(rb_vm_t *vm, rb_ractor_t *cr)
12741299
{

thread_pthread_mn.c

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -394,25 +394,14 @@ nt_free_stack(void *mstack)
394394
rb_native_mutex_unlock(&nt_machine_stack_lock);
395395
}
396396

397-
398397
static int
399398
native_thread_check_and_create_shared(rb_vm_t *vm)
400399
{
401400
bool need_to_make = false;
402401

403-
rb_native_mutex_lock(&vm->ractor.sched.lock);
402+
ractor_sched_lock(vm, NULL);
404403
{
405-
unsigned int schedulable_ractor_cnt = vm->ractor.cnt;
406-
RUBY_ASSERT(schedulable_ractor_cnt >= 1);
407-
408-
if (!vm->ractor.main_ractor->threads.sched.enable_mn_threads)
409-
schedulable_ractor_cnt--; // do not need snt for main ractor
410-
411-
unsigned int snt_cnt = vm->ractor.sched.snt_cnt;
412-
if (((int)snt_cnt < MINIMUM_SNT) ||
413-
(snt_cnt < schedulable_ractor_cnt &&
414-
snt_cnt < vm->ractor.sched.max_cpu)) {
415-
404+
if (need_more_shared_native_threads_p(vm)) {
416405
RUBY_DEBUG_LOG("added snt:%u dnt:%u ractor_cnt:%u grq_cnt:%u",
417406
vm->ractor.sched.snt_cnt,
418407
vm->ractor.sched.dnt_cnt,
@@ -426,7 +415,7 @@ native_thread_check_and_create_shared(rb_vm_t *vm)
426415
RUBY_DEBUG_LOG("snt:%d ractor_cnt:%d", (int)vm->ractor.sched.snt_cnt, (int)vm->ractor.cnt);
427416
}
428417
}
429-
rb_native_mutex_unlock(&vm->ractor.sched.lock);
418+
ractor_sched_unlock(vm, NULL);
430419

431420
if (need_to_make) {
432421
struct rb_native_thread *nt = native_thread_alloc();

0 commit comments

Comments
 (0)