Bug #3696: Fwd: [ruby-list:47272] case when 整数と実数の比較 (ruby 1.9)

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 1.9.3dev (2010-09-01 trunk 29077) [i386-mingw32]

[ko1 (Koichi Sasada)]

=begin
　ささだです．

　本件をバグとして登録します．

-------- Original Message --------
Subject: [ruby-list:47272] case when 整数と実数の比較 (ruby 1.9)
Date: Fri, 30 Jul 2010 14:41:58 +0900
From: 小田 利通 [e-mail]
Reply-To: [e-mail]
To: [e-mail] (ruby mailing list)

小田利通と申します。

ruby 1.9.1 での動作がおかしいように思うのですが、どうですか。

$ ruby -e '
puts RUBY_VERSION
case 1
when 1.0 ; puts "SAME"
else puts "DIFFERNT"
end '

=> 1.9.1
DIFFERNT

=> 1.8.7
SAME

when 節を変数にするとちゃんと動きます。

T.Oda
=end

[ikk775 (Ikuo KOBORI)]

=begin
小堀といいます。

以下のようにして修正できましたので、報告します。
whenにあらわれるのがすべて定数のとき、整数だろうと浮動小数点数であろうとハッシュテーブルに登録しています。
すると小数と整数の組み合わせでは、比較しても一致しなくなってしまいます（1 と 1.0 の場合など）。
なので小数を整数に丸めても精度が落ちないときは、変換してからテーブルに登録するようにしました。case に小数があらわれたときも同様です。

その他の場合にはテーブルを作ったとしても、入る物と型が一致していないと値は同じになることはないので、それぞれ特別扱いにしました。こうすることで case に独自のオブジェクトが渡されたときに対応できます。例えばつぎのような場合です。

class C
def ==(other)
true
end
end

obj = C.new
case obj
when 1
p "OK"
else
p "NG"
end

このときにも "OK" と出力されるようになります。

diff --git a/compile.c b/compile.c
index 2fd804c..f5f3508 100644
--- a/compile.c
+++ b/compile.c
@@ -2302,14 +2302,22 @@ case_when_optimizable_literal(NODE * node)
{
switch (nd_type(node)) {
case NODE_LIT: {

• VALUE v = node->nd_lit;
• if (SYMBOL_P(v) || rb_obj_is_kind_of(v, rb_cNumeric)) {

•  return v;
  

• }
• break;

•    VALUE v = node->nd_lit;
  

•    if ((SYMBOL_P(v) || rb_obj_is_kind_of(v, rb_cInteger))) {
  

•        return v;
  

•    }
  

•    if (rb_obj_is_kind_of(v, rb_cFloat)) {
  

•        if (RFLOAT_VALUE(v) == (long long)RFLOAT_VALUE(v)) {
  

•            return rb_funcall(v, rb_intern("to_i") , 0);
  

•        }
  

•        else {
  

•            return v;
  

•        }
  

•    }
  

•    break;
   }
   case NODE_STR:
  

• return node->nd_lit;

•    return node->nd_lit;
  

  }
  return Qfalse;
  }
  diff --git a/insns.def b/insns.def
  index fcd97ae..bcf5aa8 100644
  --- a/insns.def
  +++ b/insns.def
  @@ -1260,28 +1260,40 @@ opt_case_dispatch
  (..., VALUE key)
  () // inc += -1;
  {

• if (BASIC_OP_UNREDEFINED_P(BOP_EQQ)) {
• VALUE val;
• if (st_lookup(RHASH_TBL(hash), key, &val)) {

•  JUMP(FIX2INT(val));
  

• }
• else {

•  JUMP(else_offset);
  

• }
• }
• else {
• struct opt_case_dispatch_i_arg arg;

• struct opt_case_dispatch_i_arg arg;

• arg.obj = key;
• arg.label = -1;
• st_foreach(RHASH_TBL(hash), opt_case_dispatch_i, (st_data_t)&arg);
• if (arg.label != -1) {

•  JUMP(arg.label);
  

• }
• else {

•  JUMP(else_offset);
  

• }

• switch(TYPE(key))
• {

•  case T_FLOAT:
  

•      if (RFLOAT_VALUE(key) == (long long)RFLOAT_VALUE(key)) {
  

•          key = rb_funcall(key, rb_intern("to_i") , 0);
  

•      }
  

•  case T_SYMBOL: /* fall through */
  

•  case T_FIXNUM:
  

•  case T_BIGNUM:
  

•  case T_STRING:
  

•    if (BASIC_OP_UNREDEFINED_P(BOP_EQQ)) {
  

•        VALUE val;
  

•        if (st_lookup(RHASH_TBL(hash), key, &val)) {
  

•            JUMP(FIX2INT(val));
  

•        }
  

•        else {
  

•            JUMP(else_offset);
  

•        }
  

•        break;
  

•    }
  

•  default: /* fall through (else) */
  

•      arg.obj = key;
  

•      arg.label = -1;
  

•      st_foreach(RHASH_TBL(hash), opt_case_dispatch_i, (st_data_t)&arg);
  

•      if (arg.label != -1) {
  

•          JUMP(arg.label);
  

•      }
  

•      else {
  

•          JUMP(else_offset);
  

•      }
  

•      break;
  

  }
  }

=end

[wanabe (_ wanabe)]

• ruby -v set to ruby 1.9.3dev (2010-09-01 trunk 29077) [i386-mingw32]

=begin
ワナベと申します。

パッチありがとうございます。拝見しました。
大筋ではバッチリだと思うのですが、long long を使っているところや
速度的に不利な rb_obj_is_kind_of() を使っているところなど
ごく細かい部分が気になったので少し変えてみました。

VM の変更ではありますが、影響は限定的と思われるので
コミットしてしまおうと思いますがよろしいでしょうか。

diff --git a/compile.c b/compile.c
index 2fd804c..189ee43 100644
--- a/compile.c
+++ b/compile.c
@@ -2303,6 +2303,10 @@ case_when_optimizable_literal(NODE * node)
switch (nd_type(node)) {
case NODE_LIT: {
VALUE v = node->nd_lit;

• if (TYPE(v) == T_FLOAT &&

•  RFLOAT_VALUE(v) == (LONG_LONG)RFLOAT_VALUE(v)) {
  

•  return LL2NUM(RFLOAT_VALUE(v));
  

• }
  if (SYMBOL_P(v) || rb_obj_is_kind_of(v, rb_cNumeric)) {
  return v;
  }
  diff --git a/insns.def b/insns.def
  index fcd97ae..d6f8153 100644
  --- a/insns.def
  +++ b/insns.def
  @@ -1260,16 +1260,26 @@ opt_case_dispatch
  (..., VALUE key)
  () // inc += -1;
  {

• if (BASIC_OP_UNREDEFINED_P(BOP_EQQ)) {
• VALUE val;
• if (st_lookup(RHASH_TBL(hash), key, &val)) {

•  JUMP(FIX2INT(val));
  

• }
• else {

•  JUMP(else_offset);
  

• switch(TYPE(key)) {

•  case T_FLOAT:
  

• if (RFLOAT_VALUE(key) == (LONG_LONG)RFLOAT_VALUE(key)) {

•  key = LL2NUM(RFLOAT_VALUE(key));
  

• }

•  case T_SYMBOL: /* fall through */
  

•  case T_FIXNUM:
  

•  case T_BIGNUM:
  

•  case T_STRING:
  

• if (BASIC_OP_UNREDEFINED_P(BOP_EQQ)) {

•  VALUE val;
  

•  if (st_lookup(RHASH_TBL(hash), key, &val)) {
  

•  JUMP(FIX2INT(val));
  

•  }
  

•  else {
  

•  JUMP(else_offset);
  

•  }
  

•  break;
  

  }

• }
• else {

•  default: { /* fall through (else) */
  

  struct opt_case_dispatch_i_arg arg;

  arg.obj = key;
  @@ -1282,6 +1292,7 @@ opt_case_dispatch
  else {
  JUMP(else_offset);
  }

•  }
  

  }
  }

=end

[matz (Yukihiro Matsumoto)]

=begin
まつもと ゆきひろです

In message "Re: [ruby-dev:42179] [Ruby 1.9-Bug#3696] Fwd: [ruby-list:47272] case when 整数と実数の比較 (ruby 1.9)"
on Sun, 5 Sep 2010 09:11:48 +0900, _ wanabe [e-mail] writes:

|パッチありがとうございます。拝見しました。
|大筋ではバッチリだと思うのですが、long long を使っているところや
|速度的に不利な rb_obj_is_kind_of() を使っているところなど
|ごく細かい部分が気になったので少し変えてみました。
|
|VM の変更ではありますが、影響は限定的と思われるので
|コミットしてしまおうと思いますがよろしいでしょうか。

どうぞ。

=end

[wanabe (_ wanabe)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

=begin
This issue was solved with changeset r29203.
Koichi, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end

[ko1 (Koichi Sasada)]

=begin
　ささだです。

　この件があって、opt_case_dispatch を見ていたんですが、ヒットしないとき
の挙動について、少し考えてみました。

　ヒットしないとき（型がこの最適化できる型じゃなかった、もしくは値が違っ
た）ときは、st_foreach で === を行い、case/when を実現しています。です
が、いくつかの理由から（たぶん）、=== を順に行うバイトコードも（無駄に）
生成されています。つまり、opt_case_dispatch から、単に抜けるだけでも、き
ちんと === をするわけです。

　さて、どっちが速いのか、と調べたところ、st_foreach よりも、バイトコー
ド実行に任せたほうが若干速いことがわかりました。st_foreach でたどるリス
トが長ければ長いほど、その傾向が顕著であることがわかりました。

　理由は、foreach でたどるよりも VM でたどる方が速いとか、そういうのだと
思います。

　というわけで、その報告と、修正をお送りします。

　何度も同じハッシュに対して Hash#each するようなコードは eval で展開し
ちゃった方が速いかもね、という知見でもありますが、さてそういうケースはど
れだけあるんだろうか？

mswin32
http://www.atdot.net/fp_store/f.0tse9l/file.graph.png

linux32 (on VirtualBox)
http://www.atdot.net/fp_store/f.7tse9l/file.graph.png

グラフは、
横軸が「失敗する when の数（st_foreach でたどる回数）」
縦軸が「実行時間（秒）」
です。

ちなみに、もちろんヒットするときは when の数は関係なく速いです。

テストコード：
require 'benchmark'

n = 10_000_000
Benchmark.bm{|x|
[true, false, Object.new].each{|obj|
20.times{|i|
ary = (0..i).to_a
obj = i if obj == true
x.report("#{obj}\t#{i+1}"){
eval %Q{
n.times{
case obj
when #{ary.join(", ")}
# ignore
else
# ignore
end
}
}
}
}
}
}

パッチ：

Index: insns.def

--- insns.def (revision 29350)
+++ insns.def (working copy)
@@ -1281,20 +1281,8 @@
}
break;
}

•  default: { /* fall through (else) */
  

• struct opt_case_dispatch_i_arg arg;
• arg.obj = key;
• arg.label = -1;
• st_foreach(RHASH_TBL(hash), opt_case_dispatch_i, (st_data_t)&arg);
• if (arg.label != -1) {

•  JUMP(arg.label);
  

• }
• else {

•  JUMP(else_offset);
  

• }

•  }
  

•  default:
  

• break;
  }
  }

Index: vm_insnhelper.c

--- vm_insnhelper.c (revision 29350)
+++ vm_insnhelper.c (working copy)
@@ -1699,22 +1699,3 @@
return Qundef;
}

-struct opt_case_dispatch_i_arg {

• VALUE obj;
• int label;
  -};

-static int
-opt_case_dispatch_i(st_data_t key, st_data_t data, st_data_t p)
-{

• struct opt_case_dispatch_i_arg *arg = (void *)p;
• if (RTEST(rb_funcall((VALUE)key, idEqq, 1, arg->obj))) {
• arg->label = FIX2INT((VALUE)data);
• return ST_STOP;
• }
• else {
• return ST_CONTINUE;
• }
  -}

--
// SASADA Koichi at atdot dot net

=end