From: Yusuke Endoh <mame@...>
Date: 2012-11-27T00:53:58+09:00
Subject: [ruby-dev:46633] Re: [ruby-changes:25793] kosaki:r37850 (trunk): * bignum.c (bigdivrem): restart calculation when bigdivrem1 was

遠藤です。

2012/11/26 KOSAKI Motohiro <kosaki.motohiro@gmail.com>:
> mrknさんへ、
>
> 今日、ささださんとThread.control_interruptを議論しているときに、bignumのrb_thread_call_without_gvl()の使い方が間違っていることに気づいて修正を２つほど入れました。

この辺を最初に作ったのは遠藤の予感。すんません。
修正はどちらも妥当だと思います。


> さて、申し訳ないのですが、依頼が２点あります。
>
> １）test-all にテストを追加したいのですが、上記テストはあまりにも遅すぎて好ましくありません。なにかテストを考えてもらえないでしょうか

うーん、テストデータを徐々に大きくして、適度に割り込めるタイミングを
探すとか？
うまく動くかわかりませんが、適当に作ってみたものをパッチに含めてあります。


> ２）現状の
> restart処理は一番最初まで、大きく処理を戻してるので大変非効率。たぶんもう少し綺麗にretryできるはずなので、最適化についてちょっと考えてみてもらえませんか？

bigdivrem1 の計算状態を struct big_div_struct に持たせれば再開できる
と思うので、以下のパッチでどうでしょう。
volatile 不足とか自信ないので誰かに見てもらいたいなあ。


diff --git a/bignum.c b/bignum.c
index 798571d..234a036 100644
--- a/bignum.c
+++ b/bignum.c
@@ -2655,7 +2655,7 @@ rb_big_mul(VALUE x, VALUE y)
 }

 struct big_div_struct {
-    long nx, ny;
+    long nx, ny, j, nyzero;
     BDIGIT *yds, *zds;
     volatile VALUE stop;
 };
@@ -2664,21 +2664,23 @@ static void *
 bigdivrem1(void *ptr)
 {
     struct big_div_struct *bds = (struct big_div_struct*)ptr;
-    long nx = bds->nx, ny = bds->ny;
-    long i, j, nyzero;
+    long ny = bds->ny;
+    long i, j;
     BDIGIT *yds = bds->yds, *zds = bds->zds;
     BDIGIT_DBL t2;
     BDIGIT_DBL_SIGNED num;
     BDIGIT q;

-    j = nx==ny?nx+1:nx;
-    for (nyzero = 0; !yds[nyzero]; nyzero++);
+    j = bds->j;
     do {
-	if (bds->stop) return 0;
+	if (bds->stop) {
+	    bds->j = j;
+	    return 0;
+	}
 	if (zds[j] ==  yds[ny-1]) q = (BDIGIT)BIGRAD-1;
 	else q = (BDIGIT)((BIGUP(zds[j]) + zds[j-1])/yds[ny-1]);
 	if (q) {
-           i = nyzero; num = 0; t2 = 0;
+           i = bds->nyzero; num = 0; t2 = 0;
 	    do {			/* multiply and subtract */
 		BDIGIT_DBL ee;
 		t2 += (BDIGIT_DBL)yds[i] * q;
@@ -2713,22 +2715,16 @@ rb_big_stop(void *ptr)
 }

 static VALUE
-bigdivrem(VALUE x, VALUE y_, volatile VALUE *divp, volatile VALUE *modp)
+bigdivrem(VALUE x, VALUE y, volatile VALUE *divp, volatile VALUE *modp)
 {
-    VALUE y;
     struct big_div_struct bds;
-    long nx, ny;
+    long nx = RBIGNUM_LEN(x), ny = RBIGNUM_LEN(y);
     long i, j;
     VALUE z, yy, zz;
     BDIGIT *xds, *yds, *zds, *tds;
     BDIGIT_DBL t2;
     BDIGIT dd, q;

-  retry:
-    y = y_;
-    nx = RBIGNUM_LEN(x);
-    ny = RBIGNUM_LEN(y);
-
     if (BIGZEROP(y)) rb_num_zerodiv();
     xds = BDIGITS(x);
     yds = BDIGITS(y);
@@ -2799,7 +2795,11 @@ bigdivrem(VALUE x, VALUE y_, volatile VALUE
*divp, volatile VALUE *modp)
     bds.zds = zds;
     bds.yds = yds;
     bds.stop = Qfalse;
+    bds.j = nx==ny?nx+1:nx;
+    for (bds.nyzero = 0; !yds[bds.nyzero]; bds.nyzero++);
     if (nx > 10000 || ny > 10000) {
+      retry:
+	bds.stop = Qfalse;
 	rb_thread_call_without_gvl(bigdivrem1, &bds, rb_big_stop, &bds);

 	if (bds.stop == Qtrue) {
diff --git a/test/ruby/test_bignum.rb b/test/ruby/test_bignum.rb
index b1497a9..eca3a44 100644
--- a/test/ruby/test_bignum.rb
+++ b/test/ruby/test_bignum.rb
@@ -581,6 +581,36 @@ class TestBignum < Test::Unit::TestCase
     assert_interrupt {(65536 ** 65536).to_s}
   end

+  def test_interrupt2
+    return unless Process.respond_to?(:kill)
+    begin
+      trace = []
+      oldtrap = Signal.trap(:INT) {|sig| trace << :int }
+      a = 456 ** 100
+      b = 123 ** 100
+      c = nil
+      100.times do |n|
+        a **= 3
+        b **= 3
+        trace.clear
+        th = Thread.new do
+          sleep 0.1; Process.kill :INT, $$
+          sleep 0.1; Process.kill :INT, $$
+        end
+        c = a / b
+        trace << :end
+        th.join
+        if trace == [:int, :int, :end]
+          assert_equal(a / b, c)
+          return
+        end
+      end
+      skip "cannot create suitable test case"
+    ensure
+      Signal.trap(:INT, oldtrap) if oldtrap
+    end
+  end
+
   def test_too_big_to_s
     if (big = 2**31-1).is_a?(Fixnum)
       return

-- 
Yusuke Endoh <mame@tsg.ne.jp>