Bug #4926: --gc-stress付きtest/ruby/enc/test_emoji.rbが失敗する

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: 1.9.3
• ruby -v: -

[ktsj (Kazuki Tsujimoto)]

=begin
辻本です。

--gc-stressオプションをつけてtest/ruby/enc/test_emoji.rbを実行するとテストに失敗します。

$ make RUBYOPT=-W TESTS='ruby/enc/test_emoji.rb --gc-stress' test-all
/home/k_tsj/work/ruby-trunk/test/ruby/enc/test_emoji.rb:154: warning: instance variable @aiueo_sjis not initialized

1. Failure:
   test_from_iso2022jp(Emoji::TestKDDI) [/home/k_tsj/work/ruby-trunk/test/ruby/enc/test_emoji.rb:154]:
   Exception raised:
   <#<NoMethodError: undefined method `force_encoding' for nil:NilClass>>.
   (全ログは長いので添付します。)

この時の流れは以下の通りです。

(1) vm_setivarにてic->ic_classにRBASIC(obj)->klass(Emoji::TestDoCoMoオブジェクトの特異クラス)が代入される。
(2) 1.の特異クラスがGCで解放される。
(3) Emoji::TestKDDIオブジェクトの特異クラスが1.の特異クラスと同じアドレスで作成される。
(4) icのindexとklassのindexが不一致を起こしてインスタンス変数の参照に失敗する。

ivar_get/rb_ivar_setではRBASIC(obj)->klassではなくrb_obj_class(obj)を使ってIV_INDEX_TBLを扱っており、
そちらに揃えれば問題は起きなくなります。以下のパッチでどうでしょうか。

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 366ac4a..3736055 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1259,7 +1259,7 @@ vm_getivar(VALUE obj, ID id, IC ic)
#if USE_IC_FOR_IVAR
if (TYPE(obj) == T_OBJECT) {
VALUE val = Qundef;

• VALUE klass = RBASIC(obj)->klass;

• VALUE klass = rb_obj_class(obj);

  if (ic->ic_class == klass) {
  long index = ic->ic_value.index;
  @@ -1311,7 +1311,7 @@ vm_setivar(VALUE obj, ID id, VALUE val, IC ic)
  rb_check_frozen(obj);

  if (TYPE(obj) == T_OBJECT) {

• VALUE klass = RBASIC(obj)->klass;

• VALUE klass = rb_obj_class(obj);
  st_data_t index;

  if (ic->ic_class == klass) {
  =end

[ktsj (Kazuki Tsujimoto)]

=begin
辻本です。

自己レスです。
チケット登録してから気づいたのですが、これは本質的にはvm_clear_all_inline_method_cacheを
実装しないといけないという話でした。

vm.c=63=static void
vm.c:64:vm_clear_all_inline_method_cache(void)
vm.c-65-{
vm.c-66- /* TODO: Clear all inline cache entries in all iseqs.
vm.c-67- How to iterate all iseqs in sweep phase?
vm.c-68- rb_objspace_each_objects() doesn't work at sweep phase.
vm.c-69- */
vm.c-70-}
=end

[ko1 (Koichi Sasada)]

• ruby -v changed from ruby 1.9.3dev (2011-06-26 trunk 32229) [x86_64-linux] to -

　ささだです．

　なかなか見つけづらいバグを見つけて頂いてありがとうございます．

　この問題は，vm_clear_all_inline_method_cache() が未実装だから，という
理由 ではなく，単に vm_getivar() が vm state を見ていないから，ではな
いかと思っています．そもそも，なんで見てないのか，記憶が確かではないんで
すが...．

　vm_method_search() で行っているように，

 GET_VM_STATE_VERSION() == ic->ic_vmstat

の比較，および ic->ic_vmstat = GET_VM_STATE_VERSION() の代入が必要になる
んではないかと思います．

　しかし，うーん，なんでやってないんだろう．書いたときには，ここが被るこ
とは無い，と何か確信があったんだろうか．と，考えていて返事が遅くなりまし
た．もうちょっと考えてみようと思います．

　なお，vm_clear_all_inline_method_cache(void) は，vmstate カウンタが
オーバーフローしたときにクリアするための関数で，まずここが問題になること
は無いだろう，と思って TODO にしています（そもそも，ここが振り切れること
は，ふつーのプログラムでは考えづらい.... 多分）．

(2011/06/26 10:31), Kazuki Tsujimoto wrote:

自己レスです。
チケット登録してから気づいたのですが、これは本質的にはvm_clear_all_inline_method_cacheを
実装しないといけないという話でした。

vm.c=63=static void
vm.c:64:vm_clear_all_inline_method_cache(void)
vm.c-65-{
vm.c-66- /* TODO: Clear all inline cache entries in all iseqs.
vm.c-67- How to iterate all iseqs in sweep phase?
vm.c-68- rb_objspace_each_objects() doesn't work at sweep phase.
vm.c-69- */
vm.c-70-}
=end

---

Bug #4926: --gc-stress付きtest/ruby/enc/test_emoji.rbが失敗する
http://redmine.ruby-lang.org/issues/4926

Author: Kazuki Tsujimoto
Status: Open
Priority: Normal
Assignee:
Category: core
Target version:
ruby -v: ruby 1.9.3dev (2011-06-26 trunk 32229) [x86_64-linux]

=begin
辻本です。

--gc-stressオプションをつけてtest/ruby/enc/test_emoji.rbを実行するとテストに失敗します。

$ make RUBYOPT=-W TESTS='ruby/enc/test_emoji.rb --gc-stress' test-all
/home/k_tsj/work/ruby-trunk/test/ruby/enc/test_emoji.rb:154: warning: instance variable @aiueo_sjis not initialized

1. Failure:
   test_from_iso2022jp(Emoji::TestKDDI) [/home/k_tsj/work/ruby-trunk/test/ruby/enc/test_emoji.rb:154]:
   Exception raised:
   <#<NoMethodError: undefined method `force_encoding' for nil:NilClass>>.
   (全ログは長いので添付します。)

この時の流れは以下の通りです。

(1) vm_setivarにてic->ic_classにRBASIC(obj)->klass(Emoji::TestDoCoMoオブジェクトの特異クラス)が代入される。
(2) 1.の特異クラスがGCで解放される。
(3) Emoji::TestKDDIオブジェクトの特異クラスが1.の特異クラスと同じアドレスで作成される。
(4) icのindexとklassのindexが不一致を起こしてインスタンス変数の参照に失敗する。

ivar_get/rb_ivar_setではRBASIC(obj)->klassではなくrb_obj_class(obj)を使ってIV_INDEX_TBLを扱っており、
そちらに揃えれば問題は起きなくなります。以下のパッチでどうでしょうか。

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 366ac4a..3736055 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1259,7 +1259,7 @@ vm_getivar(VALUE obj, ID id, IC ic)
#if USE_IC_FOR_IVAR
if (TYPE(obj) == T_OBJECT) {
VALUE val = Qundef;

• VALUE klass = RBASIC(obj)->klass;

• VALUE klass = rb_obj_class(obj);

  if (ic->ic_class == klass) {
  long index = ic->ic_value.index;
  @@ -1311,7 +1311,7 @@ vm_setivar(VALUE obj, ID id, VALUE val, IC ic)
  rb_check_frozen(obj);

  if (TYPE(obj) == T_OBJECT) {

• VALUE klass = RBASIC(obj)->klass;

• VALUE klass = rb_obj_class(obj);
  st_data_t index;

  if (ic->ic_class == klass) {
  =end

--
// SASADA Koichi at atdot dot net

[ko1 (Koichi Sasada)]

　ささだです．

　なかなか見つけづらいバグを見つけて頂いてありがとうございます．

　この問題は，vm_clear_all_inline_method_cache() が未実装だから，という
理由 ではなく，単に vm_getivar() が vm state を見ていないから，ではな
いかと思っています．そもそも，なんで見てないのか，記憶が確かではないんで
すが...．

　vm_method_search() で行っているように，

 GET_VM_STATE_VERSION() == ic->ic_vmstat

の比較，および ic->ic_vmstat = GET_VM_STATE_VERSION() の代入が必要になる
んではないかと思います．

　しかし，うーん，なんでやってないんだろう．書いたときには，ここが被るこ
とは無い，と何か確信があったんだろうか．と，考えていて返事が遅くなりまし
た．もうちょっと考えてみようと思います．

　なお，vm_clear_all_inline_method_cache(void) は，vmstate カウンタが
オーバーフローしたときにクリアするための関数で，まずここが問題になること
は無いだろう，と思って TODO にしています（そもそも，ここが振り切れること
は，ふつーのプログラムでは考えづらい.... 多分）．

(2011/06/26 10:31), Kazuki Tsujimoto wrote:

自己レスです。
チケット登録してから気づいたのですが、これは本質的にはvm_clear_all_inline_method_cacheを
実装しないといけないという話でした。

vm.c=63=static void
vm.c:64:vm_clear_all_inline_method_cache(void)
vm.c-65-{
vm.c-66- /* TODO: Clear all inline cache entries in all iseqs.
vm.c-67- How to iterate all iseqs in sweep phase?
vm.c-68- rb_objspace_each_objects() doesn't work at sweep phase.
vm.c-69- */
vm.c-70-}
=end

---

Bug #4926: --gc-stress付きtest/ruby/enc/test_emoji.rbが失敗する
http://redmine.ruby-lang.org/issues/4926

Author: Kazuki Tsujimoto
Status: Open
Priority: Normal
Assignee:
Category: core
Target version:
ruby -v: ruby 1.9.3dev (2011-06-26 trunk 32229) [x86_64-linux]

=begin
辻本です。

--gc-stressオプションをつけてtest/ruby/enc/test_emoji.rbを実行するとテストに失敗します。

$ make RUBYOPT=-W TESTS='ruby/enc/test_emoji.rb --gc-stress' test-all
/home/k_tsj/work/ruby-trunk/test/ruby/enc/test_emoji.rb:154: warning: instance variable @aiueo_sjis not initialized

1. Failure:
   test_from_iso2022jp(Emoji::TestKDDI) [/home/k_tsj/work/ruby-trunk/test/ruby/enc/test_emoji.rb:154]:
   Exception raised:
   <#<NoMethodError: undefined method `force_encoding' for nil:NilClass>>.
   (全ログは長いので添付します。)

この時の流れは以下の通りです。

(1) vm_setivarにてic->ic_classにRBASIC(obj)->klass(Emoji::TestDoCoMoオブジェクトの特異クラス)が代入される。
(2) 1.の特異クラスがGCで解放される。
(3) Emoji::TestKDDIオブジェクトの特異クラスが1.の特異クラスと同じアドレスで作成される。
(4) icのindexとklassのindexが不一致を起こしてインスタンス変数の参照に失敗する。

ivar_get/rb_ivar_setではRBASIC(obj)->klassではなくrb_obj_class(obj)を使ってIV_INDEX_TBLを扱っており、
そちらに揃えれば問題は起きなくなります。以下のパッチでどうでしょうか。

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 366ac4a..3736055 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1259,7 +1259,7 @@ vm_getivar(VALUE obj, ID id, IC ic)
#if USE_IC_FOR_IVAR
if (TYPE(obj) == T_OBJECT) {
VALUE val = Qundef;

• VALUE klass = RBASIC(obj)->klass;

• VALUE klass = rb_obj_class(obj);

  if (ic->ic_class == klass) {
  long index = ic->ic_value.index;
  @@ -1311,7 +1311,7 @@ vm_setivar(VALUE obj, ID id, VALUE val, IC ic)
  rb_check_frozen(obj);

  if (TYPE(obj) == T_OBJECT) {

• VALUE klass = RBASIC(obj)->klass;

• VALUE klass = rb_obj_class(obj);
  st_data_t index;

  if (ic->ic_class == klass) {
  =end

--
// SASADA Koichi at atdot dot net

[ktsj (Kazuki Tsujimoto)]

=begin
辻本です。

すみません、vm_clear_all_inline_method_cacheの動きについては勘違いしていました。

ちなみに、ポストしたパッチはキャッシュヒット率を向上させる面もあるので
別途取り込みを検討していただければと思います。

現在の実装では、以下のコードでのm呼び出しによる@ivへのアクセスはいずれもキャッシュミスしますが、
パッチを当てると初回のo0.m以外はキャッシュヒットするようになります。

class C
def initialize
@iv = 0
end
def m
@iv
end
end

o0 = C.new
o1 = C.new; class <<o1; end
o2 = C.new; class <<o2; end

100000000.times {
o0.m
o1.m
o2.m
}

手元の環境ではパッチ適用前後の所要時間は以下の通りでした。

パッチ適用前

$ time ./ruby bench.rb
./ruby bench.rb 63.15s user 0.03s system 72% cpu 1:27.33 total

パッチ適用後

$ time ./ruby bench.rb
./ruby bench.rb 58.29s user 0.03s system 71% cpu 1:21.22 total
=end

[naruse (Yui NARUSE)]

• Status changed from Open to Assigned
• Assignee set to ko1 (Koichi Sasada)

[nahi (Hiroshi Nakamura)]

• Target version set to 1.9.3

[ko1 (Koichi Sasada)]

　ささだです．

(2011/06/26 16:19), Kazuki Tsujimoto wrote:

ちなみに、ポストしたパッチはキャッシュヒット率を向上させる面もあるので
別途取り込みを検討していただければと思います。

現在の実装では、以下のコードでのm呼び出しによる@ivへのアクセスはいずれもキャッシュミスしますが、
パッチを当てると初回のo0.m以外はキャッシュヒットするようになります。

　この話は，

i) RBASIC(obj)->klass と rb_obj_class(obj) による速度差（○，×）
ii) 上記によるキャッシュミス率の差（×，○）

のトレードオフになるかと思います．つまり，ごく普通のオブジェクトのほうが
多いと思うのですが，このとき RBASIC(obj)->klass じゃなく
て，rb_obj_class(obj) を使うことによるオーバヘッド増がどれくらい効くの
か，ってなことだと思います（殆ど誤差，という気もします）．

　というわけで，この比較を見てから判断したいのですが，すみませんがこの観
点からベンチマークなどを取ってみては頂けないでしょうか．

　ちなみに，ミスを考慮しないベンチマークは，
trunk/benchmark/bm_vm1_ivar[_set].rb にあります．ミスを考慮したベンチ
マークも加えた方が良いかな．

--
// SASADA Koichi at atdot dot net

[ko1 (Koichi Sasada)]

　ささだです．

(2011/06/26 16:19), Kazuki Tsujimoto wrote:

ちなみに、ポストしたパッチはキャッシュヒット率を向上させる面もあるので
別途取り込みを検討していただければと思います。

現在の実装では、以下のコードでのm呼び出しによる@ivへのアクセスはいずれもキャッシュミスしますが、
パッチを当てると初回のo0.m以外はキャッシュヒットするようになります。

　この話は，

i) RBASIC(obj)->klass と rb_obj_class(obj) による速度差（○，×）
ii) 上記によるキャッシュミス率の差（×，○）

のトレードオフになるかと思います．つまり，ごく普通のオブジェクトのほうが
多いと思うのですが，このとき RBASIC(obj)->klass じゃなく
て，rb_obj_class(obj) を使うことによるオーバヘッド増がどれくらい効くの
か，ってなことだと思います（殆ど誤差，という気もします）．

　というわけで，この比較を見てから判断したいのですが，すみませんがこの観
点からベンチマークなどを取ってみては頂けないでしょうか．

　ちなみに，ミスを考慮しないベンチマークは，
trunk/benchmark/bm_vm1_ivar[_set].rb にあります．ミスを考慮したベンチ
マークも加えた方が良いかな．

--
// SASADA Koichi at atdot dot net

[ktsj (Kazuki Tsujimoto)]

辻本です。

　というわけで，この比較を見てから判断したいのですが，すみませんがこの観
点からベンチマークなどを取ってみては頂けないでしょうか．

了解しました。

Bugの話からそれてしまったので、パフォーマンス向上が見込めそうなら

Featureとして改めてチケット登録します。

[ko1 (Koichi Sasada)]

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

This issue was solved with changeset r32347.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• vm_insnhelper.c (vm_getivar): check vm state version
  to invalidate inline chache (ivar index).
  fixes Bug #4926.
• vm_insnhelper.c (vm_setivar): ditto.