Feature #6812: Refactor gc.c

• Status: Closed
• Assignee: authorNari (Narihiro Nakamura)
• Target version: 2.0.0

[authorNari (Narihiro Nakamura)]

nariです。

gc.cがごちゃごちゃしてきたので見通しがよくなるようにvm_xx.cみたいにファ
イルを分割しました。この修正ではCやRubyレベルのAPIの変更はありません。

2ヶ月前の開発会議で議論していた件です。

http://bugs.ruby-lang.org/projects/ruby/wiki/DevelopersMeeting20120601

変更内容は以下のURLで閲覧できます。
https://github.com/authorNari/ruby/commit/11e4bffd9e3

パッチは以下のとおりです。
https://github.com/authorNari/ruby/commit/11e4bffd9e3.patch

改善案や意見等ありましたら教えていただけると嬉しいです。

= パッチの簡単な解説

以下のような階層で分割しています。

• gc.c
  +-- gc_alloc.[ch]
  +-- gc_ms.[ch]
  +-- gc_ms_heap.c
  +-- gc_ms_profiler.c

それぞれのファイルの簡単な説明は以下のとおりです。

• gc_alloc.h
  メモリアロケータで実装すべき関数が定義される
  (ruby_xmallocなど)

• gc_ms.h
  GCに実装すべき関数などが定義される
  (gc_markなど)

• gc_alloc.c
  メモリアロケータ周りのコードを記述

• gc_ms.c
  GCアルゴリズム周りのコードを記述

• gc_ms_heap.c
  GC対象のヒープに依存するコードを記述

• gc_ms_profiler.c
  GCのプロファイラ周りのコードを記述する場所

[ko1 (Koichi Sasada)]

パッチを細かく見てないんですが，確認させて下さい．

(2012/07/30 17:07), authorNari (Narihiro Nakamura) wrote:

gc.cがごちゃごちゃしてきたので見通しがよくなるようにvm_xx.cみたいにファ
イルを分割しました。この修正ではCやRubyレベルのAPIの変更はありません。

2ヶ月前の開発会議で議論していた件です。

http://bugs.ruby-lang.org/projects/ruby/wiki/DevelopersMeeting20120601

変更内容は以下のURLで閲覧できます。
https://github.com/authorNari/ruby/commit/11e4bffd9e3

パッチは以下のとおりです。
https://github.com/authorNari/ruby/commit/11e4bffd9e3.patch

改善案や意見等ありましたら教えていただけると嬉しいです。

　方針は賛成です．

　確認したいんですが，コードを移動しただけでしょうか．それとも，gc.c 内
のコードも結構変わってる漢字でしょうか．

= パッチの簡単な解説

以下のような階層で分割しています。

• gc.c
  +-- gc_alloc.[ch]
  +-- gc_ms.[ch]
  +-- gc_ms_heap.c
  +-- gc_ms_profiler.c

それぞれのファイルの簡単な説明は以下のとおりです。

• gc_alloc.h
  メモリアロケータで実装すべき関数が定義される
  (ruby_xmallocなど)

• gc_ms.h
  GCに実装すべき関数などが定義される
  (gc_markなど)

　*.h って要りますか？　インターフェースを切るとき，こっちのほうがよかっ
たりするんだろうか．

• gc_ms_heap.c
  GC対象のヒープに依存するコードを記述

　heap.h は不要でしょうか．

　あと，heap の構造は ms（mark & sweep）に依存するでしょうか．たしかに
copying とかするならバッサリ変わる気がする．が，変わる可能性はあるんだろ
うか．

　実は，その辺を mmap で一度に沢山とっておいて云々，ってのを以前放り出し
ていたのに再チャレンジしたいと思っているんですが，その場合，ms_heap とい
う名前なのかなと考えたのでした．

--
// SASADA Koichi at atdot dot net

[kosaki (Motohiro KOSAKI)]

(7/30/12 9:51 AM), Urabe Shyouhei wrote:

On 2012年07月30日 17:07, authorNari (Narihiro Nakamura) wrote:

+-- gc_ms.[ch]

すいませんがさすがにmsでmark&sweepの意味とは普通の人は分かりません、別の名前にしてください。
俺もわかんなかったので間違いないです。

一般論としてはアルゴリズムはあとから変更されうるのでファイル名としてはもう１つな気がしますねえ

[authorNari (Narihiro Nakamura)]

ko1 (Koichi Sasada) wrote:

パッチを細かく見てないんですが，確認させて下さい．

(2012/07/30 17:07), authorNari (Narihiro Nakamura) wrote:

gc.cがごちゃごちゃしてきたので見通しがよくなるようにvm_xx.cみたいにファ
イルを分割しました。この修正ではCやRubyレベルのAPIの変更はありません。

2ヶ月前の開発会議で議論していた件です。

http://bugs.ruby-lang.org/projects/ruby/wiki/DevelopersMeeting20120601

変更内容は以下のURLで閲覧できます。
https://github.com/authorNari/ruby/commit/11e4bffd9e3

パッチは以下のとおりです。
https://github.com/authorNari/ruby/commit/11e4bffd9e3.patch

改善案や意見等ありましたら教えていただけると嬉しいです。

　方針は賛成です．

　確認したいんですが，コードを移動しただけでしょうか．それとも，gc.c 内
のコードも結構変わってる漢字でしょうか．

ファイル分割する都合で何個か関数を追加してますが、基本的には移動しただけです。

= パッチの簡単な解説

以下のような階層で分割しています。

• gc.c
  +-- gc_alloc.[ch]
  +-- gc_ms.[ch]
  +-- gc_ms_heap.c
  +-- gc_ms_profiler.c

それぞれのファイルの簡単な説明は以下のとおりです。

• gc_alloc.h
  メモリアロケータで実装すべき関数が定義される
  (ruby_xmallocなど)

• gc_ms.h
  GCに実装すべき関数などが定義される
  (gc_markなど)

　*.h って要りますか？　インターフェースを切るとき，こっちのほうがよかっ
たりするんだろうか．

インタフェースとなる関数とかデータ構造がまとまってたほうが読みやすいかなと思ったんですが、いらないかもですね。

• gc_ms_heap.c
  GC対象のヒープに依存するコードを記述

　heap.h は不要でしょうか．

　あと，heap の構造は ms（mark & sweep）に依存するでしょうか．たしかに
copying とかするならバッサリ変わる気がする．が，変わる可能性はあるんだろ
うか．

現在はheapの構造に依存しています。完全に依存なくすとしたら結構大変ですね。
MostlyCopyingの可能性はありますが、当分は入らないと思いますので無視して
いいかもしれません。

　実は，その辺を mmap で一度に沢山とっておいて云々，ってのを以前放り出し
ていたのに再チャレンジしたいと思っているんですが，その場合，ms_heap とい
う名前なのかなと考えたのでした．

なるほど。なるべくGC周りをいじりやすくしたいので、ヒープ構造に依存しないように作り
直してみます。

--
// SASADA Koichi at atdot dot net

[authorNari (Narihiro Nakamura)]

kosaki (Motohiro KOSAKI) wrote:

(7/30/12 9:51 AM), Urabe Shyouhei wrote:

On 2012年07月30日 17:07, authorNari (Narihiro Nakamura) wrote:

+-- gc_ms.[ch]

すいませんがさすがにmsでmark&sweepの意味とは普通の人は分かりません、別の名前にしてください。
俺もわかんなかったので間違いないです。

一般論としてはアルゴリズムはあとから変更されうるのでファイル名としてはもう１つな気がしますねえ

上記、ありがとうございます。アルゴリズム名以外の名前…。gc_coreとかですかね。

[ko1 (Koichi Sasada)]

(2012/07/31 8:23), authorNari (Narihiro Nakamura) wrote:

上記、ありがとうございます。アルゴリズム名以外の名前…。gc_coreとかですかね。

core はやめたほうがいいです（と，vm_core.h という名前を付けた人が反省し
ています）．

個人的には，gc.c -> objspace.c にして，objspace_gc.c,
objspace_gc_(mark|sweep).c みたいにするのが綺麗な気がするんですが，変わ
りすぎてるのでよろしくなさそうです．mark と sweep を別にするのも変だしな．

というわけで，対案が無くて -1 で大変心苦しいですが，

個人的には ms で mark & sweep 以外無いだろう，と思ったので，あまり違和感
が無かったのですが...．

--
// SASADA Koichi at atdot dot net

[authorNari (Narihiro Nakamura)]

ko1 (Koichi Sasada) wrote:

(2012/07/31 8:23), authorNari (Narihiro Nakamura) wrote:

上記、ありがとうございます。アルゴリズム名以外の名前…。gc_coreとかですかね。

core はやめたほうがいいです（と，vm_core.h という名前を付けた人が反省し
ています）．

個人的には，gc.c -> objspace.c にして，objspace_gc.c,
objspace_gc_(mark|sweep).c みたいにするのが綺麗な気がするんですが，変わ
りすぎてるのでよろしくなさそうです．mark と sweep を別にするのも変だしな．

というわけで，対案が無くて -1 で大変心苦しいですが，

個人的には ms で mark & sweep 以外無いだろう，と思ったので，あまり違和感
が無かったのですが...．

なるほど…。

個人的にはgc.c自体は変える気はなく、gc.cにincludeされるファイルの名前なのでアルゴリズム名もありかと思っています。
アルゴリズムが変わったらファイル名も変えればいいかなと。
ですので、gc_mark_sweep.cあたりでどうでしょうか。

ちなみにHotspotVM界隈ですとCMSとか平気で名付けるようです。

[shyouhei (Shyouhei Urabe)]

On 2012年07月31日 10:48, authorNari (Narihiro Nakamura) wrote:

個人的には ms で mark & sweep 以外無いだろう，と思ったので，あまり違和感
が無かったのですが...．

いくらなんでも2文字じゃあちょっと

なるほど…。

個人的にはgc.c自体は変える気はなく、gc.cにincludeされるファイルの名前なのでアルゴリズム名もありかと思っています。
アルゴリズムが変わったらファイル名も変えればいいかなと。
ですので、gc_mark_sweep.cあたりでどうでしょうか。

ちなみにHotspotVM界隈ですとCMSとか平気で名付けるようです。

たとえば今後RubyにもG1GCが実装されてgc_g1gc.cができたら「G1GCはmark&sweepじゃ
ないんかい」的なツッコミは当然入るのではないでしょうか。

[authorNari (Narihiro Nakamura)]

shyouhei (Shyouhei Urabe) wrote:

On 2012年07月31日 10:48, authorNari (Narihiro Nakamura) wrote:

個人的には ms で mark & sweep 以外無いだろう，と思ったので，あまり違和感
が無かったのですが...．

いくらなんでも2文字じゃあちょっと

なるほど…。

個人的にはgc.c自体は変える気はなく、gc.cにincludeされるファイルの名前なのでアルゴリズム名もありかと思っています。
アルゴリズムが変わったらファイル名も変えればいいかなと。
ですので、gc_mark_sweep.cあたりでどうでしょうか。

ちなみにHotspotVM界隈ですとCMSとか平気で名付けるようです。

たとえば今後RubyにもG1GCが実装されてgc_g1gc.cができたら「G1GCはmark&sweepじゃ
ないんかい」的なツッコミは当然入るのではないでしょうか。

上記の発言から読み取ると卜部さんがご指摘されているのは以下の2点と考えて
よろしいでしょうか？

• msは何の略かわからないので避けるべき
• そもそもmark_sweepをファイル名に出すのはよくない
  • mark_sweepの別のgcを実装するときに違うファイル名になると混乱するから

もうすこし抽象的なファイル名にするとしたらやはりgcを使いたいので、
ささださんがおっしゃったように現在のgc.cをなんらかの名前に改名して、
xx_gc.cを新しく作ったほうがスッキリしそうですね。

[shyouhei (Shyouhei Urabe)]

authorNari (Narihiro Nakamura) wrote:

gc.cがごちゃごちゃしてきたので見通しがよくなるように

とありますが、真の狙いは違うのではないか、というふうに思います。見通しがよくなるように、でよければ、mark&sweepとか言わなくてもいいはずですよね。というか、言うまでもない。

本当にやりたいことを整理すると、適切な名前が見えてくるのかもしれないと思います(保証するわけではありませんが)。

[authorNari (Narihiro Nakamura)]

shyouhei (Shyouhei Urabe) wrote:

authorNari (Narihiro Nakamura) wrote:

gc.cがごちゃごちゃしてきたので見通しがよくなるように

とありますが、真の狙いは違うのではないか、というふうに思います。見通しがよくなるように、でよければ、mark&sweepとか言わなくてもいいはずですよね。というか、言うまでもない。

本当にやりたいことを整理すると、適切な名前が見えてくるのかもしれないと思います(保証するわけではありませんが)。

アドバイスありがとうございます。そうですね。目的についてもう少し整理してから出なおしてきます。

[authorNari (Narihiro Nakamura)]

• Status changed from Open to Closed

だいぶん前になりますが、gc.cを少し整理してみたのでこのチケットは閉じておきます。