[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: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>