From: ko1@... Date: 2018-08-14T08:09:38+00:00 Subject: [ruby-core:88475] [Ruby trunk Misc#14937] [PATCH] thread_pthread: lazy-spawn timer-thread only on contention Issue #14937 has been updated by ko1 (Koichi Sasada). Let me clear your commits. Maybe "Description" of this ticket is obsolete. They are my understandings (fix me if they are wrong): * Your idea is using POSIX timer if possible. ``` #define UBF_TIMER_NONE 0 #define UBF_TIMER_POSIX 1 #define UBF_TIMER_PTHREAD 2 ``` `UBF_TIMER_POSIX` uses POSIX timer. `UBF_TIMER_PTHREAD` is traditional timer thread approach. BTW ``` * UBF_TIMER is to close TOCTTOU signal race on programs * without GVL contention blocking read/write to sockets. ``` I can't understand these lines. What is related between GVL contention and read/write sockets? Single thread application do not kick `gvl_acquire_common` and `timer_thread_function()` is not kicked, right? * GVL acquire loop invoke `timer_thread_function()` ... sorry I'm giving up to understand them correctly. Could you explain more for current trunk? After your commit, there are no timer threads? ---- # Comments/Questions ## `gvl_acquire_common` > ` VM_ASSERT(th->unblock.func == 0 && "we reuse ubf_list for GVL waitq");` I strongly disagree that reusing. It is very confusing. > `rb_timer_(|dis)arm()` English question (sorry). What does it mean with word "arm"? > `native_cond_timeout()` This function is not good name because I misunderstand it is same as `native_cond_timedwait()`. Maybe I had named it :p > ` list_add_tail(&vm->gvl.waitq, &nd->ubf_list);` Why add it? What`vm->gvl.waitq` manages? To specify `designate_timer_thread()` There are two condvars `gvlq` and `switch_cond`. Is it intentional? Who remove it from the list? `list_top()` removes? (impl doesn't seem such deletion). > `ubf_wakeup_all_threads();` why wakeup all blocking threads? ## `rb_timer_create()` There is a guard `#if UBF_TIMER == UBF_TIMER_POSIX` but not for `rb_timer_pthread_create()`. I misunderstand that `UBF_TIMER_POSIX` needs timer pthread. ## `rb_timer_` prefix making `timer_` will help to understand they are local static functions. If you want to expose them outside thread_pthread.c, they should be more verbose name like `rb_vm_timer` and so on to recognize they are no `Timer` class in Ruby world (it is my opinion). ---------------------------------------- Misc #14937: [PATCH] thread_pthread: lazy-spawn timer-thread only on contention https://bugs.ruby-lang.org/issues/14937#change-73540 * Author: normalperson (Eric Wong) * Status: Closed * Priority: Normal * Assignee: ---------------------------------------- [ruby-core:87773] ``` thread_pthread: lazy-spawn timer-thread only on contention To reduce resource use and reduce CI failure; lazy spawn timer-thread only in processes which use Ruby Threads AND those Ruby Threads hit contention. Single-threaded Ruby processes (including forked children) will never have timer-thread overhead. To simplify the thread_pthread.c code, I eliminated busy timer thread [Misc #14851]. Maybe the thread_win32.c code can use self-pipe, too; and they won't need busy wakeups. There is only one self-pipe, now, as wakeups for timeslice are handled via condition variables. This reduces FD pressure slightly. Signal handling is handled directly by one Ruby Thread (instead of timer-thread) by exposing signal self-pipe to callers of rb_thread_fd_select, native_sleep, rb_wait_for_single_fd, etc... Acquiring, using, and releasing the self-pipe is exposed via 4 new internal functions: 1) rb_sigwait_fd_get - exclusively acquire timer_thread_pipe.normal[0] 2) rb_sigwait_fd_sleep - sleep and wait for signal (and no other FDs) 3) rb_sigwait_fd_put - release acquired result from rb_sigwait_fd_get 4) rb_sigwait_fd_migrate - migrate signal handling to another thread after calling rb_sigwait_fd_put. rb_sigwait_fd_migrate is necessary for waitpid callers because only one thread can wait on self-pipe at a time, otherwise a deadlock will occur if threads fight over the self-pipe. TRAP_INTERRUPT_MASK is now set for the main thread directly in signal handler via rb_thread_wakeup_timer_thread. Originally, I wanted to use POSIX timers (timer_create/timer_settime) and avoid timer-thread completely. Unfortunately, this proved unfeasible for one reason: Mutex#sleep resumes on spurious wakeups and test/thread/test_cv.rb::test_condvar_timed_wait failed. In the future, I hope [Feature #14717] is accepted so Threads may be made non-preemptible. This will allow users to prevent timer-thread creation completely. ``` git repository also available at: https://80x24.org/ruby.git tt-lazy (commit a2990cefccba55300ad44275ee4adf18e6f95ece) ---Files-------------------------------- 0001-thread_pthread-lazy-spawn-timer-thread-only-on-conte.patch (42.3 KB) -- https://bugs.ruby-lang.org/ Unsubscribe: