Skip to content

Conversation

@qryxip
Copy link
Member

@qryxip qryxip commented Sep 11, 2025

内容

次の両方の役割を果たせるベンチマークコードを追加し、2.をCodSpeedに載せるようにする。

  1. TTSの速度に変化があったときにアナウンスできるようにするための、GPUと製品版VVMを使ってPC上で行うベンチマーク
  2. 意図せぬパフォーマンス低下が無いかどうかをチェックするための、CPUとsample.vvmを使ってCI上で行うベンチマーク

CIでのベンチマークは、すべてubuntu-latest上でのwalltimeとする。CodSpeed Macro Runnerを使うためにはいくつかハードルがありそうなので、このPRでは諦める。
(cf. #1153 (comment))

関連 Issue

Resolves: #901

その他

@qryxip qryxip changed the title test: 手元用とCI用にそれぞれベンチを用意し、CI用をCodSpeedに test: 手元用とCodSpeed用にそれぞれベンチマークを用意 Sep 11, 2025
@qryxip
Copy link
Member Author

qryxip commented Sep 11, 2025

@Hiroshiba CodSpeedを利用するにはCodSpeed HQというGitHub Appをインストールする必要があります。VOICEVOX/voicevox_coreを対象にrequestを飛ばしたので、(そちらからはどう見えるのかわかりませんが)ご検討頂けないでしょうか。

参考: https://codspeed.io/docs/integrations/providers/github#permissions

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 11, 2025

@qryxip すみません、先にGo/NoGoを決めたいです!
PR見てコード量把握しました。コード量はそんなに厳しくなさそうなので問題なさそう!

目的を言語化しておきたいです。
聞く理由は、メンテナ間の合意形成のためと、コミッターがゴールを想像できるようにと、レビューしやすいように、です。
目的はドキュメントに書いて、メンテナンス性を上げたいなと思ってます。

僕が決めると「TTSの速度に変化があったときにアナウンスできるようにするため」とかになっちゃって、.tts以外のパフォーマンス測定はあってもなくても良いとなってしまいそうです。
(「やってみたかったというのが理由の大半」とかでもOKです。それはそれで、例えば後々消す判断をしやすかったり。)

その目的からこの辺りを導き出したみ。
逆に言うとこのあたりの仕様(理想)を決められるような目的を作っておくと良さそう。

  • 何を測るのか(e2eのみ?各ラッパーも全部?非同期も?ソングも?文量は?)
  • 遅いときどうなるようにするか(テストfail?測るだけ?どこかに通知?)
  • いつ動くようにするのか(mainマージ時?push時?リリース時?)
  • 何をドキュメントに書くのか(どうやって測るのかとか、CodSpeedの見方とか、URLとか)

Copy link
Member

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コメントでも良さそう。

Copy link
Member Author

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"なんてものはどうせしばらく私かヒホさんしかやらないだろうから、少々変な手順になってもよいはず。

Copy link
Member

Choose a reason for hiding this comment

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

注入の経路は環境変数でJSONを流し込む

ま~とりあえずそれで良さそう感!
どこかにドキュメント置いときたいですね。

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@qryxip
Copy link
Member Author

qryxip commented Sep 12, 2025

TTS以外にも意図せぬパフォーマンス低下が無いかどうかをチェックしたい、というのも目的にしたいです。できればCIで動くテストと同列に考え、pull_requestトリガーでも動作するようにしたいです。利点としては、外部ライブラリのアップデート(例: #1147)やリファクタを安心して行えるようになる効果があると考えています。例えば私はCOREについて以下のようなことが気になっています。

  • Rustの同期APIとPythonの非同期APIの間には、blockingクレート、PyO3、Python本体、あとはvoicevox_core_python_apiのちょっとした機構が挟まっています。ちなみに我々は今PyO3のasyncio対応を利用していますが、これはexperimentalだそうです。
  • VVMをZIPとして読み込むところではasync_zipというライブラリを使っています。これは中でそこそこ複雑なことをしており、実際パフォーマンス関係のトラブルが過去にありました (#747 (comment))。

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 12, 2025

TTS以外にも意図せぬパフォーマンス低下が無いかどうかをチェックしたい

なるほどです!
僕が目標として書いたTTSの速度に変化があったときにアナウンスできるようにするためは内包されてそう。

であれば、主要なユーザー操作のe2eのパフォーマンス計測が重要で、あとは問題が起きそうなとこを計測って感じですかね!
前者の計測結果が遅かったら後者を動かして原因を探るのが自然だけど、計測がすぐ完了するならどっちもやっても良さそう。

となると、目的から導けるのはこんな感じですかね!

  • 何を測るのか
    • e2e
    • 各言語ラッパーe2e(主要なのだけでも良いかも)
    • 非同期も
    • ソングも(願わくば)
    • 問題が起きそうなとこ?
  • 遅いときどうなるようにするか
    • テストfailか、どこかに通知
  • いつ動くようにするのか
    • push時か、mainマージ時
  • 何をドキュメントに書くのか
    • 目的
    • どうやって測るのかとか、CodSpeedの見方とか?

Comment on lines 9 to 12
const MIDDLE_INPUT: InputText = InputText {
name: "MIDDLE",
value: "この音声は、ボイスボックスを使用して、出力されています。",
};
Copy link
Member

Choose a reason for hiding this comment

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

これ最初だけはpythonとの比較用に有用ですが、次回以降いらなくなりそう・・・!
遅くなったら消しても良いな、くらい。

Copy link
Member Author

Choose a reason for hiding this comment

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

一つ削るのはよさそうですね。今のSHORTを削ってMIDDLELONGの二つにするというのもよいかも。というのも"こんにちは""こんにちは"で短すぎるので、たくさん計測しないとブレが出そうかなと思っています。実際qryxip/voicevox_coreにてサンプル数100×10で試したときは何もしていないのに「+17%の速度改善」がされてしまいました。
(ただ、デフォルトのモードであるCPU Instrumentationだったらブレがもうちょっとマシにはなっていたとは思います)

image

Copy link
Member Author

Choose a reason for hiding this comment

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

SHORTを消し、MIDDLESHORTにリネームしました。

Copy link
Member Author

@qryxip qryxip Sep 13, 2025

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)

@Hiroshiba
Copy link
Member

@qryxip Github Appをインストールしました!
https://codspeed.io/VOICEVOX/voicevox_core で見れるはず?

@qryxip
Copy link
Member Author

qryxip commented Sep 13, 2025

86af1ff (#1153): 色々。

@qryxip qryxip marked this pull request as ready for review September 13, 2025 19:27
@qryxip qryxip requested a review from Hiroshiba September 13, 2025 19:27
@qryxip qryxip changed the title test: 手元用とCodSpeed用にそれぞれベンチマークを用意 test: 手元用兼CodSpeed用のベンチマークを用意する Sep 13, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 13, 2025

CodSpeed Performance Report

Congrats! CodSpeed is installed 🎉

🆕 16 new benchmarks were detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

Detected benchmarks

  • analyze_text[SHORT] (315.5 µs)
  • construct_open_jtalk (88 µs)
  • open_and_close_vvm (402.1 µs)
  • replace_mora_pitch[SHORT] (614.7 µs)
  • replace_phoneme_length[SHORT] (106.9 µs)
  • synthesis[SHORT] (2.3 s)
  • unload_and_load_vvm (1.8 s)
  • analyze_text[SHORT] (358.4 µs)
  • construct_open_jtalk (137.1 µs)
  • open_and_close_vvm (1.4 ms)
  • replace_mora_pitch[SHORT] (664.7 µs)
  • replace_phoneme_length[SHORT] (137.7 µs)
  • synthesis[SHORT] (2.3 s)
  • unload_and_load_vvm (1.9 s)
  • test_asyncio_tts (2.3 s)
  • test_blocking_tts (2.3 s)

Comment on lines +3 to +5
//! [`CONFIG`]の内容は環境変数`VOICEVOX_CORE_BENCH_CONFIG`から上書きすることができる。
//!
//! # 例
Copy link
Member Author

@qryxip qryxip Sep 13, 2025

Choose a reason for hiding this comment

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

📝 rustdocの特殊記法 ([`Rustのシンボル`]はRustのシンボルへのリンクになり、#<h2>になる)。

Comment on lines 24 to 29
- 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
Copy link
Member Author

@qryxip qryxip Sep 13, 2025

Choose a reason for hiding this comment

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

📝 このsetup-rustはCodSpeedのドキュメントで紹介されていたもの。今回使ってみて結構よさそうだったので、 #313 から使い続けてきてる機構をこれで置き換えてみたい。

@qryxip qryxip force-pushed the pr/test-add-benchmarks-and-use-codspeed branch from 026f64a to 03a159d Compare September 14, 2025 06:29
@qryxip
Copy link
Member Author

qryxip commented Sep 14, 2025

CodSpeedHQ/actionの最中にGHAをキャンセルするとこうなるっぽい?

[追記] あ、CodSpeedのメッセージが消えた!(これ↓)

image

@qryxip
Copy link
Member Author

qryxip commented Sep 14, 2025

以前話した、"16-core ARM64 CPU, 32 GB RAM"というスペックだけど月120分しか使えないというMacro Runnerですが先週の時点で月600分になったようです。しかも"open source project"であれば連絡すれば増加を検討してくれるようです。

More Free Macro Runners minutes

image

Starting today, every plan comes with 600 free macro runner minutes per month (previously 120) for the Walltime instrument. That's more room to run your benchmarks without worrying about hitting limits.

Open Source Boost

Working on an open source project? We'll happily grant even more minutes. Just send us a note with a few details about your repo.

More Free Macro Runners minutes - Changelog - CodSpeed

ベンチマークの時間が短縮されるであろう上にブレもおそらくかなり軽減されることを考えると、検討の余地があると思います。なおサードパーティのrunnerを使う関係上、organizationからの設定が必要なようです。
Usage on public repositories - Walltime Instrument Overview - CodSpeed Docs

[追記] 仮に月600分じゃ足りなかったとしても、そのときになってからでもGHAランナーに戻せるのでMacro Runnerからトライしてみるのは十分にありだと思います。

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 14, 2025

時間が足りなくなったら差がわからなくなってTTSの速度に変化があったときにアナウンスできるようにするためが満たせないので、足りなくなったら目標未達なのでダメと考えといて良さそうに思います。
1回3分くらいだとして、pushのたびだと厳しいかも(そうでもないかも)、mainマージ時だったら絶対大丈夫、位の感じですかね?

ブレもおそらくかなり軽減される

これ理由気になりました!差を知らないので、特にブレが軽減されそうな理由が思いつかなかったためです。
ちなみにGithub Actionsのブレは、目的を達成できないほどひどかったりしますかね?

@Hiroshiba Hiroshiba requested a review from Copilot September 14, 2025 09:23
Copy link
Contributor

Copilot AI left a 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.

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.

Rustコードのレビューは @sevenc-nanashi さん頼れるとありがたい!
けどざっと見た感じ別に大丈夫そう感!

Comment on lines +61 to +76
- 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
Copy link
Member

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/
Copy link
Member

Choose a reason for hiding this comment

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

ちなみにこの差の理由とかあったりしますか 👀

Copy link
Member Author

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/に限定しました。

Copy link
Member

@Hiroshiba Hiroshiba Sep 14, 2025

Choose a reason for hiding this comment

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

あ、なるほどです!!

「pytestを実行」という目的からならpytestをそのまま動かすほうが良さそう感。
「pytestでベンチマーク以外のテストを実行」ならこれで良さそう感。

「pytestでテストを実行」だけでも良いかも。
(ベンチマークもテストな気がするから違和感あるけど、まあ。。。諦められる範囲かも。)

@qryxip
Copy link
Member Author

qryxip commented Sep 14, 2025

時間が足りなくなったら差がわからなくなってTTSの速度に変化があったときにアナウンスできるようにするためが満たせない

アナウンスできそうなレベルの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のランナーは実行の安定性について一切保証してないので、高精度を謳ってくれるだけ期待できるかなと思った次第です。まあ一度試さないとよくわからないとは思います。

ちなみにGithub Actionsのブレは、目的を達成できないほどひどかったりしますかね?

"こんにちは"replace_durationをベンチしてたら、何もしていないのに「17%の速度改善」がされてしまった」ということは起きました。ちなみにこのときのサンプル回数は100×10回でした。
#1153 (comment)

"こんにちは"じゃなくしても、"regression"のしきいである10%ギリギリまでは結構ブレるという体感です。GHAでやるのなら、regression扱いを撤回させるためにベンチをre-runするということが今後たまにあるかもしれません。

@Hiroshiba
Copy link
Member

時間が足りなくなったら差がわからなくなってTTSの速度に変化があったときにアナウンスできるようにするためが満たせない

アナウンスできそうなレベルのTTS速度の変化が期待できそうなときはどの道Windows PCのGPUでやる(benchmarks.mdに書いた1.の方)というのを考えてました。

あ、目的の片方だけ引用しましたが、もう片方の目的も満たせないのではと思っています。
まあテスト回し直せば比較できるんだけど、それだったら最初からずっとGithub Runnerで良くね?って思いです。


ブレがでかいのは厄介ですね!
かつリモートでブレが少ないのであれば、試してみる価値はありそう。

とりあえず現時点で600min/月を超えそうかどうかだけ見積もってみるのはどうでしょうか。
1回辺り何分かかるか調べてるだけでOKだと思います。
それで何回動かせるか計算して、超えそうかなんとなく考えるくらいで良いのかなと。

ということで1回CodSpeedのリモート環境で動かしてみるのはどうでしょう?

@qryxip
Copy link
Member Author

qryxip commented Sep 14, 2025

ということで1回CodSpeedのリモート環境で動かしてみるのはどうでしょう?

そうですね。このPRをマージしなくてもこのPRの中で試せると思います。ただそのためには、organization側での設定が必要なようです(↓)。ということで"Organization Settings > Actions > Runner groups > Default"の"Repository Access"の設定をお願いできないでしょうか。

なおサードパーティのrunnerを使う関係上、organizationからの設定が必要なようです。
Usage on public repositories - Walltime Instrument Overview - CodSpeed Docs

@Hiroshiba
Copy link
Member

@qryxip
なるほどです!!
Defaultランナーグループの権限を変えるのちょっと危なそうに感じたので、CodSpeed Runner Groupを作ってみました!

ただこのグループにCodSpeedのRunnerを追加できてないので、このグループの設定が反映されない気もします。。

@Hiroshiba
Copy link
Member

なんかダメっぽかったのでCodSpeedランナーブループは消しました! 🙏

@qryxip
Copy link
Member Author

qryxip commented Sep 15, 2025

ダメっぽかったので、Walltime on GHAランナーに戻すことに…。

CodSpeed Macro Runnerを使うためのハードルは多分こんな感じ。このPRでは諦める。

  1. たまたま不調だったのかはわからないけど、滅茶苦茶遅かったのでそこの対策
  2. 月600分を超過して使うためのお願いをしに行く
  3. Runner Groupが現時点で未対応なので、対応されるまで待つ

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.

1つ確認が問題なさそうであればLGTM!!

これってベンチマークが10%とか差があったとき、テストfailする感じでしょうか? 👀
もしそうならしばらくはワーニングにしとくのもありかもとか思いました!
どれくらいブレるかわからないので。
(ChatGPTに聞いた感じ、テストが落ちるわけじゃない・・・? https://chatgpt.com/s/t_68c95ab4d33081919e350dc0b5bd2746

まあでもとりあえずテストは落ちるかもってことで様子見するのもダメじゃないと思います。
新規コミッターが遭遇すると驚くと思うので、早めにフォローしてあげたいなくらい。

qryxip added a commit to qryxip/voicevox_core that referenced this pull request Sep 16, 2025
@qryxip qryxip force-pushed the pr/test-add-benchmarks-and-use-codspeed branch from 1e2f893 to a33b2f0 Compare September 16, 2025 16:53
@qryxip
Copy link
Member Author

qryxip commented Sep 16, 2025

基本的にそのChatGPTの通りだとは思いますが、CodSpeed Performance AnalysisがこのPRで既に出現しているらしいことを考えると、branch protectionでrequire status checkしなくてもステータスを ❌ にしてくるような気がします。

フォローとしては、CONTRIBUTING.mdでの言及を入れました。どうでしょうか?
a33b2f0 (#1153)

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 16, 2025

❌になりえるなら案内欲しいですね!
ガイドラインに追加した内容で案内は十分そうに思いました!

Copy link
Member

@sevenc-nanashi sevenc-nanashi left a comment

Choose a reason for hiding this comment

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

特に問題なさそう。

@qryxip qryxip merged commit d46a5d0 into VOICEVOX:main Sep 18, 2025
38 checks passed
@qryxip
Copy link
Member Author

qryxip commented Sep 19, 2025

Valgrind抜きのGHAランナーだと、わかってはいましたが結構ブレが生じますね。特にopen_and_close_vvmunload_and_load_vvmがやばく、+22%とか-11%みたいなブレを普通に起こす。

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.

ベンチマークコード

3 participants