-
Notifications
You must be signed in to change notification settings - Fork 120
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
change: rework GPU features #810
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
一旦ダウンローダー周りだけ見てみました。
リリースノートをパースする形で別にいいんじゃないかなと思いました!
思ってたより複雑じゃなかった。lockの変更は大きいかもだけど。
ただまぁ HTML 的 にこっちの方がいいのではと思った点が1個だけあったのでコメントしてみました。
全然強い意見じゃないです。別にそのままでも良さそう。
|
* add: リリースノートに機械可読な`<table>`を置く * `<table>`の組み立てを`build-spec-table`に集約する <#43 (comment)> * `data-*`属性に情報を書く <VOICEVOX/voicevox_core#810 (comment)> * fixup! `data-*`属性に情報を書く * "アーキテクチャ"に統一する <#43 (comment)>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ちょっといくつか気になった点をコメントしました 🙇
#[error("GPU機能をサポートすることができません:\n{_0}")] | ||
GpuSupport(DeviceAvailabilities), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
エラー文を外から突っ込む、みたいな形式なんですね。
一般的なのかはわからないけど、型あるから追えるし良さそう。
GpuSpec::defaults(), | ||
crate::blocking::Onnxruntime::DISPLAY_NAME, | ||
onnxruntime.supported_devices()?, | ||
|gpu| onnxruntime.test_gpu(gpu), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この処理って結構時間かかったりしますかね・・・?
なんか結構かかりそう感ありますね。。。。
まあ仮に結構時間かかっても、気になってるのはエンジン側の起動時間なので、synthesizer作るのを遅延させるとかで迂回できそうであれば、まあ良さそうかな~~~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
一瞬だと思ったんですがうーんどうなんでしょう。まだ計測はしてませんが、もしかしたら数百msはかかるかも。
後でDirectMLで計測してみようかと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
計測ありがとうございます 🙇
初回起動は割とかかってもおかしくないと直感しています。
デバイスの初期化も入る可能性があるので、そこそこ待つ感じでもおかしくないはず。
まあDirectMLで300msくらいなら全く問題ないかも!
CUDAだともっとかかるかもだけど、まあユーザー数で見ればだいぶ少ないでしょうし・・・。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LinuxのCUDAだと成功時で200msから350msといった感じでした。NVIDIA GPUの数を超えたdevice_id
を指定して失敗した場合も同じような時間で、システムからCUDA自体を抜くと0.5msで失敗するという感じ。
ただ後でWindows機二台(片方はGeForce機、もう片方はRadeon機)でも試すつもりです。Windowsだと秒単位かかるという可能性を否定しきれない… その前にWindows Updateからですが。
どうも「一瞬」よりはだいぶ遅いようなので、非同期ランタイムとか表示にはもうちょっと工夫は入れないとですね。あとSessionOptions
はSynthesizer
内にキャッシュしないと…と思いましたがort側でSend
付けないとですね。ちょっと面倒だ。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
うおおなるほどです、ありがとうございます!!!
そうか、失敗時も時間かかる場合あるんですね・・・。
SessionOptionsはSynthesizer内にキャッシュしないと
わかってないのですが、SessionOptions生成に必要なパラメータをキャッシュする形もあり得るかもと思いました。
SessionOptionsをキャッシュできたほうが手っ取り早いかもなのですが、アイデアまで 🙇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
わかってないのですが、SessionOptions生成に必要なパラメータをキャッシュする形もあり得るかもと思いました。
多分それが現状の形ですね。現状可変なのはIntraOpNumThreads
しかないので、それに対応するものとしてcpu_num_threads
と、EPの有無だけ持っていたはずです。
SessionOptions
自体をキャッシュしたいと言ったのは、SessionOptions
にEPを登録するという形なので、SessionOptions
を使いまわせたら個々のdecodeセッション作成のパフォーマンスが向上する上に設計的にもよいのではないかと思ったからでした。
…と思いましたが、どうやらパフォーマンスについてはONNX Runtime側が上手く吸収してくれているっぽい? WindowsやmacOSだとどうかわかりませんが、そもそもSessionOptions
を使いまわしてよいものかどうかわからないので現状のままでよさそう。
(それでも0.15msくらいはかかりそうですが、許容できる範囲ではあるかと)
[crates/voicevox_core/src/infer/runtimes/onnxruntime.rs:55:9] gpu = Cuda
[crates/voicevox_core/src/infer/runtimes/onnxruntime.rs:66:9] end - start = 312.693262ms
[crates/voicevox_core/src/infer/runtimes/onnxruntime.rs:55:9] gpu = Cuda
[crates/voicevox_core/src/infer/runtimes/onnxruntime.rs:66:9] end - start = 153.807µs
それはともかく、300msから(最悪)数秒かかるならSynthesizer::new
はasync化した方がよいかも。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あ~なるほどです!!
何もせずに早いならそのままでも良い説ありそう。
というかもし使い回す形なら、Synthesizerをまたいで(別のインスタンスにも)渡したくなってきたりも考えれてしまってもっと複雑化してきそうなので、使い回さない形で済むならありがたいかもしれないですね 😇
Synthesizer::newはasync化した方がよいかも。
そうかもだけど、時間かかりそうなら後回しでもそんなに問題ないかもって感じかなと!
VOICEVOX的に気になるのはエンジンの起動速度ですが、まあ多分さすがにかかるにしてもここは1秒未満だと思ってて、まだ他に大きなボトルネックがありそうなのでここが並列になってもあまり変わらないかなと。
ライブラリ的にも、そんなに起動速度が必要になる場面はあまりない気がしてます。
とはいえ確かに並列にできた方がいいかもしれない。
(その100倍ぐらいリリースが待ち遠しいけど・・・!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!!
//! - **`load-onnxruntime`**: ONNX Runtimeを`dlopen`/`LoadLibraryExW`で | ||
//! 開きます。[CUDA]と[DirectML]が利用できます。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これをダウンロードするだけじゃなくproviderが必要なので若干語弊があるかもですが、まあ・・・。
次の流れ的にはonnxruntime 1.18.1のビルドを目指す感じですかね! |
@takejohn 共有です!
|
内容
#804 (comment)の案2.を実装します。
Cargo featureとして
cuda
とdirectml
を廃止し、次の2つに統合します。load-onnxruntime
: すべてのEPが利用可能link-onnxruntime
: CPU以外のEPは利用不可さらにリリースにはONNX Runtimeを含めないようにして、代わりにダウンローダーにonnxruntime-builderからのダウンロード機能を持たせます。これによりVOICEVOX COREとしては「CUDA版ビルド」と「DirectML版ビルド」が無くなります。
また #783 も行います。
acceleration_mode
がAuto
かGpu
のときは「GPUをテスト」し、Auto
のときにすべて失敗するならCPU版にフォールバックします。関連 Issue
ref #804
Closes #783.
その他