New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak in compress zlib/lzo column #221

Merged
merged 1 commit into from Oct 20, 2014

Conversation

Projects
None yet
4 participants
@naoa
Member

naoa commented Oct 18, 2014

現在のGroongaでは、zlib圧縮、lzo圧縮させたカラムを参照すると、
伸長させるためにメモリアロケーションしたvalueを開放するタイミング
がなく、メモリリークとなっています。
(既知の不具合で現在、デフォルトでは作れないようになっています。)

https://github.com/groonga/groonga/blob/master/lib/store.c#L1215

これを回避する方法として、grn_io_winに圧縮データを伸長させた値
へのポインタを保持するメンバvalueを追加するのはいかがでしょうか?

grn_io_winにMALLOCしたポインタを保持させれば、grn_ja_unref
メモリを開放させてやることができます。

これにより、APIに大きな変更を与えることなくzlib圧縮がメモリリーク
することなく使えるようになると思うのですがいかがでしょうか?

(そもそもgrn_io_winに乗せるべき値ではないかもしれませんし、
APIの仕様を変えたほうがいいのかもしれませんが。。)

お手数ですが、ご検討ください。

伸長にかかるCPUコストやメモリアロケーションのコストにより、どれほど
利用価値があるものになるかはまだ検証できていませんが、zlibの圧縮率
は高く、Groongaはカラム指向でカラムごとに柔軟に設定できるので、
zlib圧縮が使えると非常に嬉しいなぁと考えています。

@naoa naoa referenced this pull request Oct 18, 2014

Closed

Add snappy compression #222

@naoa

This comment has been minimized.

Show comment
Hide comment
@naoa

naoa Oct 18, 2014

Member

Wikipedia(ja) 30万件、文字数制限なしでベンチマークをとりました。
以下のdrn_benchを使ってますが、ちょっと別の問題が発生してそうだったのでクエリは少し変えており、snippet_htmlではなくtext全部を10件出力しています。

http://droonga.org/ja/tutorial/1.0.7/benchmark/

よければ参考にしてください。この結果では非常に有効だと思われるので是非使いたいと考えています。(スニペット関数等、他に問題がでなければ)
あと、snappyも、伸縮、展開が速く、効果的だと思われるので、よければ #222 もご検討いただけるとうれしいです。

  • 検証環境
    CPU : Intel(R) Xeon(R) CPU E5620 @ 2.40GHz 1CPU 4Core
    Memory : 32GB
    HDD : 2TB(SATA 7200rpm) * 2 Hardware RAID 0 (IOPS:446)

qps

size

load

Member

naoa commented Oct 18, 2014

Wikipedia(ja) 30万件、文字数制限なしでベンチマークをとりました。
以下のdrn_benchを使ってますが、ちょっと別の問題が発生してそうだったのでクエリは少し変えており、snippet_htmlではなくtext全部を10件出力しています。

http://droonga.org/ja/tutorial/1.0.7/benchmark/

よければ参考にしてください。この結果では非常に有効だと思われるので是非使いたいと考えています。(スニペット関数等、他に問題がでなければ)
あと、snappyも、伸縮、展開が速く、効果的だと思われるので、よければ #222 もご検討いただけるとうれしいです。

  • 検証環境
    CPU : Intel(R) Xeon(R) CPU E5620 @ 2.40GHz 1CPU 4Core
    Memory : 32GB
    HDD : 2TB(SATA 7200rpm) * 2 Hardware RAID 0 (IOPS:446)

qps

size

load

@s-yata

This comment has been minimized.

Show comment
Hide comment
@s-yata

s-yata Oct 20, 2014

Contributor

高速な圧縮・復元ができるライブラリとしては LZ4 というものもあります.

圧縮率は Snappy と同じくらいで,圧縮・復元が速いことで知られています.
こちらも検討してみてはいかがでしょうか.

ただ,グラフを見る限り, Snappy から LZ4 に変更したところで,改善の余地はあまりなさそうです.

Contributor

s-yata commented Oct 20, 2014

高速な圧縮・復元ができるライブラリとしては LZ4 というものもあります.

圧縮率は Snappy と同じくらいで,圧縮・復元が速いことで知られています.
こちらも検討してみてはいかがでしょうか.

ただ,グラフを見る限り, Snappy から LZ4 に変更したところで,改善の余地はあまりなさそうです.

daijiro added a commit that referenced this pull request Oct 20, 2014

Merge pull request #221 from naoa/fix-memory-leak-in-zlib/lzo
Fix memory leak in compress zlib/lzo column

@daijiro daijiro merged commit 31cd730 into groonga:master Oct 20, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@daijiro

This comment has been minimized.

Show comment
Hide comment
@daijiro

daijiro Oct 20, 2014

Member

ありがとうございます!取り込ませていただきました。

Member

daijiro commented Oct 20, 2014

ありがとうございます!取り込ませていただきました。

@naoa

This comment has been minimized.

Show comment
Hide comment
@naoa

naoa Oct 20, 2014

Member

森さん
マージありがとうございます。カラム圧縮が使えると非常に
うれしいです。zlibやlzoについては、デフォルト有効でパッケージング
されていたほうが嬉しいと思うので、そういった細かい部分について、
引き続き修正してご提案しようと思います。

矢田さん
LZ4の情報ありがとうございます!知りませんでした。
さらにモダンな圧縮ライブラリがあったんですね。
LZ4は圧縮、展開が速く、LZ4HCは圧縮速度を犠牲にして、
展開は速く圧縮率が高いなどなかなか使いどころがありそうです。

ただ、LZ4はまだRHEL系のbase入りしていなかったり、
Debianのstable入りはしていなかったりするようですし、
Groongaのカラム圧縮の用途ではそこまで頑張らなくても
いいのかもしれませんね。

そういったところも踏まえて、SnappyとLZ4の有用性
を検討したいと思います。
今回はとりあえず、Snappyはクローズしておきました。

Member

naoa commented Oct 20, 2014

森さん
マージありがとうございます。カラム圧縮が使えると非常に
うれしいです。zlibやlzoについては、デフォルト有効でパッケージング
されていたほうが嬉しいと思うので、そういった細かい部分について、
引き続き修正してご提案しようと思います。

矢田さん
LZ4の情報ありがとうございます!知りませんでした。
さらにモダンな圧縮ライブラリがあったんですね。
LZ4は圧縮、展開が速く、LZ4HCは圧縮速度を犠牲にして、
展開は速く圧縮率が高いなどなかなか使いどころがありそうです。

ただ、LZ4はまだRHEL系のbase入りしていなかったり、
Debianのstable入りはしていなかったりするようですし、
Groongaのカラム圧縮の用途ではそこまで頑張らなくても
いいのかもしれませんね。

そういったところも踏まえて、SnappyとLZ4の有用性
を検討したいと思います。
今回はとりあえず、Snappyはクローズしておきました。

@kou

This comment has been minimized.

Show comment
Hide comment
@kou

kou Oct 20, 2014

Member

個人的には、パッケージ配布という観点から、LZOサポートを外してLZ4サポートを入れたい(Snappyでもいいですが、性能がよいならLZ4でいいんじゃないというくらい)ところです。LZOはGPLなので、LZOサポート付きのGroongaパッケージはLGPLではなくGPLにしなければいけません。パッケージを使うのではなく自分でビルドして使う分にはそれでもよいのですが、パッケージのユーザー向けにはLGPLで使えるようにしておきたいです。

今のままだと grn_obj_get_value_() を使っているソート処理で死にそうな気がするので、後でなにか対策を入れておきます。

Member

kou commented Oct 20, 2014

個人的には、パッケージ配布という観点から、LZOサポートを外してLZ4サポートを入れたい(Snappyでもいいですが、性能がよいならLZ4でいいんじゃないというくらい)ところです。LZOはGPLなので、LZOサポート付きのGroongaパッケージはLGPLではなくGPLにしなければいけません。パッケージを使うのではなく自分でビルドして使う分にはそれでもよいのですが、パッケージのユーザー向けにはLGPLで使えるようにしておきたいです。

今のままだと grn_obj_get_value_() を使っているソート処理で死にそうな気がするので、後でなにか対策を入れておきます。

@naoa naoa deleted the naoa:fix-memory-leak-in-zlib/lzo branch Aug 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment