Feature #4871
closedenvのコンパクト化
Description
=begin
辻本です。
rb_env_tのenvは以下の構造を持ちますが、env[2](prev env val)は不要ではないでしょうか。
/*
env{
env[0] // special (block or prev env)
env[1] // env object
env[2] // prev env val
};
*/
根拠は次の通りです。
- prev env valに無条件にNULLを代入するようにコードを変更してもmake test、make test-allをパスする。
- コードを読む限りこの値が使われるのはcheck_envの中で値をデバッグ用に出力するときのみ。
- check_envはPROC_DEBUGが真でない限り呼ばれない。通常は偽。
- そもそもcheck_envはYARVがマージされて以来変更されておらず、現在のenvの構造に対応できていない。
prev env valを持たないようにした上でcheck_envをこの変更に対応させたパッチを添付します。
なお、check_envはstdout/stderr両方にデバッグメッセージを出力するため
flushのタイミングによっては出力が混在して見づらいので
出力先をstderrに統一するためのパッチもあわせて用意しました。
=end
Files
Updated by ko1 (Koichi Sasada) almost 13 years ago
ささだです.
返事が遅くなって済みません.
(2011/06/11 21:03), Kazuki Tsujimoto wrote:
rb_env_tのenvは以下の構造を持ちますが、env[2](prev env val)は不要ではないでしょうか。
/*
env{
env[0] // special (block or prev env)
env[1] // env object
env[2] // prev env val
};
*/根拠は次の通りです。
- prev env valに無条件にNULLを代入するようにコードを変更してもmake test、make test-allをパスする。
- コードを読む限りこの値が使われるのはcheck_envの中で値をデバッグ用に出力するときのみ。
- check_envはPROC_DEBUGが真でない限り呼ばれない。通常は偽。
- そもそもcheck_envはYARVがマージされて以来変更されておらず、現在のenvの構造に対応できていない。
prev env valを持たないようにした上でcheck_envをこの変更に対応させたパッチを添付します。
なお、check_envはstdout/stderr両方にデバッグメッセージを出力するため
flushのタイミングによっては出力が混在して見づらいので
出力先をstderrに統一するためのパッチもあわせて用意しました。
この env->env[2] = prevenv は,GC の mark のために作ったんではないかと
思います.で,env->prev_envval があるのに何故あるのかというと,えっと,
うーん.なんであるんだろう.env[1] = self_env も,切っちゃっていいような
気がしてきました.昔と構造が変わって,不要になったのかな.
この辺は構造が入り組んでいるので,とりあえず 1.9.3 はここは触らない,
と刺せて下さい.それで,GC.stress で動かして,無事だったらその後入れる,
というのはどうでしょうか.というか,この辺の再確認が必要かも.
check_env は仰るとおり,初期のデバッグ用に作ったものなので,最近使って
いないので追従できていないのだと思います.
--
// SASADA Koichi at atdot dot net
Updated by ko1 (Koichi Sasada) almost 13 years ago
ささだです.
返事が遅くなって済みません.
(2011/06/11 21:03), Kazuki Tsujimoto wrote:
rb_env_tのenvは以下の構造を持ちますが、env[2](prev env val)は不要ではないでしょうか。
/*
env{
env[0] // special (block or prev env)
env[1] // env object
env[2] // prev env val
};
*/根拠は次の通りです。
- prev env valに無条件にNULLを代入するようにコードを変更してもmake test、make test-allをパスする。
- コードを読む限りこの値が使われるのはcheck_envの中で値をデバッグ用に出力するときのみ。
- check_envはPROC_DEBUGが真でない限り呼ばれない。通常は偽。
- そもそもcheck_envはYARVがマージされて以来変更されておらず、現在のenvの構造に対応できていない。
prev env valを持たないようにした上でcheck_envをこの変更に対応させたパッチを添付します。
なお、check_envはstdout/stderr両方にデバッグメッセージを出力するため
flushのタイミングによっては出力が混在して見づらいので
出力先をstderrに統一するためのパッチもあわせて用意しました。
この env->env[2] = prevenv は,GC の mark のために作ったんではないかと
思います.で,env->prev_envval があるのに何故あるのかというと,えっと,
うーん.なんであるんだろう.env[1] = self_env も,切っちゃっていいような
気がしてきました.昔と構造が変わって,不要になったのかな.
この辺は構造が入り組んでいるので,とりあえず 1.9.3 はここは触らない,
と刺せて下さい.それで,GC.stress で動かして,無事だったらその後入れる,
というのはどうでしょうか.というか,この辺の再確認が必要かも.
check_env は仰るとおり,初期のデバッグ用に作ったものなので,最近使って
いないので追従できていないのだと思います.
--
// SASADA Koichi at atdot dot net
Updated by ktsj (Kazuki Tsujimoto) almost 13 years ago
辻本です。
検討ありがとうございます。
From: SASADA Koichi ko1@atdot.net
Date: Thu, 16 Jun 2011 18:38:33 +0900
この辺は構造が入り組んでいるので,とりあえず 1.9.3 はここは触らない,
と刺せて下さい.それで,GC.stress で動かして,無事だったらその後入れる,
というのはどうでしょうか.というか,この辺の再確認が必要かも.
1.9.3に入れないという方針に異存はありません。
GCの件はこちらでも確認してみたいと思います。
--
Kazuki Tsujimoto
Updated by ktsj (Kazuki Tsujimoto) almost 13 years ago
辻本です。
検討ありがとうございます。
From: SASADA Koichi ko1@atdot.net
Date: Thu, 16 Jun 2011 18:38:33 +0900
この辺は構造が入り組んでいるので,とりあえず 1.9.3 はここは触らない,
と刺せて下さい.それで,GC.stress で動かして,無事だったらその後入れる,
というのはどうでしょうか.というか,この辺の再確認が必要かも.
1.9.3に入れないという方針に異存はありません。
GCの件はこちらでも確認してみたいと思います。
--
Kazuki Tsujimoto
Updated by ktsj (Kazuki Tsujimoto) over 12 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r32781.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
-
vm.c (vm_make_env_each): don't save prev env value.
It is no longer used. [Feature #4871] [ruby-dev:43743] -
vm.c (check_env): changed accordingly.
Updated by ktsj (Kazuki Tsujimoto) over 12 years ago
辻本です。
--gc-stressオプションをつけた状態でtest-allを実行しましたが、
このパッチに起因するテストの失敗はなかったのでtrunkにコミットしました。
本格的な見直しは改めてということでお願いできればと思います。