Feature #6440

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

Added by Masaki Matsushita almost 2 years ago. Updated over 1 year ago.

[ruby-dev:45637]
Status:Closed
Priority:Normal
Assignee:Nobuyoshi Nakada
Category:core
Target version:2.0.0

Description

=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

patch.diff Magnifier (4.61 KB) Masaki Matsushita, 05/16/2012 12:49 PM

patch2.diff Magnifier (6.15 KB) Masaki Matsushita, 06/11/2012 10:25 PM

Associated revisions

Revision 37772
Added by glass over 1 year ago

  • 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. [Feature #6440]

History

#1 Updated by Yusuke Endoh almost 2 years ago

  • Status changed from Open to Assigned
  • Assignee set to Nobuyoshi Nakada

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

Yusuke Endoh mame@tsg.ne.jp

#2 Updated by Nobuyoshi Nakada almost 2 years ago

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

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

あとは感想をいくつか。

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

#3 Updated by Nobuyoshi Nakada almost 2 years ago

  • Description updated (diff)

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

#4 Updated by Masaki Matsushita almost 2 years ago

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

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

#5 Updated by Koichi Sasada almost 2 years ago

(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

#6 Updated by Akira Tanaka almost 2 years ago

2012年5月17日 18:21 Tanaka Akira akr@fsij.org:

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

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

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

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

rbforkerr で、子プロセスの例外を 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 READFROMCHILD(ptr, len) \
(NILP(io) ? read(ep[0], (ptr), (len)) : rbiobufread(io,
(ptr), (len)))
if ((size = READ
FROMCHILD(&err, sizeof(err))) < 0) {
err = errno;
}
if (size == sizeof(err) &&
errmsg && 0 < errmsg
buflen) {
ssizet ret = READFROMCHILD(errmsg, errmsgbuflen-1);
if (0 <= ret) {
errmsg[ret] = '\0';
}
}

のあたりです。
--
[田中 哲][たなか あきら][Tanaka Akira]

#7 Updated by Masaki Matsushita almost 2 years ago

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

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

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

今回は、struct loadargに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バイト多く先読みする為だけにrbyteやrbytes0にtypeを渡す必要もないと思ったので、そこまではやっていません。

と同様のベンチマークを実行したところ、以下の結果となりました。

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を添付します。

#8 Updated by Masaki Matsushita over 1 year ago

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

#9 Updated by Anonymous over 1 year ago

  • 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. [Feature #6440]

Also available in: Atom PDF