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

音声合成の処理を丸ごとスレッド分離して実行する #692

Merged

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Nov 25, 2023

内容

Synthesizerの音声合成系のメソッドを、すべて丸ごとスレッド分離して実行するようにし、asyncのランタイムを阻害しないようにします。

#545 の以下のタスクを完全に解決します。

- [ ] モデルを実行するところとかは`tokio::task::spawn_blocking`で囲ったりするべきでは?

関連 Issue

#545

その他

Copy link
Member Author

@qryxip qryxip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

補足: #662 を念頭に置いています。

Copy link
Member Author

@qryxip qryxip Nov 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synthesizerstruct Synthesizer(Arc<_>)という構造になったことにより、Arcで包む意味が無くなったため。
(Python APIも同様)

追記: #662 のときには結局Arc<voicevox_core::blocking::Synthesizer>ってなってそうですが、今放置しておく理由も特に無いし、diff的にも別にいいかなと。

Copy link
Member Author

@qryxip qryxip Nov 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java APIと同様。

追記: こっちは #662 以降もそのままのはず (何故ならこっちはasyncなAPIなので)

self.use_gpu
}

// FIXME: ブロッキング版を作る
Copy link
Member Author

@qryxip qryxip Nov 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async_zip さえ差し替えられればよいはず。
(tokio::fsstd::fsの単なるラッパーなので、std::fsに置き換えた上で丸ごとスレッドに包めばよい)

tokio::fsの方、タスクキャンセルを考えると丸ごと包むのはやめた方が良いかも


use super::InferenceRuntimeImpl;

pub(super) struct Synthesizer {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#662 のときになったら、これはvoicevox_core::blocking::Synthesizerとして配置(re-export)する。

///
/// # Performance
///
/// CPU/GPU-boundな操作であるため、非同期ランタイム上では直接実行されるべきではない。
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

すみません! 前にもみたいなこと聞いたかもなのですが、これって何ででしたっけ 🙇

非同期ランタイムはRustの文脈(というよりtokio?)でグローバルにたった1つだけしかなく、別の非同期処理(ファイルI/Oなど)が実行されなくなるから、とかでしたっけ

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!!

1つ質問コメント書いているのですが、おそらく理解はあってると思うのでOKかなと!
マーズします!

@Hiroshiba Hiroshiba merged commit 402c994 into VOICEVOX:main Nov 25, 2023
31 checks passed
mod user_dict;
mod version;
mod voice_model;

#[doc(hidden)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、ここ手元でcargo docで確認するときに外して、そのまま忘れてgit addしてしまったやつですね...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants