-
Notifications
You must be signed in to change notification settings - Fork 131
test: 手元用兼CodSpeed用のベンチマークを用意する #1153
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
test: 手元用兼CodSpeed用のベンチマークを用意する #1153
Conversation
|
@Hiroshiba CodSpeedを利用するにはCodSpeed HQというGitHub Appをインストールする必要があります。VOICEVOX/voicevox_coreを対象にrequestを飛ばしたので、(そちらからはどう見えるのかわかりませんが)ご検討頂けないでしょうか。 参考: https://codspeed.io/docs/integrations/providers/github#permissions |
|
@qryxip すみません、先にGo/NoGoを決めたいです! 目的を言語化しておきたいです。 僕が決めると「TTSの速度に変化があったときにアナウンスできるようにするため」とかになっちゃって、 その目的からこの辺りを導き出したみ。
|
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.
これってcpu-and-sample-vvm.rs側と結構被ってたりしそうでしょうか。
であれば共通化しておくのも良いのかもな〜と思いました!
TODOコメントでも良さそう。
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.
CPU←→GPUだったりsample.vvm←→製品版VVMだったりサンプル数の差はあります。
ただ今気付いたのですが、それらは外部から注入可能です。となると共通化はできそうだし、した方がよさそうに思えてきました。注入の経路は環境変数でJSONを流し込むというのがよさそうかな。"gpu-and-voicevox-vvm"なんてものはどうせしばらく私かヒホさんしかやらないだろうから、少々変な手順になってもよいはず。
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.
注入の経路は環境変数でJSONを流し込む
ま~とりあえずそれで良さそう感!
どこかにドキュメント置いときたいですね。
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.
done.
|
TTS以外にも意図せぬパフォーマンス低下が無いかどうかをチェックしたい、というのも目的にしたいです。できればCIで動くテストと同列に考え、
|
なるほどです! であれば、主要なユーザー操作のe2eのパフォーマンス計測が重要で、あとは問題が起きそうなとこを計測って感じですかね! となると、目的から導けるのはこんな感じですかね!
|
| const MIDDLE_INPUT: InputText = InputText { | ||
| name: "MIDDLE", | ||
| value: "この音声は、ボイスボックスを使用して、出力されています。", | ||
| }; |
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.
これ最初だけはpythonとの比較用に有用ですが、次回以降いらなくなりそう・・・!
遅くなったら消しても良いな、くらい。
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.
一つ削るのはよさそうですね。今のSHORTを削ってMIDDLEとLONGの二つにするというのもよいかも。というのも"こんにちは"は"こんにちは"で短すぎるので、たくさん計測しないとブレが出そうかなと思っています。実際qryxip/voicevox_coreにてサンプル数100×10で試したときは何もしていないのに「+17%の速度改善」がされてしまいました。
(ただ、デフォルトのモードであるCPU Instrumentationだったらブレがもうちょっとマシにはなっていたとは思います)
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.
SHORTを消し、MIDDLEをSHORTにリネームしました。
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.
PRにおいてはLONGのベンチをしないようにしました。mainブランチではされるはずです。
LONG抜きだと実行時間はこんな感じになりました。結構余裕ありそうなのでベンチ内容をもうちょっと盛ってもよいかも?
[追記] Macro Runnerの制限が月120分から月600分になったらしいので、そっちを使えば時間はもっと縮むかもしれません。
#1153 (comment)
|
@qryxip Github Appをインストールしました! |
|
|
CodSpeed Performance ReportCongrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
| //! [`CONFIG`]の内容は環境変数`VOICEVOX_CORE_BENCH_CONFIG`から上書きすることができる。 | ||
| //! | ||
| //! # 例 |
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.
📝 rustdocの特殊記法 ([`Rustのシンボル`]はRustのシンボルへのリンクになり、#は<h2>になる)。
| - name: Set up Rust toolchain, cache and cargo-codspeed binary | ||
| uses: moonrepo/setup-rust@v1 | ||
| with: | ||
| bins: cargo-codspeed | ||
| cache-target: release | ||
| inherit-toolchain: true |
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.
📝 このsetup-rustはCodSpeedのドキュメントで紹介されていたもの。今回使ってみて結構よさそうだったので、 #313 から使い続けてきてる機構をこれで置き換えてみたい。
026f64a to
03a159d
Compare
|
以前話した、"16-core ARM64 CPU, 32 GB RAM"というスペックだけど月120分しか使えないというMacro Runnerですが先週の時点で月600分になったようです。しかも"open source project"であれば連絡すれば増加を検討してくれるようです。
More Free Macro Runners minutes - Changelog - CodSpeed ベンチマークの時間が短縮されるであろう上にブレもおそらくかなり軽減されることを考えると、検討の余地があると思います。なおサードパーティのrunnerを使う関係上、organizationからの設定が必要なようです。 [追記] 仮に月600分じゃ足りなかったとしても、そのときになってからでもGHAランナーに戻せるのでMacro Runnerからトライしてみるのは十分にありだと思います。 |
|
時間が足りなくなったら差がわからなくなって
これ理由気になりました!差を知らないので、特にブレが軽減されそうな理由が思いつかなかったためです。 |
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.
Pull Request Overview
This PR sets up benchmarking infrastructure for both local development and CI integration with CodSpeed. The benchmarks serve two purposes: performance monitoring during ONNX Runtime changes and automated CI benchmarking to detect performance regressions.
- Adds comprehensive benchmark suites for both Rust and Python APIs
- Integrates with CodSpeed service for continuous performance monitoring
- Updates dependency versions across the workspace
Reviewed Changes
Copilot reviewed 10 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/guide/dev/benchmarks.md | Documents the benchmarking strategy and usage |
| deny.toml | Updates build script hashes for dependency version changes |
| crates/voicevox_core_python_api/python/benches/test_tts.py | Implements Python benchmark suite with CodSpeed integration |
| crates/voicevox_core_python_api/pyproject.toml | Adds pytest-codspeed dependency and Pyright configuration |
| crates/voicevox_core/benches/benches.rs | Implements comprehensive Rust benchmark suite |
| crates/voicevox_core/Cargo.toml | Configures benchmark binary with required features |
| Cargo.toml | Updates workspace dependencies and adds divan benchmarking framework |
| CHANGELOG.md | Documents dependency version changes |
| .github/workflows/test.yml | Excludes benchmark directory from pytest runs |
| .github/workflows/benchmarks.yml | Adds CI workflow for automated benchmark execution |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Rustコードのレビューは @sevenc-nanashi さん頼れるとありがたい!
けどざっと見た感じ別に大丈夫そう感!
| - name: Set up Rust toolchain and cache | ||
| uses: moonrepo/setup-rust@v1 | ||
| with: | ||
| cache-target: release | ||
| inherit-toolchain: true | ||
|
|
||
| - name: Set up Python 3.10 | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: "3.10" | ||
|
|
||
| - name: Install Poetry | ||
| run: pip install --upgrade poetry | ||
|
|
||
| - name: Set up test data | ||
| run: cargo check -vp test_util |
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.
(ただのコメントです)
このあたり共通化していっても良いかもですね!
Rustをセットアップするaction、pythonをsetupするaction、みたいな。
もちろんこのPRじゃなくても良さそう。
| - run: poetry run maturin develop --locked | ||
| - name: pytestを実行 | ||
| run: poetry run pytest | ||
| run: poetry run pytest ./python/test/ |
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.
ちなみにこの差の理由とかあったりしますか 👀
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.
それが無いと./python/benches/test_tts.pyが「テスト」として実行されます。ちなみにそのときの挙動としては、私が観察した限りでは「pytest_codspeedとして設定した(warmup_)roundsの回数を無視してベンチの内容が一度だけ実行される」というものらしいです。
以上よりベンチを含めても多分無害ではあるとは思うのですが、ここでの「pytestを実行」の趣旨とは外れてくるかなと思い./python/test/に限定しました。
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.
あ、なるほどです!!
「pytestを実行」という目的からならpytestをそのまま動かすほうが良さそう感。
「pytestでベンチマーク以外のテストを実行」ならこれで良さそう感。
「pytestでテストを実行」だけでも良いかも。
(ベンチマークもテストな気がするから違和感あるけど、まあ。。。諦められる範囲かも。)
アナウンスできそうなレベルのTTS速度の変化が期待できそうなときはどの道Windows PCのGPUでやる(benchmarks.mdに書いた1.の方)というのを考えてました。CIでやる目的はあくまで不意のリグレッションを防ぐのを主にした方がよいかなと。 色々予想できないところが多いのでとりあえず一度運用してみて、600分のうち200分くらい使ったら再考する形はどうでしょうか?"send us a note with a few details about your repo"もそのときに考えたいかなと思ってます。
"low noise and high precision"と謳ってるからですね。CodSpeedのデフォルト実行モード(CPU Instrumentation)ではGHAランナーのノイズを低減するために、わざわざValgrindを使って数十倍の時間をかけて実行するくらいなのでそれに匹敵するというのであれば期待できそうです。 あとGHAのランナーは実行の安定性について一切保証してないので、高精度を謳ってくれるだけ期待できるかなと思った次第です。まあ一度試さないとよくわからないとは思います。
「
|
あ、目的の片方だけ引用しましたが、もう片方の目的も満たせないのではと思っています。 ブレがでかいのは厄介ですね! とりあえず現時点で600min/月を超えそうかどうかだけ見積もってみるのはどうでしょうか。 ということで1回CodSpeedのリモート環境で動かしてみるのはどうでしょう? |
Co-authored-by: Hiroshiba <[email protected]>
そうですね。このPRをマージしなくてもこのPRの中で試せると思います。ただそのためには、organization側での設定が必要なようです(↓)。ということで"Organization Settings > Actions > Runner groups > Default"の"Repository Access"の設定をお願いできないでしょうか。
|
|
@qryxip ただこのグループに |
|
なんかダメっぽかったので |
|
ダメっぽかったので、Walltime on GHAランナーに戻すことに…。 CodSpeed Macro Runnerを使うためのハードルは多分こんな感じ。このPRでは諦める。
|
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.
1つ確認が問題なさそうであればLGTM!!
これってベンチマークが10%とか差があったとき、テストfailする感じでしょうか? 👀
もしそうならしばらくはワーニングにしとくのもありかもとか思いました!
どれくらいブレるかわからないので。
(ChatGPTに聞いた感じ、テストが落ちるわけじゃない・・・? https://chatgpt.com/s/t_68c95ab4d33081919e350dc0b5bd2746 )
まあでもとりあえずテストは落ちるかもってことで様子見するのもダメじゃないと思います。
新規コミッターが遭遇すると驚くと思うので、早めにフォローしてあげたいなくらい。
1e2f893 to
a33b2f0
Compare
|
基本的にそのChatGPTの通りだとは思いますが、 フォローとしては、CONTRIBUTING.mdでの言及を入れました。どうでしょうか? |
|
❌になりえるなら案内欲しいですね! |
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.
特に問題なさそう。
|
Valgrind抜きのGHAランナーだと、わかってはいましたが結構ブレが生じますね。特に |


内容
次の両方の役割を果たせるベンチマークコードを追加し、2.をCodSpeedに載せるようにする。
CIでのベンチマークは、すべて
ubuntu-latest上でのwalltimeとする。CodSpeed Macro Runnerを使うためにはいくつかハードルがありそうなので、このPRでは諦める。(cf. #1153 (comment))
関連 Issue
Resolves: #901
その他