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

cache-build-resultの役割をドキュメント化する(もしくは改名) #63

Open
3 tasks done
qryxip opened this issue Dec 30, 2024 · 4 comments
Open
3 tasks done

Comments

@qryxip
Copy link
Member

qryxip commented Dec 30, 2024

内容

このcache-build-resultにコメントを書きます。

- name: Cache build result
id: cache-build-result
uses: actions/cache@v4
with:
path: build/
key: ${{ matrix.artifact_name }}-v${{ env.ONNXRUNTIME_VERSION }}-cache-${{ hashFiles('matrix.json') }}

cache-build-result${{ matrix.artifact_name }}-v${{ env.ONNXRUNTIME_VERSION }}-cache-${{ hashFiles('matrix.json') }}をキーとしてビルドディレクトリをキャッシュするもので、それ以上でもそれ以下でもありません。ワークフローファイルのうちmatrix以外の部分を書き換えてもキャッシュはそのまま使われます。

役割としては、lib(voicevox_)onnxruntimeが無事成功した後の整理部分について、試行錯誤をやりやすくするためのものだという理解をしています。ただこれを単に"cache"と呼ぶのは誤解を招くため、新しい名前を考えるかドキュメントを書くことを提案します。

Pros 良くなる点

Cons 悪くなる点

実現方法

VOICEVOXのバージョン

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

  • Windows
  • macOS
  • Linux

その他

@Hiroshiba
Copy link
Member

Hiroshiba commented Dec 30, 2024

なるほど、デバッグor開発用途!!
ビルドはとんでもない時間かかると思うので、便利だと思います!

もしbuild/ディレクトリの整理のためだけに使うのであれば、キャッシュの代わりにbuild/成果物をartifactに投げるのでも良いかもです。
artifactへのアップロードが重いのであれば、artifactへアップロードするフラグをworkflow_dispatchに生やす手もありかなと!

あとはキャッシュがデバッグ・開発用途なのであれば、キャッシュを使うフラグを生やす手もあるかもです。
デフォルトはオフにしておいて開発時にオンにして試す、みたいな。
workflow_dispatchはキャッシュ無視で、push時だけオンになる形もありかも。

今の形だとキャッシュ事故がたぶんいつか必ず起こるので(?)、なんとかしたほうが良いだろうなとは感じます。
改名で防げそうならそれもありだと思います!

@qryxip
Copy link
Member Author

qryxip commented Dec 30, 2024

cache-build-resultの一番の恩恵を忘れていました。例えばiOSとかの特定のターゲットだけ苦戦している最中において、Windows CUDA版(2時間かかる)のビルドをスキップすることができます。これによりXCFramework部分やリリースノート部分で試行錯誤が簡単にできますし、負荷の軽減にもなります。

正直純正ONNX Runtimeのビルドには便利なので、個人的には今の挙動が使い勝手がよいかなと思ってます。workflow_dispatch.inputsにフラグを用意するにしても、reuse_prev_build_resultdefault: true)とかがよいかなと。

@Hiroshiba
Copy link
Member

他のが不要なとき、自分はよくそれらのmatrixをコメントアウトしてます。
一回コミット入れないといけないですが、ビルドキャッシュ意識するのと比べると少し楽かなぁと感じます。
(それぐらい危ないものだと思っています)

たぶんこれはキャッシュによる致命的なバグを経験したことがあるかどうかが溝になっていて、議論を尽くしてもわりと平行線な気がします。
ヒヤリハットからバグを想像できるかも…?

例えばprepare.bashの実装で、このprepare.bashと相関する何かが自体がキャッシュのキーに入っておらず、prepare.bashを変えてもビルド結果が変わらないバグが発生しかけたと思います。
こんな感じでかなり気づきにくいのでかなり気を張らないといけず、基本気を張る方が大変という認識です。

まあでも、大丈夫だろうと感じるならその方針で進めてみるのも良いと思ってます。
僕がキャッシュを恐れすぎているだけかもしれないので!

@Hiroshiba
Copy link
Member

Hiroshiba commented Dec 30, 2024

にて、voicevox_onnxruntimeではキャッシュを使わない方針なことを把握しました!
であればまあ仮に問題が起きてもvoicevox_onnxruntimeは問題ないので、大丈夫かなと思いました!

issue内容のlib(voicevox_)onnxruntimeが無事成功した後の整理部分について(voicevox_)が対象外になる感じですかね。

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