Skip to content

Commit fd089e3

Browse files
committed
Revert "Wake timer to create new SNT when needed for dedicated task (ruby#16009)"
This reverts commit 7b3370a.
1 parent d5af159 commit fd089e3

3 files changed

Lines changed: 20 additions & 73 deletions

File tree

test/ruby/test_ractor.rb

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -201,45 +201,6 @@ 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-
243204
def test_symbol_proc_is_shareable
244205
pr = :symbol.to_proc
245206
assert_make_shareable(pr)

thread_pthread.c

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,6 @@ 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-
9893
#ifdef HAVE_SCHED_YIELD
9994
#define native_thread_yield() (void)sched_yield()
10095
#else
@@ -559,20 +554,6 @@ ractor_sched_timeslice_threads_contain_p(rb_vm_t *vm, rb_thread_t *th)
559554

560555
static void ractor_sched_barrier_join_signal_locked(rb_vm_t *vm);
561556

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-
576557
// setup timeslice signals by the timer thread.
577558
static void
578559
thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *cr, rb_vm_t *vm,
@@ -583,7 +564,6 @@ thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *c
583564
#endif
584565

585566
rb_thread_t *del_timeslice_th;
586-
bool wakeup_timer_thread = false;
587567

588568
if (del_th && sched->is_running_timeslice) {
589569
del_timeslice_th = del_th;
@@ -612,12 +592,6 @@ thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *c
612592
ractor_sched_barrier_join_signal_locked(vm);
613593
}
614594
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-
}
621595
}
622596

623597
if (add_th) {
@@ -642,7 +616,7 @@ thread_sched_setup_running_threads(struct rb_thread_sched *sched, rb_ractor_t *c
642616
ccan_list_add(&vm->ractor.sched.timeslice_threads, &add_timeslice_th->sched.node.timeslice_threads);
643617
sched->is_running_timeslice = true;
644618
if (was_empty) {
645-
wakeup_timer_thread = true;
619+
timer_thread_wakeup_locked(vm);
646620
}
647621
}
648622

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

654-
if (wakeup_timer_thread) {
655-
timer_thread_wakeup_locked(vm);
656-
}
657-
658628
VM_ASSERT(ractor_sched_running_threads_size(vm) == vm->ractor.sched.running_cnt);
659629
VM_ASSERT(ractor_sched_timeslice_threads_size(vm) <= vm->ractor.sched.running_cnt);
660630
}
@@ -1294,6 +1264,11 @@ ractor_sched_enq(rb_vm_t *vm, rb_ractor_t *r)
12941264
#define SNT_KEEP_SECONDS 0
12951265
#endif
12961266

1267+
#ifndef MINIMUM_SNT
1268+
// make at least MINIMUM_SNT snts for debug.
1269+
#define MINIMUM_SNT 0
1270+
#endif
1271+
12971272
static rb_ractor_t *
12981273
ractor_sched_deq(rb_vm_t *vm, rb_ractor_t *cr)
12991274
{

thread_pthread_mn.c

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

397+
397398
static int
398399
native_thread_check_and_create_shared(rb_vm_t *vm)
399400
{
400401
bool need_to_make = false;
401402

402-
ractor_sched_lock(vm, NULL);
403+
rb_native_mutex_lock(&vm->ractor.sched.lock);
403404
{
404-
if (need_more_shared_native_threads_p(vm)) {
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+
405416
RUBY_DEBUG_LOG("added snt:%u dnt:%u ractor_cnt:%u grq_cnt:%u",
406417
vm->ractor.sched.snt_cnt,
407418
vm->ractor.sched.dnt_cnt,
@@ -415,7 +426,7 @@ native_thread_check_and_create_shared(rb_vm_t *vm)
415426
RUBY_DEBUG_LOG("snt:%d ractor_cnt:%d", (int)vm->ractor.sched.snt_cnt, (int)vm->ractor.cnt);
416427
}
417428
}
418-
ractor_sched_unlock(vm, NULL);
429+
rb_native_mutex_unlock(&vm->ractor.sched.lock);
419430

420431
if (need_to_make) {
421432
struct rb_native_thread *nt = native_thread_alloc();

0 commit comments

Comments
 (0)