Feature #6440: 引数にIOを渡した場合のMarshal.loadにバッファを持たせたい

• Status: Closed
• Assignee: nobu (Nobuyoshi Nakada)
• Target version: 2.0.0

[Glass_saga (Masaki Matsushita)]

=begin
現在の(({Marshal.load}))では、引数に(({IO}))を渡すと(({IO#getbyte}))や(({IO#read}))で当座に必要な部分のみの読み出しを繰り返すので
大量のメソッド呼び出しが発生し、そのコストが無視できません。
そこで、引数に(({IO}))を渡した場合の(({Marshal.load}))にバッファを持たせる事を提案します。

require 'benchmark'
require 'tempfile'

ary = Array.new(1000){ "hoge" }
file = Tempfile.new("foo")
Marshal.dump(ary, file)

Benchmark.bm do |x|
x.report do
100.times do
file.rewind
Marshal.load(file)
end
end
end

file.close

上記のベンチマークでバッファを持つようにしたrubyとtrunkを比較したところ、以下の結果となりました。

trunk(r35660):
user system total real
1.880000 0.000000 1.880000 ( 1.874681)

proposed:
user system total real
0.180000 0.000000 0.180000 ( 0.178556)

patchを添付します。
=end

[mame (Yusuke Endoh)]

• Status changed from Open to Assigned
• Assignee set to nobu (Nobuyoshi Nakada)

なんとなくなかださんに。

--
Yusuke Endoh [e-mail]

[nobu (Nobuyoshi Nakada)]

=begin
細かい問題を修正すればいいんじゃないですかね。

• (({struct load_arg.buf}))のメモリリーク
• (({r_bytes1_partial()}))で(({readpartial}))での不足分を(({read}))で追加したときに((|tmp_ptr|))が不正

あとは感想をいくつか。

• (({arg->partial}))をフラグにするよりsymbolを持たせてはどうか
• (({r_bytes()}))と(({r_bytes0()}))の追加部分は関数に分けることはできないだろうか
• 予約語((({if})))とカッコの間が空いていない
  =end

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)

=begin
もう一点、(({s_getbyte}))も不要になるはずです。
=end

[Glass_saga (Masaki Matsushita)]

なかださん、田中さん、レビューとご意見をありがとうございます。

現在のMarshalの形式では全体のサイズを知る方法がない為に、バッファを持とうとするとどうしても読み過ぎてしまいます。
ungetcでは駄目となると、seekが可能なIOに対してのみバッファを持つようにして、最後にIO#seekで辻褄を合わせるというのはどうでしょうか。
高速化できるIOの種類が限られてしまいますが、互換性は崩さずに済むと思います。

[ko1 (Koichi Sasada)]

(2012/05/18 1:39), Glass_saga (Masaki Matsushita) wrote:

現在のMarshalの形式では全体のサイズを知る方法がない為に、バッファを持とうとするとどうしても読み過ぎてしまいます。
ungetcでは駄目となると、seekが可能なIOに対してのみバッファを持つようにして、最後にIO#seekで辻褄を合わせるというのはどうでしょうか。
高速化できるIOの種類が限られてしまいますが、互換性は崩さずに済むと思います。

　面白いですね．少し考えてみました．

ご提案の「seek が効くもののみ」というのは，要するに File の時は，

って感じですかねぇ．pipe 的なもの（ソケット等）を使うのとどっちが

用途として多いんだろう．

案1）
　でかいオブジェクトをやりとりするのは意識が高い人に違いないので，もっと
意識を高く（低く？），こんな感じで workaround をしてもらう．Marshal した
文字列を Marshal して送る．メモリは食いますが，今時気にするだろうか．

require 'benchmark'
require 'tempfile'

ary = Array.new(10_000){ "hoge" }
file = Tempfile.new("foo")
file2 = Tempfile.new("bar")
Marshal.dump(ary, file)
Marshal.dump(Marshal.dump(ary), file2)

Benchmark.bm do |x|
x.report do
100.times do
file.rewind
Marshal.load(file)
end
end
x.report do
100.times do
file2.rewind
Marshal.load(Marshal.load(file2))
end
end
end

file.close

ruby 2.0.0dev (2012-05-04 trunk 35535) [i386-mswin32_100]
user system total real
6.068000 0.031000 6.099000 ( 6.334804)
0.530000 0.016000 0.546000 ( 0.573573)

案2）真面目に先読み機構を入れる．

　n 要素の Array を load する場合，少なくとも n byte 先読み出来ます（と
いうことを調べるために，始めて Marshal フォーマット *1 を見た）．m 番目
まで読み進めた段階で先読みしたバッファが不足した場合，n - m byte 先読み
出来ます．ネストしてると面倒になりますが（この情報をスタックで管理して，
一番先読み可能なものを集めて先読み，みたいな）．

*1:
http://www.ruby-lang.org/ja/old-man/html/Marshal_A5D5A5A9A1BCA5DEA5C3A5C8.html

　ちょっと，複雑に過ぎるかも．

--
// SASADA Koichi at atdot dot net

[akr (Akira Tanaka)]

2012年5月17日 18:21 Tanaka Akira [e-mail]:

バッファがあると、読みすぎることがあるのではないでしょうか。

読みすぎたぶんを捨ててしまうと、
marshal したデータが複数並んでいるものを読み込む場合に、
動作しなくなってしまうと思います。

個人的に最近作っている tb というもので、この動作を利用しているので、
気になるところです。

気がついたので記録のために書いておきますが、marshal の load が読みすぎないという
性質を process.c でも利用しているようです。

rb_fork_err で、子プロセスの例外を marshal で読み出し、次に errno を読み出し、
エラーメッセージを読み出す、というようなことをやっています。

     if ((read(ep[0], &state, sizeof(state))) == sizeof(state) && state) {
         io = rb_io_fdopen(ep[0], O_RDONLY|O_BINARY, NULL);
         exc = rb_marshal_load(io);
         rb_set_errinfo(exc);
     }

#define READ_FROM_CHILD(ptr, len)
(NIL_P(io) ? read(ep[0], (ptr), (len)) : rb_io_bufread(io,
(ptr), (len)))
if ((size = READ_FROM_CHILD(&err, sizeof(err))) < 0) {
err = errno;
}
if (size == sizeof(err) &&
errmsg && 0 < errmsg_buflen) {
ssize_t ret = READ_FROM_CHILD(errmsg, errmsg_buflen-1);
if (0 <= ret) {
errmsg[ret] = '\0';
}
}

のあたりです。

[田中 哲][たなか あきら][Tanaka Akira]

[Glass_saga (Masaki Matsushita)]

• File patch2.diff patch2.diff added

ささださんの案を元に読み過ぎないような先読み機構を考えてみました。

ささださんが指摘するように、n要素のArrayをm番目まで読んだところでバッファが不足した場合には、
少なくともn - m バイトは先読みできる事が保証されます。

Arrayがネストしている場合には、最も内側にあるArrayに関してはn - m バイト読む事ができますが、
そのひとつ外側のArrayに関しては、最も内側のArrayについて先読みした時点で既に1要素読んでしまっているので、
n - m - 1 バイト先読みする事ができます。それより外側のArrayに関しても同様です。
従って、最も内側のArrayのみn - m バイト先読みする事ができ、それ以外はn - m - 1 バイト先読みする事ができます。

今回は、struct load_argにreadableという「少なくとも何バイト先読みできるか」を表すメンバを追加する事にしました。
r_object0()でtypeがARRAYである場合には、arg->readableにlen - 1を足し、1要素読む毎にデクリメントします。
これは、n - m - 1バイトに対応します。

バッファが不足した場合には、arg->readable + 1バイト先読みできます。
arg->readable + 1とするのは、最も内側のArrayに関してだけn - mバイト先読みする事に相当します。

HashやStructに関しても同様の処理を行なっています。
ただし、これらは1要素に少なくとも2バイトは必要なのでarg->readableには(len - 1) * 2を足し、1要素読む毎に2を引いています。
最も内側のHashやStructに関しては(n - m) * 2バイト、つまりarg->readable + 2バイトまで読めるのですが、
1バイト多く先読みする為だけにr_byteやr_bytes0にtypeを渡す必要もないと思ったので、そこまではやっていません。

[ruby-dev:45637]と同様のベンチマークを実行したところ、以下の結果となりました。

trunk(r35983)
user system total real
0.560000 0.030000 0.590000 ( 0.601471)

proposed:
user system total real
0.090000 0.010000 0.100000 ( 0.113099)

patchを添付します。

[Glass_saga (Masaki Matsushita)]

patch2.diffを適用してコミットしてよいでしょうか？
反対がなければコミットします。

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

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

---

• marshal.c: add marshal readahead. marshalized Array, Hash and Struct
  have size at least number of its elements, marshal readahead will
  read the certain readable length and buffer when it needs more bytes.
  marshal readahead prevents many calls to IO#getbyte and IO#read,
  then it enables performace improvement.
  [ruby-dev:45637] [Feature #6440]