From: Narihiro Nakamura <authornari@...>
Date: 2011-07-03T23:20:35+09:00
Subject: [ruby-dev:44028] Re: [ruby-core:37707] [Ruby 1.9 - Bug #3781] FIBER_USE_NATIVE 	が有効だと落ちるスクリプトがある

nariです。

ちと話を整理させてください。
本件で、GCに関するバグを直す方法は今のところ以下の2つの案があります。

(1)gc_mark()時に必ずstack_check()を呼ぶようにする
(2)GC_WATER_MARKをlev<=GC_LEVEL_MAXまでgc_mark()を呼び出せる（おおまかな）サイズにする

(1)に関してはstack_check()の精度がよく、ギリギリまでマシンスタックを使
うことができますが、gc_mark()の速度低下を引き起こすので、駄目そうな気配
がしてます。

一方、(2)はstack_check()の精度が粗いのですが、gc_mark()の速度は変わりま
せん。また、Fiber以外の環境で普通にRubyを使ってる範囲では、ギリギリまで
マシンスタックを使うことはあまりないと思いますので、stack_check()を厳密
にやりすぎるのもどうかという気持ちもあります。

といったところで、速度面でも劣化がない(2)の修正をいれようと思うのですが
いかがでしょうか？

遠藤さんの仰ったような、stack_check()の引数にwater_markを入れるパッチを
作りました。本スレッドで上げてきたベンチマークプログラムの結果では性能
劣化はないことを確認しています。


diff --git a/gc.c b/gc.c
index d5b8dfd..d7b6fc3 100644
--- a/gc.c
+++ b/gc.c
@@ -1277,7 +1277,9 @@ ruby_get_stack_grow_direction(volatile VALUE *addr)
 }
 #endif

-#define GC_WATER_MARK 512
+#define GC_LEVEL_MAX 250
+#define MARK_CALL_FRAME_SIZE 28
+#define GC_WATER_MARK (GC_LEVEL_MAX * MARK_CALL_FRAME_SIZE)

 size_t
 ruby_stack_length(VALUE **p)
@@ -1289,28 +1291,30 @@ ruby_stack_length(VALUE **p)
 }

 static int
-stack_check(void)
+stack_check(int water_mark)
 {
     int ret;
     rb_thread_t *th = GET_THREAD();
     SET_STACK_END;
-    ret = STACK_LENGTH > STACK_LEVEL_MAX - GC_WATER_MARK;
+    ret = STACK_LENGTH > STACK_LEVEL_MAX - water_mark;
 #ifdef __ia64
     if (!ret) {
         ret = (VALUE*)rb_ia64_bsp() - th->machine_register_stack_start >
-              th->machine_register_stack_maxsize/sizeof(VALUE) - GC_WATER_MARK;
+              th->machine_register_stack_maxsize/sizeof(VALUE) - water_mark;
     }
 #endif
     return ret;
 }

+#define WATER_MARK 512
+
 int
 ruby_stack_check(void)
 {
 #if defined(POSIX_SIGNAL) && defined(SIGSEGV) && defined(HAVE_SIGALTSTACK)
     return 0;
 #else
-    return stack_check();
+    return stack_check(WATER_MARK);
 #endif
 }

@@ -1600,8 +1604,6 @@ rb_gc_mark_maybe(VALUE obj)
     }
 }

-#define GC_LEVEL_MAX 250
-
 static void
 gc_mark(rb_objspace_t *objspace, VALUE ptr, int lev)
 {
@@ -1614,7 +1616,7 @@ gc_mark(rb_objspace_t *objspace, VALUE ptr, int lev)
     obj->as.basic.flags |= FL_MARK;
     objspace->heap.live_num++;

-    if (lev > GC_LEVEL_MAX || (lev == 0 && stack_check())) {
+    if (lev > GC_LEVEL_MAX || (lev == 0 && stack_check(GC_WATER_MARK))) {
 	if (!mark_stack_overflow) {
 	    if (mark_stack_ptr - mark_stack < MARK_STACK_MAX) {
 		*mark_stack_ptr = ptr;