ruby-changes:52181
From: normal <ko1@a...>
Date: Thu, 16 Aug 2018 17:27:01 +0900 (JST)
Subject: [ruby-changes:52181] normal:r64389 (trunk): thread_pthread.c: reduce ubf_timer arming for non-signal wakeups
normal 2018-08-16 17:26:56 +0900 (Thu, 16 Aug 2018) New Revision: 64389 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=64389 Log: thread_pthread.c: reduce ubf_timer arming for non-signal wakeups We do not need to rely on SIGVTALRM for non-sighandler wakeups. This will reduce spurious wakeups in cases where sigwait_fd is not grabbed again, soon. Modified files: trunk/process.c trunk/thread_pthread.c Index: thread_pthread.c =================================================================== --- thread_pthread.c (revision 64388) +++ thread_pthread.c (revision 64389) @@ -247,7 +247,7 @@ gvl_acquire_common(rb_vm_t *vm, rb_threa https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L247 vm->gvl.acquired = th; if (!vm->gvl.timer) { if (!designate_timer_thread(vm) && !ubf_threads_empty()) { - rb_thread_wakeup_timer_thread(0); + rb_thread_wakeup_timer_thread(-1); } } } @@ -1324,10 +1324,10 @@ ubf_select(void *ptr) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L1324 * exit from ubf function. We must designate a timer-thread to perform * this operation. */ - rb_native_mutex_lock(&vm->gvl.lock); - if (!vm->gvl.timer) { + rb_native_mutex_lock(&vm->gvl.lock); + if (!vm->gvl.timer) { if (!designate_timer_thread(vm)) { - rb_thread_wakeup_timer_thread(0); + rb_thread_wakeup_timer_thread(-1); } } rb_native_mutex_unlock(&vm->gvl.lock); @@ -1416,7 +1416,8 @@ static void https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L1416 ubf_timer_arm(rb_pid_t current) /* async signal safe */ { #if UBF_TIMER == UBF_TIMER_POSIX - if (timer_posix.owner == current && !ATOMIC_CAS(timer_posix.armed, 0, 1)) { + if ((!current || timer_posix.owner == current) && + !ATOMIC_CAS(timer_posix.armed, 0, 1)) { struct itimerspec it; it.it_interval.tv_sec = it.it_value.tv_sec = 0; @@ -1446,7 +1447,7 @@ ubf_timer_arm(rb_pid_t current) /* async https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L1447 } } #elif UBF_TIMER == UBF_TIMER_PTHREAD - if (current == timer_pthread.owner) { + if (!current || current == timer_pthread.owner) { if (ATOMIC_EXCHANGE(timer_pthread.armed, 1) == 0) rb_thread_wakeup_timer_thread_fd(timer_pthread.low[1]); } @@ -1456,8 +1457,19 @@ ubf_timer_arm(rb_pid_t current) /* async https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L1457 void rb_thread_wakeup_timer_thread(int sig) { + rb_pid_t current; + + /* non-sighandler path */ + if (sig <= 0) { + rb_thread_wakeup_timer_thread_fd(signal_self_pipe.normal[1]); + if (sig < 0) { + ubf_timer_arm(0); + } + return; + } + /* must be safe inside sighandler, so no mutex */ - rb_pid_t current = getpid(); + current = getpid(); if (signal_self_pipe.owner_process == current) { rb_thread_wakeup_timer_thread_fd(signal_self_pipe.normal[1]); @@ -1465,7 +1477,7 @@ rb_thread_wakeup_timer_thread(int sig) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L1477 * system_working check is required because vm and main_thread are * freed during shutdown */ - if (sig && system_working > 0) { + if (system_working > 0) { volatile rb_execution_context_t *ec; rb_vm_t *vm = GET_VM(); rb_thread_t *mth; @@ -1485,9 +1497,6 @@ rb_thread_wakeup_timer_thread(int sig) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L1497 RUBY_VM_SET_TRAP_INTERRUPT(ec); ubf_timer_arm(current); } - } - else if (sig == 0 && system_working > 0) { - ubf_timer_arm(current); } } } Index: process.c =================================================================== --- process.c (revision 64388) +++ process.c (revision 64389) @@ -1162,6 +1162,10 @@ waitpid_nogvl(void *x) https://github.com/ruby/ruby/blob/trunk/process.c#L1162 /* another thread calling rb_sigwait_sleep will process * signals for us */ if (SIGCHLD_LOSSY) { + /* + * XXX this may not be needed anymore with new GVL + * and sigwait_fd usage + */ rb_thread_wakeup_timer_thread(0); } rb_native_cond_wait(w->cond, &th->interrupt_lock); -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/