normal	2018-08-28 09:24:08 +0900 (Tue, 28 Aug 2018)

  New Revision: 64581

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=64581

  Log:
    thread_pthread.h: rename `gvl.acquired' to `gvl.owner' and document
    
    `acquired' was an old boolean variable, but nowadays it is a
    rb_thread_t pointer;  "gvl.owner" seems like a more appropriate
    name.  And document the contended path including waitq, timer,
    and timer_err.

  Modified files:
    trunk/thread_pthread.c
    trunk/thread_pthread.h
Index: thread_pthread.c
===================================================================
--- thread_pthread.c	(revision 64580)
+++ thread_pthread.c	(revision 64581)
@@ -186,7 +186,7 @@ designate_timer_thread(rb_vm_t *vm) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L186
 }
 
 /*
- * We become designated timer thread to kick vm->gvl.acquired
+ * We become designated timer thread to kick vm->gvl.owner
  * periodically.  Continue on old timeout if it expired.
  */
 static void
@@ -220,14 +220,14 @@ do_gvl_timer(rb_vm_t *vm, rb_thread_t *t https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L220
      * Timeslice.  Warning: the process may fork while this
      * thread is contending for GVL:
      */
-    if (vm->gvl.acquired) timer_thread_function();
+    if (vm->gvl.owner) timer_thread_function();
     vm->gvl.timer = 0;
 }
 
 static void
 gvl_acquire_common(rb_vm_t *vm, rb_thread_t *th)
 {
-    if (vm->gvl.acquired) {
+    if (vm->gvl.owner) {
         native_thread_data_t *nd = &th->native_thread_data;
 
         VM_ASSERT(th->unblock.func == 0 &&
@@ -242,7 +242,7 @@ gvl_acquire_common(rb_vm_t *vm, rb_threa https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L242
             else {
                 rb_native_cond_wait(&nd->cond.gvlq, &vm->gvl.lock);
             }
-        } while (vm->gvl.acquired);
+        } while (vm->gvl.owner);
 
         list_del_init(&nd->node.gvl);
 
@@ -254,7 +254,7 @@ gvl_acquire_common(rb_vm_t *vm, rb_threa https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L254
     else { /* reset timer if uncontended */
         vm->gvl.timer_err = ETIMEDOUT;
     }
-    vm->gvl.acquired = th;
+    vm->gvl.owner = th;
     if (!vm->gvl.timer) {
         if (!designate_timer_thread(vm) && !ubf_threads_empty()) {
             rb_thread_wakeup_timer_thread(-1);
@@ -274,7 +274,7 @@ static native_thread_data_t * https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L274
 gvl_release_common(rb_vm_t *vm)
 {
     native_thread_data_t *next;
-    vm->gvl.acquired = 0;
+    vm->gvl.owner = 0;
     next = list_top(&vm->gvl.waitq, native_thread_data_t, node.ubf);
     if (next) rb_native_cond_signal(&next->cond.gvlq);
 
@@ -333,7 +333,7 @@ gvl_init(rb_vm_t *vm) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L333
     rb_native_cond_initialize(&vm->gvl.switch_cond);
     rb_native_cond_initialize(&vm->gvl.switch_wait_cond);
     list_head_init(&vm->gvl.waitq);
-    vm->gvl.acquired = 0;
+    vm->gvl.owner = 0;
     vm->gvl.timer = 0;
     vm->gvl.timer_err = ETIMEDOUT;
     vm->gvl.need_yield = 0;
Index: thread_pthread.h
===================================================================
--- thread_pthread.h	(revision 64580)
+++ thread_pthread.h	(revision 64581)
@@ -47,11 +47,21 @@ typedef struct native_thread_data_struct https://github.com/ruby/ruby/blob/trunk/thread_pthread.h#L47
 
 typedef struct rb_global_vm_lock_struct {
     /* fast path */
-    const struct rb_thread_struct *acquired;
-    rb_nativethread_lock_t lock;
+    const struct rb_thread_struct *owner;
+    rb_nativethread_lock_t lock; /* AKA vm->gvl.lock */
 
-    /* slow path */
-    struct list_head waitq;
+    /*
+     * slow path, protected by vm->gvl.lock
+     * - @waitq - FIFO queue of threads waiting for GVL
+     * - @timer - it handles timeslices for @owner.  It is any one thread
+     *   in @waitq, there is no @timer if @waitq is empty, but always
+     *   a @timer if @waitq has entries
+     * - @timer_err tracks timeslice limit, the timeslice only resets
+     *   when pthread_cond_timedwait returns ETIMEDOUT, so frequent
+     *   switching between contended/uncontended GVL won't reset the
+     *   timer.
+     */
+    struct list_head waitq; /* <=> native_thread_data_t.node.ubf */
     const struct rb_thread_struct *timer;
     int timer_err;
 

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/