Bug #4926

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

Added by Kazuki Tsujimoto almost 3 years ago. Updated almost 3 years ago.

[ruby-dev:43902]
Status:Closed
Priority:Normal
Assignee:Koichi Sasada
Category:core
Target version:1.9.3
ruby -v:- Backport:

Description

=begin
辻本です。

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

$ make RUBYOPT=-W TESTS='ruby/enc/testemoji.rb --gc-stress' test-all
/home/k
tsj/work/ruby-trunk/test/ruby/enc/testemoji.rb:154: warning: instance variable @aiueosjis not initialized
1) Failure:
testfromiso2022jp(Emoji::TestKDDI) [/home/ktsj/work/ruby-trunk/test/ruby/enc/testemoji.rb:154]:
Exception raised:
<#>.
(全ログは長いので添付します。)

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

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

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

diff --git a/vminsnhelper.c b/vminsnhelper.c
index 366ac4a..3736055 100644
--- a/vminsnhelper.c
+++ b/vm
insnhelper.c
@@ -1259,7 +1259,7 @@ vmgetivar(VALUE obj, ID id, IC ic)
#if USE
ICFORIVAR
if (TYPE(obj) == TOBJECT) {
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 @@ vmsetivar(VALUE obj, ID id, VALUE val, IC ic)
rb
check_frozen(obj);

  if (TYPE(obj) == T_OBJECT) {
  • VALUE klass = RBASIC(obj)->klass;
  • VALUE klass = rbobjclass(obj);
    stdatat index;

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

test_emoji.rb.log (4.86 KB) Kazuki Tsujimoto, 06/26/2011 02:44 AM

Associated revisions

Revision 32347
Added by Koichi Sasada almost 3 years ago

  • vminsnhelper.c (vmgetivar): check vm state version to invalidate inline chache (ivar index). fixes Bug #4926.
  • vminsnhelper.c (vmsetivar): ditto.

History

#1 Updated by Kazuki Tsujimoto almost 3 years ago

=begin
辻本です。

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

vm.c=63=static void
vm.c:64:vmclearallinlinemethodcache(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
objspaceeachobjects() doesn't work at sweep phase.
vm.c-69- */
vm.c-70-}
=end

#2 Updated by Koichi Sasada almost 3 years ago

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

 ささだです.

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

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

 vmmethodsearch() で行っているように,

 GET_VM_STATE_VERSION() == ic->ic_vmstat

の比較,および ic->icvmstat = GETVMSTATEVERSION() の代入が必要になる
んではないかと思います.

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

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

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

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

vm.c=63=static void
vm.c:64:vmclearallinlinemethodcache(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
objspaceeachobjects() 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/testemoji.rb --gc-stress' test-all
/home/k
tsj/work/ruby-trunk/test/ruby/enc/testemoji.rb:154: warning: instance variable @aiueosjis not initialized
1) Failure:
testfromiso2022jp(Emoji::TestKDDI) [/home/ktsj/work/ruby-trunk/test/ruby/enc/testemoji.rb:154]:
Exception raised:
<#>.
(全ログは長いので添付します。)

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

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

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

diff --git a/vminsnhelper.c b/vminsnhelper.c
index 366ac4a..3736055 100644
--- a/vminsnhelper.c
+++ b/vm
insnhelper.c
@@ -1259,7 +1259,7 @@ vmgetivar(VALUE obj, ID id, IC ic)
#if USE
ICFORIVAR
if (TYPE(obj) == TOBJECT) {
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 @@ vmsetivar(VALUE obj, ID id, VALUE val, IC ic)
rb
check_frozen(obj);

  if (TYPE(obj) == T_OBJECT) {
  • VALUE klass = RBASIC(obj)->klass;
  • VALUE klass = rbobjclass(obj);
    stdatat index;

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

--
// SASADA Koichi at atdot dot net

#3 Updated by Koichi Sasada almost 3 years ago

 ささだです.

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

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

 vmmethodsearch() で行っているように,

 GET_VM_STATE_VERSION() == ic->ic_vmstat

の比較,および ic->icvmstat = GETVMSTATEVERSION() の代入が必要になる
んではないかと思います.

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

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

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

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

vm.c=63=static void
vm.c:64:vmclearallinlinemethodcache(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
objspaceeachobjects() 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/testemoji.rb --gc-stress' test-all
/home/k
tsj/work/ruby-trunk/test/ruby/enc/testemoji.rb:154: warning: instance variable @aiueosjis not initialized
1) Failure:
testfromiso2022jp(Emoji::TestKDDI) [/home/ktsj/work/ruby-trunk/test/ruby/enc/testemoji.rb:154]:
Exception raised:
<#>.
(全ログは長いので添付します。)

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

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

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

diff --git a/vminsnhelper.c b/vminsnhelper.c
index 366ac4a..3736055 100644
--- a/vminsnhelper.c
+++ b/vm
insnhelper.c
@@ -1259,7 +1259,7 @@ vmgetivar(VALUE obj, ID id, IC ic)
#if USE
ICFORIVAR
if (TYPE(obj) == TOBJECT) {
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 @@ vmsetivar(VALUE obj, ID id, VALUE val, IC ic)
rb
check_frozen(obj);

  if (TYPE(obj) == T_OBJECT) {
  • VALUE klass = RBASIC(obj)->klass;
  • VALUE klass = rbobjclass(obj);
    stdatat index;

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

--
// SASADA Koichi at atdot dot net

#4 Updated by Kazuki Tsujimoto almost 3 years ago

=begin
辻本です。

すみません、vmclearallinlinemethod_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

#5 Updated by Yui NARUSE almost 3 years ago

  • Status changed from Open to Assigned
  • Assignee set to Koichi Sasada

#6 Updated by Hiroshi Nakamura almost 3 years ago

  • Target version set to 1.9.3

#7 Updated by Koichi Sasada almost 3 years ago

 ささだです.

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

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

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

 この話は,

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

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

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

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

--
// SASADA Koichi at atdot dot net

#8 Updated by Koichi Sasada almost 3 years ago

 ささだです.

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

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

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

 この話は,

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

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

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

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

--
// SASADA Koichi at atdot dot net

#9 Updated by Kazuki Tsujimoto almost 3 years ago

辻本です。

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

了解しました。

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

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

#10 Updated by Koichi Sasada almost 3 years ago

  • 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.


  • vminsnhelper.c (vmgetivar): check vm state version to invalidate inline chache (ivar index). fixes Bug #4926.
  • vminsnhelper.c (vmsetivar): ditto.

Also available in: Atom PDF