Feature #4871: envのコンパクト化

• Status: Closed
• Assignee: -
• Target version: -

[ktsj (Kazuki Tsujimoto)]

=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

[ko1 (Koichi Sasada)]

　ささだです．

　返事が遅くなって済みません．

(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

[ko1 (Koichi Sasada)]

　ささだです．

　返事が遅くなって済みません．

(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

[ktsj (Kazuki Tsujimoto)]

辻本です。

検討ありがとうございます。

From: SASADA Koichi [e-mail]
Date: Thu, 16 Jun 2011 18:38:33 +0900

　この辺は構造が入り組んでいるので，とりあえず 1.9.3 はここは触らない，
と刺せて下さい．それで，GC.stress で動かして，無事だったらその後入れる，
というのはどうでしょうか．というか，この辺の再確認が必要かも．

1.9.3に入れないという方針に異存はありません。
GCの件はこちらでも確認してみたいと思います。

--
Kazuki Tsujimoto

[ktsj (Kazuki Tsujimoto)]

辻本です。

検討ありがとうございます。

From: SASADA Koichi [e-mail]
Date: Thu, 16 Jun 2011 18:38:33 +0900

　この辺は構造が入り組んでいるので，とりあえず 1.9.3 はここは触らない，
と刺せて下さい．それで，GC.stress で動かして，無事だったらその後入れる，
というのはどうでしょうか．というか，この辺の再確認が必要かも．

1.9.3に入れないという方針に異存はありません。
GCの件はこちらでも確認してみたいと思います。

--
Kazuki Tsujimoto

[ktsj (Kazuki Tsujimoto)]

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

[ktsj (Kazuki Tsujimoto)]

辻本です。

--gc-stressオプションをつけた状態でtest-allを実行しましたが、
このパッチに起因するテストの失敗はなかったのでtrunkにコミットしました。

本格的な見直しは改めてということでお願いできればと思います。