Skip to content
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

#889 以降非同期APIがたまにハングする? #968

Open
3 tasks done
qryxip opened this issue Feb 5, 2025 · 8 comments
Open
3 tasks done

#889 以降非同期APIがたまにハングする? #968

qryxip opened this issue Feb 5, 2025 · 8 comments

Comments

@qryxip
Copy link
Member

qryxip commented Feb 5, 2025

不具合の内容

よくわかっていないのですが、感覚的に、 #889 以降に起こるようになったような気がします。

現象・ログ

https://github.com/VOICEVOX/voicevox_core/actions/runs/13134531858/job/36646837852?pr=964

↑ リンク先は揮発しそうなのでスクショ:

Image

再現手順

期待動作

VOICEVOXのバージョン

OSの種類/ディストリ/バージョン

  • Windows
  • macOS
  • Linux

(Windows以外でも普通に起きるという可能性も普通にある)

その他

@qryxip
Copy link
Member Author

qryxip commented Feb 22, 2025

結構頻繁に発生するっぽいですね…。せめてドキュメントに書いておくべきかも?

@qryxip
Copy link
Member Author

qryxip commented Feb 23, 2025

昨日と今日で2回起きてる。ドキュメントに書いておくのは多分must寄りですね。

原因はなんだろう。ONNX Runtimeかpykeio/ortをアップデートすれば解決しそうな気がしないでもないですが、両者のissueにもそれっぽいものが無さそう。私の直感的にはpykeio/ort側な気がするので、報告される前に問題が解決されたという可能性もありそう。

ハングしている状態を見るに「一度に複数のOrtSessionに対してRunAsyncしている」のが駄目という可能性もありそう?だとすると誰も困ってなくて誰も報告していないということもありそう…?

Image

[追記] いや待て。Pythonでよく起きているということは、ORT/ortとpyo3/experimental-asyncの両方に爆弾を抱えているという可能性も…?

@Hiroshiba
Copy link
Member

ドキュメントでの案内、書くにしてもどう書けば良いかな…。
「現在ハングするバグがあります、詳しくはこのissue見てね」みたいな…?

@qryxip
Copy link
Member Author

qryxip commented Feb 25, 2025

GHA上ではたまにどころか50%くらいの確率でハングしているけど、手元では再現しない…
cpu_num_threads2に絞るだけだと再現しないっぽい。

せめてハングする条件がわかればその条件についてだけドキュメントに書くなりRunAsyncRunに置き換えたりできるのですが…

[追記] pytestが駄目なあたり、「一度に複数のOrtSessionに対してRunAsyncしている」というわけではなさそう。

@qryxip
Copy link
Member Author

qryxip commented Feb 26, 2025

あ、手元で再現したかも。10か20個くらいのプロセスを同時に動かすとそのうちの1,2個くらいがハングする。

seq 1 20 | xargs -I {} -P 20 pytest python/test/test_asyncio_user_dict_load.py

Image

…となると結構厄介。まあドキュメントに書くだけかな…。

あるいはenable_interrogative_upspeak=trueと同じようにenable_cancellable_synthesis=falseを増設して、デフォルトではRunAsyncじゃなくてRun+Rust側のスレッドプールにするとか。

@Hiroshiba
Copy link
Member

普段遣いする条件で起こらないのであれば、ドキュメントでの案内もなくても良いかもですね!
まあでも本当に簡単に案内書いておくと親切で良いかも。

10か20個くらいのプロセスを同時に動かすとそのうちの1,2個くらいがハングする

Github Actionsの環境だと並列に動いてない気がするので、条件が謎ですねぇ。
例えばある程度PCに負荷がかかると処理が間に合わなくなってハングする、みたいな・・・?
いろんな人の環境で動かしてみてもらいたいかもですね。

enable_cancellable_synthesis=falseを増設して

少なくとも0.16では無しで良さそうですが、将来的に作るか難しいですねぇ。
まあもうちょっと検証進めてからが良いかも?
同時に動かしたらたまにハングするということは、実装に何かしらのバグがある(=完全解決できる)可能性もありえるかも。

まとめると、普通に動かす分には大丈夫ならまあ急いで対処も不要そうと考えて、まず原因の特定を目指すのが良さそう!

@qryxip
Copy link
Member Author

qryxip commented Feb 26, 2025

Github Actionsの環境だと並列に動いてない気がするので、条件が謎ですねぇ。
例えばある程度PCに負荷がかかると処理が間に合わなくなってハングする、みたいな・・・?

私はその認識です。GHAでは2,3コアを使えることになってますが、体感的に、計算資源をいつでもフルに使えるとは限らないんじゃないかなと。

調査についてですが、pytestを100回連続でやるやつを用意してみました。

for _ in {1..100}; do poetry run pytest ./python/test/test_asyncio_user_dict_load.py; done

RunAsyncを使うものとRunにしたものを比較したところ、RunAsync版は途中で力尽きる一方、Run版は完走しました。このことから本件は #889 によるものであると思われます。

--- a/crates/voicevox_core/src/infer/runtimes/onnxruntime.rs
+++ b/crates/voicevox_core/src/infer/runtimes/onnxruntime.rs
@@ -194,7 +194,7 @@ impl InferenceRuntime for self::blocking::Onnxruntime {
     async fn run_async(
         OnnxruntimeRunContext { sess, inputs }: Self::RunContext,
     ) -> anyhow::Result<Vec<OutputTensor>> {
-        extract_outputs(&sess.lock().await.run_async(inputs)?.await?)
+        ::blocking::unblock(move || extract_outputs(&sess.lock_blocking().run(inputs)?)).await
     }
 }

Rust APIだと再現しなくて現状Python APIだけで出ているのがちょっともやりますが、少なくともデフォルトの動作としてはRunに倒した方がよいと思いました。というのもユーザーがCIで動かしたりすると多分高確率で発現するので…

@Hiroshiba
Copy link
Member

なるほどです、まあその変更だけで問題が一旦解決するなら、0.16に実装してしまっても良さそうなきがしますね!
テストを回し直す手間もなくせるし、ドキュメントで案内すべきか考える手間もなくせるし!

qryxip added a commit that referenced this issue Feb 27, 2025
デフォルトで`OrtApi::RunAsync`ではなく`OrtApi::Run`を使うようにする。そ
の代わり非同期APIにおけるdecode系の推論のみ、`cancellable`というオプショ
ンで`RunAsync`を使うようにする。predict系の推論は完全にキャンセル不可と
なる。

BREAKING-CHANGE: 推論の挙動の変更。
Refs: #968 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants