Python APIのSynthesizerをclose()可能にする

[qryxip]

内容

以下のことをやります。

ところで最近考えていることとして、Synthsizerとかのオブジェクト、Python APIでは.close()と__enter__を持つべきなんじゃないかと思ってます

https://discord.com/channels/879570910208733277/893889888208977960/1131974735920635985

Synthsizerは計算資源を大きく占有するため、丸ごと破棄する選択肢をPython APIでも取れるようにした方がよいと思いました。また慣習的にも、close()で破棄できるのが期待される可能性はそこそこあるんじゃないかと思います。

逆にUserDictやOpenJtalkあたりは、まあGCに任せてもいいのかなと思います。

関連 Issue

ref #545

その他

もしかしたら必要なのはunload_all(self) -> Noneかもしれない...? (この場合C APIにも同じものがあった方がよい)

[Hiroshiba]

個人的には__enter__やclose()の実装はだいぶ否定的です。

• close()でメモリ解放するのはPythonで一般的ではない
  • ビルトインライブラリでそういうのをご存知でしたら教えて頂ければ 🙇
  • 巨大なメモリを確保しうるnumpyのarrayもメモリ解放のためのメソッドはなさそう
• withの一般的な使い方ではない
  • コンテキストマネージャの代表的な使い方としては、様々なグローバル情報の保存および更新、リソースのロックとアンロック、ファイルのオープンとクローズなどが挙げられます。
• Synthesizerでwithを使う機会がない
  • withは局所的に機能を使いたいときに用いることが多いけど、Synthesizerを局所的に使う機会が思いつかない

もしかしたら必要なのはunload_all(self) -> Noneかもしれない...?

これはあっても良いなと思います！
１つのSynthesizerを使いまわしたい人もいそうで、その場合はunload_allして別VVMをloadする使い方となりそうです。

[qryxip]

まずONNX Runtimeのsessionに注目すると、メモリというよりは外部リソースに近いものだと理解しています (「計算資源」という表現はよくなかったかもしれません)。

あとPython以外だと、ONNX Runtimeの公式Java APIではSessionはAutoClosableですし、公式C# APIではIDisposableです。しかもC#の方はファイナライザでは中身の破棄が行われず、それについての注意書きもあります。

肝心のPython APIにはそのようなものはありませんが、それ以前に__del__の実装すら無さそうなので、Python APIだとcontext manager以前にsessionが抱えるリソースの破棄について何も考えられていないということかと思われます。
(これ、困る人はいないんでしょうか...?)

❯ rg ReleaseSession ./onnxruntime/python/
❯ echo $?
1

❯ rg __del__ ./onnxruntime/python/
❯ echo $?
1

以上よりSessionについては、Pythonでも(広義の)RAIIができてもよいと思いました。

ただこれはさっき気づいたのですが、Synthesizerに対するSession(もしくはVVM単位でのSessionの組)ってis-aじゃないんですよね。Synthesizerは複数のSessionをその中に持ち、追加/削除のI/Fを持ちます。

何で今こんなことを言うのかというと、Java APIとC# APIの需要があるかもしれないことでそれらの実装が現実的になったからですね。慣習に反しない範囲においては、合わせるのがよいのではないかと思っています。

@windymelt @shigobu @yamachu お時間があるならでいいのですが、一点ご意見を伺えると幸いです。SessionがAutoClosable/IDisposableだとして、それを束ねて追加/削除の操作を提供するSynthesizerもそうであるべきでしょうか?
(追記) 重要な情報が抜けてました。SynthesizerにはファイルパスでSessionの追加が行われ、ユーザーはSessionの存在を直接認識しません。

---

close()でメモリ解放するのはPythonで一般的ではない

collections.abcも見たのですが、close()という名前ってそんなに狭い意味を持ってましたっけ...? これについては私の経験がそこまで無いのではっきりとは言えないのですが。

---

• withの一般的な使い方ではない
  • コンテキストマネージャの代表的な使い方としては、様々なグローバル情報の保存および更新、リソースのロックとアンロック、ファイルのオープンとクローズなどが挙げられます。

その説明だと、広義のRAIIとして括ってよいのでは?という気がします。RAMも拡大解釈すれば、リソースと言えるでしょうし。

---

巨大なメモリを確保しうるnumpyのarrayもメモリ解放のためのメソッドはなさそう

NumPyはまあ、その仕組み上容易には提供できないということではないでしょうか。

---

あと今回の件で改めて思ったのですが、今のVoiceModel、ファイルパスとmanifestとmetasだけ知っていて、ファイルディスクリプタを抱えているわけでもなければONNXモデルの中身を持っているわけでもないんですよね。役割がちょっと中途半端なように思えてきました。

[Hiroshiba]

ちょっと言語レベルの話をすると議論が平行しそうなので、APIを利用するユーザーのニーズベースで話すと早いかもと思いました。
（そもそもユーザーのニーズを満たすためにライブラリを提供しているので、その目線がいちばん大事なはず）

unload_allに関しては、１つのSynthesizerを使いまわしたい人にとって便利そうなので賛成です。
例えばVVM１つだけをloadするwebサーバーを作る場合、こうやって使うと思います。（かなり適当な擬似コードです）

synthesizer = Synthesizer()

@POST_API
def set_vvm(vvm_path: str):
  synthesizer.unload_all()
  synthesizer.load(vvm_path)

@POST_API
def synthesize(text: str):
  return synthesizer.synthesis(text)

このコードはwithを使って書けないんですよね。
こんな感じで、Synthesizerをデーモンの中で使う人が主だと思っていて、それとwithが相性悪いから誰も使わないんじゃないかな～という感じです。

withが必要になるありそうなユースケースがあれば納得できると思います。
（unload_allは賛成です！！！！）

(これ、困る人はいないんでしょうか...?)

少なくともメモリーリークしないのであれば、まあ困ることはほぼないだろうなと思いました。
もしGCのタイミングが問題でメモリエラーを起こすのであれば、メモリ増強すると思います 😇

[Hiroshiba]

まあでも、リソースを全部キレイに掃除したいという需要がPythonユーザーに絶対ないかというと、そんなことは無いと思います。
別に重い実装でもない（メンテナンスコストが高い訳では無い）ので、あとはテストがあれば良いかな～と思いました･･･！！

[qryxip]

ユースケース

JupyterとかColabでsynthesizerを見失わないようにとか...? あとは利用頻度が低いと考えられる声を再生する際、一回限りのSynthesizerを作ったりするケースがもしかしたらあるかもしれません。

with await Synthesizer.new_with_initialize(open_jtalk) as synthesizer:
    await synthesizer.load_voice_model(vvm)
    aq = await synthesizer.audio_query("こんにちは", 0)
    wav = await synthesizer.synthesis(aq, 0)

    IPython.display.Audio(wav)

---

もしGCのタイミングが問題でメモリエラーを起こすのであれば、メモリ増強すると思います 😇

...VRAMをですか? それは少しばかりハードルが高いような...

[Hiroshiba]

JupyterとかColabでsynthesizerを見失わないようにとか...? あとは利用頻度が低いと考えられる声を再生する際、一回限りのSynthesizerを作ったりするケースがもしかしたらあるかもしれません。

あー　APIを利用するユーザーのニーズベースなので、ユーザーが何の作業をしているときにそのコードを書くのかのユースケースが知りたい感じでした。「Web APIサーバーを作るとき」とか。

...VRAMをですか? それは少しばかりハードルが高いような...

普通のメモリを想像してました。まあともかくユースケースかなと！！

---

1. ユースケースを議論してwithを実装
2. テスト実装してwithを実装
3. unload_allだけを実装

どれでもOKです！
２か３ならユースケースの議論はパスできると思います。

[qryxip]

• Merge branch 'main' into HEAD
• 挙動をIOBaseのようにする (10c468b (#555))
• def __exit__に、-> Noneを明示( 068f635 (#555))

[qryxip]

なんか名無し。さんのJavaもちゅうこさんのC#も、"closable"にする方向になっている...?

[sevenc-nanashi]

内部でRustの型を持つためにJNIEnv.set_rust_fieldを使っているんですけど、これはtake_rust_fieldを呼ばないとメモリリークするんですよね。Safetyに「AutoClosableとかを実装して、closeされることを保証すべき」と書かれているのでcloseを追加しました。（finalizerもあったらしいんですけどJava9から非推奨になってるので）

[qryxip]

@sevenc-nanashi
Java APIって確かJava8向けですよね。だとするとfinalizerでもよさそうな気がします。
(Java9でもデストラクタの提供自体が非推奨になったわけではなく、仕組みが変わっただけだと思うので)

[Hiroshiba]

テストの実装だけお願いします 🙇
e2eテストで、withしたあとにcloseされているべきリソースにアクセスできない、みたいな感じのテストでもいいのかなと。
（withが使えることの正常系テストをやっときたい感じです！）

ちなみにwithを２回呼べないことに関して、Pythonはそれがどちらかというと普通みたいな感じでした！
https://docs.python.org/ja/3/library/contextlib.html#single-use-reusable-and-reentrant-context-managers

[qryxip]

• Merge branch 'main' into HEAD
  • Fix: voicevox_json_freeの対象が漏れていたことの修正 #571
  • RustのdoctestをCI #573
  • VoiceModelのget_all_modelsがvvm以外のファイルも読み込もうとしてクラッシュすることの修正 #574
  • C-APIのnew_with_initializeで初期化した場合、metas jsonが空になってしまうことの修正 #575
• 簡単なテストを追加 (1e5f590 (#555))

[qryxip]

• ↑のテストの更新 (244acb6 (#555))

[qryxip]

• ↑のテストの更新(OpenJtalkをfixtureに) (47f5388 (#555))

[sevenc-nanashi]

Java APIって確かJava8向けですよね。だとするとfinalizerでもよさそうな気がします。

確かに。

[qryxip]

• Synthesizerもfixtureに (2f557f3 (#555))

[Hiroshiba]

LGTM！！！

調整ありがとうございます！！明確で綺麗なテストに感じました。
コメントいただければマージしたいと思います。

[Hiroshiba]

マージします！