[ruby-core:32686] [Ruby 1.9-Feature#3436] Spawn the timer thread lazily

From: Mark Somerville <redmine@...>
Date: 2010-10-04 18:09:42 UTC
List: ruby-core #32686
Issue #3436 has been updated by Mark Somerville.

File bug_3436-spawn_the_timer_thread_lazily.patch added

I've attached a patch to fix this.

The thread is now only used when it is required to schedule Ruby threads. When there is only the main thread, signals are handled immediately in sighandler().

I only have access to Linux boxes. One of the added tests isn't used on other platforms.


----------------------------------------
http://redmine.ruby-lang.org/issues/show/3436

----------------------------------------
http://redmine.ruby-lang.org

Attachments (1)

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 29410)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+Mon Oct  4 18:14:47 2010  Mark Somerville  <mark@scottishclimbs.com>
+
+	* process.c, signal.c, test/ruby/test_signal.rb,
+	  test/thread/test_timer_thread.rb: use the timer thread only when
+	  needed to schedule Ruby threads [bug #3436].
+
 Mon Oct  4 12:43:47 2010  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* parse.y (regexp): dregexp has literal string only at the head
Index: thread.c
===================================================================
--- thread.c	(revision 29410)
+++ thread.c	(working copy)
@@ -402,7 +402,22 @@
 static VALUE rb_threadptr_raise(rb_thread_t *, int, VALUE *);
 void rb_thread_recycle_stack_release(VALUE *);
 
+static int
+vm_living_thread_num(rb_vm_t *vm)
+{
+    return vm->living_threads->num_entries;
+}
+
 void
+rb_thread_start_timer_thread(void)
+{
+    system_working = 1;
+    if (vm_living_thread_num(GET_VM()) > 1) {
+        rb_thread_create_timer_thread();
+    }
+}
+
+void
 ruby_thread_init_stack(rb_thread_t *th)
 {
     native_thread_init_stack(th);
@@ -562,6 +577,7 @@
 	th->status = THREAD_KILLED;
 	rb_raise(rb_eThreadError, "can't create Thread (%d)", err);
     }
+    rb_thread_start_timer_thread();
     return thval;
 }
 
@@ -577,6 +593,7 @@
 	rb_raise(rb_eThreadError, "uninitialized thread - check `%s#initialize'",
 		 rb_class2name(klass));
     }
+    rb_thread_start_timer_thread();
     return thread;
 }
 
@@ -629,7 +646,6 @@
     return thread_create_core(rb_thread_alloc(rb_cThread), (VALUE)arg, fn);
 }
 
-
 /* +infty, for this purpose */
 #define DELAY_INFTY 1E30
 
@@ -711,6 +727,12 @@
     thread_debug("thread_join: success (thid: %p)\n",
 		 (void *)target_th->thread_id);
 
+    rb_disable_interrupt();
+    if (vm_living_thread_num(th->vm) == 1 && rb_signal_buff_size() == 0) {
+        rb_thread_stop_timer_thread();
+    }
+    rb_enable_interrupt();
+
     if (target_th->errinfo != Qnil) {
 	VALUE err = target_th->errinfo;
 
@@ -2107,12 +2129,6 @@
     return ST_CONTINUE;
 }
 
-static int
-vm_living_thread_num(rb_vm_t *vm)
-{
-    return vm->living_threads->num_entries;
-}
-
 int
 rb_thread_alone(void)
 {
@@ -2719,13 +2735,6 @@
     native_reset_timer_thread();
 }
 
-void
-rb_thread_start_timer_thread(void)
-{
-    system_working = 1;
-    rb_thread_create_timer_thread();
-}
-
 static int
 clear_coverage_i(st_data_t key, st_data_t val, st_data_t dummy)
 {
@@ -4262,7 +4271,7 @@
 	}
     }
 
-    rb_thread_create_timer_thread();
+    rb_thread_start_timer_thread();
 
     (void)native_mutex_trylock;
 }
Index: process.c
===================================================================
--- process.c	(revision 29410)
+++ process.c	(working copy)
@@ -998,7 +998,7 @@
 #define before_exec() \
     (rb_enable_interrupt(), (void)(forked_child ? 0 : (rb_thread_stop_timer_thread(), 1)))
 #define after_exec() \
-  (rb_thread_reset_timer_thread(), rb_thread_start_timer_thread(), forked_child = 0, rb_disable_interrupt())
+  (rb_thread_reset_timer_thread(), rb_thread_start_timer_thread(), forked_child = 0)
 #define before_fork() before_exec()
 #define after_fork() (GET_THREAD()->thrown_errinfo = 0, after_exec())
 
Index: test/ruby/test_signal.rb
===================================================================
--- test/ruby/test_signal.rb	(revision 29410)
+++ test/ruby/test_signal.rb	(working copy)
@@ -181,4 +181,20 @@
     w.close
     assert_equal(r.read, "foo")
   end
+
+  def test_signals_before_and_after_timer_thread
+    count = 0
+    Signal.trap(:USR1) { count += 1 }
+
+    Process.kill :USR1, Process.pid
+    assert_equal 1, count
+
+    th = Thread.new { sleep 0.5 }
+    Process.kill :USR1, Process.pid
+    assert_equal 2, count
+
+    th.join
+    Process.kill :USR1, Process.pid
+    assert_equal 3, count
+  end
 end
Index: test/thread/test_timer_thread.rb
===================================================================
--- test/thread/test_timer_thread.rb	(revision 0)
+++ test/thread/test_timer_thread.rb	(revision 0)
@@ -0,0 +1,21 @@
+require 'test/unit'
+require 'thread'
+
+class TestTimerThread < Test::Unit::TestCase
+  def number_of_lwps
+    `ps -o nlwp #{Process.pid} | tail -n1`.strip.to_i
+  end
+
+  def test_timer_is_created_and_destroyed
+    # TODO: enable this test on other platforms
+    return if /linux/ !~ RUBY_PLATFORM
+
+    assert_equal 1, number_of_lwps
+
+    th = Thread.new { sleep 0.1 }
+    assert_equal 3, number_of_lwps
+
+    th.join
+    assert_equal 1, number_of_lwps
+  end
+end
Index: signal.c
===================================================================
--- signal.c	(revision 29410)
+++ signal.c	(working copy)
@@ -519,6 +519,11 @@
 #if !defined(BSD_SIGNAL) && !defined(POSIX_SIGNAL)
     ruby_signal(sig, sighandler);
 #endif
+
+    rb_vm_t *vm = GET_VM();
+    if (vm->running_thread == vm->main_thread) {
+        rb_threadptr_check_signal(vm->main_thread);
+    }
 }
 
 int

In This Thread