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

コアの実装言語を C++ から Rust へ移行する #128

Closed
42 of 43 tasks
PickledChair opened this issue May 10, 2022 · 109 comments
Closed
42 of 43 tasks

コアの実装言語を C++ から Rust へ移行する #128

PickledChair opened this issue May 10, 2022 · 109 comments

Comments

@PickledChair
Copy link
Member

PickledChair commented May 10, 2022

内容

(以下の記述は必要に応じてアップデートしていきます。改善すべき点があればご指摘ください。)

現在、コアライブラリは C++ で記述されています。C++ は優れた言語ですが、ビルドツールなどを含め現代的でない部分が存在し、それにより開発の難しさを感じることがあります。ここで、開発言語として新たに Rust を採用することによって、開発体験が向上することが期待されるため、Rust による実装への移行を模索する意義があると考えられます。

この Issue では Rust による実装を段階的に進めるに当たって、実装の進め方や方針の議論、進捗の整理等を行うことができればと思います。

Pros 良くなる点

  • 基本的にメモリ安全・型安全になる
  • 非同期処理が書きやすくなる
    • C++20 と比較すると違いがないかも?
  • ビルド・パッケージ管理・テスト等が容易になる

Cons 悪くなる点

  • 学習コストの高い言語であることから、保守のための開発者を安定的に確保できるかどうか未知数な部分がある
    • Rust が言語的にフォーカスしている領域は C++ でも同様に難しい、ということはある
    • 今後の Rust 普及に伴って開発者増が期待できるかもしれない

その他、より詳しい Pros/Cons:
#128 (comment)

実現方法

作業は当面 rust branch で行われる予定です。

考えられる作業一覧:

また、並行して自動ビルド・テストも整備する:

最後に、新しいコアが期待通りに動くか確認して、移行作業完了としたいです(main branch へのマージ後に確認?)。

  • コアの動作確認(特に GPU を期待通りに使えるか)

その他

ビルド可能な最初の貢献は @qwerty2501 さんによって行われています (#126) ありがとうございます!

@qwerty2501
Copy link
Contributor

qwerty2501 commented May 10, 2022

RustでC APIを提供するライブラリを初めて書きましたが、現状pointerが多いのでそのまま実装していくとRustらしいコードがかけずRustのメリットである安全性が享受されにくくなりそうです。そのためC export用の関数定義とinternalとしてRustとしての関数定義を用意しました。将来的にはinternal側にある関数定義をリファクタリングで変えてRustっぽくかけるようにしたいなと考えてます。

個人的に考えてるPros/Consも書いてみます

Pros

  • 所有権に関する機能がコンパイラ側できちんとサポートされている
    • メモリに限らずリソース全般を安全に扱えるのは大きい
    • Copy操作は明示的にしないとできない/代入した場合デフォルトの動作がmove
  • 例外を採用していない
    • 好みがあると思いますが、例外は関数定義を見ただけでは何がとんでくるかわからないので関数型言語でよくあるEitherのようにResultを戻り値に定義することでエラーが返ってくるかどうかわかるのは大きい
  • 周辺ツールのクロスプラットフォーム性が高い

逆に非同期処理についてはC++とRustでそこまで書きやすさは変わらないと思います。C++20でコルーチンに関するコア言語機能が入ったはずなので。

Cons

  • C++で書かれたライブラリを使うことができなくなる可能性がある
  • 新しい言語なのでほしいライブラリが無い場合がある

学習コストについてですがC++も学習コストは高いのであまり変わらないと思います。
というよりはRustやC++でフォーカスしてる分野の学習コストが高いのかなと
開発者を安定的に確保できるかどうかですが、既存のC++開発者の多さを重く見るか、これから増えてくるRust開発者を重く見るかによるかと思います。
Rust移行に成功してしばらく運用してみて問題なさそうだったらそのことをブログにでも書いたらリポジトリ見てくれる人いるかもしれませんね

@nebocco
Copy link
Contributor

nebocco commented May 12, 2022

Rust ちょっとわかるので何かお手伝いできそうであればお声がけさせていただきます…!

@Hiroshiba
Copy link
Member

Hiroshiba commented May 15, 2022

ちょっとタスクを忘れないようにメモしておきます!

@qwerty2501
Copy link
Contributor

早めにonnxruntimeとリンクして動作させることができるか試したほうが良さそうですね。これができないとそもそもRust移行することができないので。
ひとまず cargo.io上で配られている Rustのonnxruntime wrapper が使えるか試す感じでしょうか?

@Hiroshiba
Copy link
Member

wrapperの活用が手軽で良さそうに感じました!
その際、いまのcoreみたく、メモリ上のバイナリからSessionを作れるのかが気になります。(暗号化の関係で必須なので…。)

@PickledChair
Copy link
Member Author

PickledChair commented May 15, 2022

その際、いまのcoreみたく、メモリ上のバイナリからSessionを作れるのかが気になります。(暗号化の関係で必須なので…。)

API を見る限り、SessionBuilderwith_model_from_memory メソッドで行けそうです👍

https://docs.rs/onnxruntime/latest/onnxruntime/session/struct.SessionBuilder.html#method.with_model_from_memory

@qwerty2501
Copy link
Contributor

それでは initializeと内部のStatus(?)の機能の一部機能実装からですかね?ちょっとやってみます

@qwerty2501
Copy link
Contributor

qwerty2501 commented May 15, 2022

やってみたんですが、Rustのonnxruntime wrapper だとuse_gpuが有効時に設定されたときにつかう ExecutionModeの設定や、 cudaに関する設定に使う関数がwrapperだと定義されてないようです。
対応策としてはcontribute送るか、自前でwrapper作るかだと思いますがどうしましょうか?

@qwerty2501
Copy link
Contributor

qwerty2501 commented May 15, 2022

それかRustあきらめるか

@Hiroshiba
Copy link
Member

お試しありがとうございます!!

やっぱりRustは魅力的なので、できれば前進したいのが個人的な本音です。
こちらでwrapperを足しつつ、将来的なことを考えて本家にPR送るか、難しければissue作成しておくとかが妥当なラインに感じました。

詳しくないのですが、cargoはgithubから気軽にパッケージ持ってくることはできそうでしょうか。
であれば、VOICEVOX orgにforkリポジトリ作っちゃおうかなと思います。
(ただ、とりあえず作ってみるにはちょっと作業量多そうなので、いったんqwertyさんがforkされたリポジトリ参照とかでも良いかも)

@PickledChair
Copy link
Member Author

PickledChair commented May 15, 2022

詳しくないのですが、cargoはgithubから気軽にパッケージ持ってくることはできそうでしょうか。

以下のような感じで Cargo.toml に書くとできます:

[dependencies]
clap = "2.27.1" # from crates.io
                # crates.ioから
rand = { git = "https://github.com/rust-lang-nursery/rand" } # from online repo
                                                             # オンラインのレポジトリから
bar = { path = "../bar" } # from a path in the local filesystem
                          # ローカルのファイルシステムのパスから

引用: https://doc.rust-jp.rs/rust-by-example-ja/cargo/deps.html

@qwerty2501
Copy link
Contributor

qwerty2501 commented May 15, 2022

@Hiroshiba @PickledChair とりあえずforkしたのですがそもそもonnxruntime-rsで参照してるonnxruntimeの対象バージョンが1.8と少し古いようなのでそこから直していったほうがいいかもです。最新版は1.11のようです
https://github.com/qwerty2501/onnxruntime-rs

@Hiroshiba
Copy link
Member

おお…結構古いですね…
追従が意外と大変、とかだと覚悟が要りそうですね…

そもそもrust用のラッパーを作らず、直接dll叩くみたいなことができたりするのでしょうか。

@qwerty2501
Copy link
Contributor

qwerty2501 commented May 15, 2022

RustにはCヘッダーからRustコードを生成する機能があります。onnxruntime-rsでも内部的にはその自動生成を使用してます。(onnxruntime-sysというcrateがそれです)。これがdllを直接叩くというのに該当するかと
しかし自動生成したコードはunsafeであり、使用する際はonnxruntime-rsのようにwrapするのが一般的です

@Hiroshiba
Copy link
Member

なるほどです。
voicevoxが使うonnxruntimeの機能は多くない(機械学習の訓練用のコードとかが一切入らない)ので、必要なとこだけラップして保守するのも悪くないのかもと感じました。

@qwerty2501
Copy link
Contributor

はい。わたしもちょうどそれを考えていました。
なのでこのリポジトリにonnxruntime bind用のcrateを作成して管理する方向でも良いかなと思いました。リポジトリ分けるかどうかはそちらの判断になりますが。
onnxruntime-rsにはbuild.rsで自動的にonnxruntimeをダウンロードしてくれるようなのでそのあたりは参考にしても良さそうです。(ライセンス的には問題なさそう)

@qwerty2501
Copy link
Contributor

qwerty2501 commented May 15, 2022

openjtalkのbind用crateも作る必要がありますがそれはおそらくこのリポジトリで管理されると思うので、それに合わせるなら個人的にはこのリポジトリでonnxruntime bind用crateを作って良いかなと思いました

@qwerty2501
Copy link
Contributor

qwerty2501 commented May 15, 2022

ちょっと試しにforkしたリポジトリでonnxruntimeのバージョン上げてみたのですがコンパイル通りました。
これに対して足りない分だけ追加で実装するほうが手間少なそうです。(変更したバージョンでちゃんと動かせることが前提ですが)
いったん仮で良いのでリポジトリforkしてやってみるか、このリポジトリでやってみるか決めていただけると助かります
意見が二転三転してしまい申し訳ありません

@Hiroshiba
Copy link
Member

たしかに一緒にcreat管理しても良いなと感じました。
forkするかbind書くかは一長一短感あるので、初手どちらでも良いかなと思います。
forkでコンパイル動いたとのことなので、こちらの方が進みやすそうかなと感じました!

@qwerty2501
Copy link
Contributor

qwerty2501 commented May 17, 2022

onnxruntime導入PR作りました
wrapper書くの結構大変だったのでforkでつくるのがまあ無難かなと感じました。1からつくるのは結構骨が折れそうです。
forkしたonnxruntime-rsですが、macosのarm64版に対応してないようなので将来的にそこを直してやる必要がありそうです

@PickledChair
Copy link
Member Author

forkしたonnxruntime-rsですが、macosのarm64版に対応してないようなので将来的にそこを直してやる必要がありそうです

調べたところ、「macOS の arm64 版には対応しているものの、onnxruntime を自動でダウンロードする処理がまだない」という感じに思えました。環境変数として ORT_STRATEGY=systemORT_LIB_LOCATION=/path/to/onnxruntime を設定するとビルドできそうに見えますが、いかがでしょうか……? (参考: nbigaouette/onnxruntime-rs#74) あるいは「macOS の arm64 向けに onnxruntime を自動でダウンロードする設定がまだない(これが onnxruntime-rs のビルドの初期設定なので、すんなりビルドできない)」という意味でおっしゃっていた場合はすみません……!

また、関連する話題なのですが、現在の voicevox_core の C++ 実装では macOS 向けに universal binary の dylib を提供していますが、現在 Cargo には universal binary をビルドするための仕組みが存在していないようなので、もし今後も universal binary を提供する場合、macOS の lipo コマンドで手動で作る必要がありそうです (参考: rust-lang/cargo#8875) 。あるいは、universal binary の提供を止めて Intel CPU と Arm64 CPU 向けに別個にコアライブラリを提供するように変更する、ということも考えられます。

@qwerty2501
Copy link
Contributor

@PickledChair 自動ダウンロードの話です。ORT_STRATEGY=systemの使用については特に考えてませんでした。

@PickledChair
Copy link
Member Author

@qwerty2501

自動ダウンロードの話です。ORT_STRATEGY=systemの使用については特に考えてませんでした。

理解しました! 実際、ビルド時に自動ダウンロードしてくれた方が便利なので、後々 M1 Mac 向けに対応を加えたいところではありますね。

@nebocco
Copy link
Contributor

nebocco commented May 25, 2022

現在 onnxruntime に関する作業を進めているかと思うのですが、落ち着いたらタスクを整理して複数 issue に分割(あるいはプロジェクトとしてまとめる)していただけるとありがたいです。

@PickledChair
Copy link
Member Author

@nebocco

タスクを整理して複数 issue に分割(あるいはプロジェクトとしてまとめる)していただけるとありがたいです。

ご意見ありがとうございます。この issue を立てた当初からそのつもりでいたのですが、時間が取れずそのままになってしまっていました。週末に今後の工程を考えてみて、ここで提案したいと思っています。その結果に沿って issue 分割等を行えればと思います。

qwerty2501 added a commit to qwerty2501/voicevox_core that referenced this issue Jul 23, 2022
RPATHがされていないとdynamiclink時にLD_LIBRARY_PATHの設定を行わないとlink errorになってしまうため

refs VOICEVOX#128
qwerty2501 added a commit to qwerty2501/voicevox_core that referenced this issue Jul 23, 2022
現在Cargo.lockファイルはgit管理対象外になっているが、cargoにはCargo.lockファイルをgit管理対象に入れるべき場合とそうでない場合がある。
Cargo.lockファイルをgit管理対象に入れないべき場合はcargo.ioなどでソースファイルごとライブラリとして公開し、ユーザー側でビルドする場合
git管理対象に入れる場合は配布形態がバイナリ等ソースコードに依存しない形で配布する場合
本リポジトリはcdylib形式でdllバイナリとして配布することを目的としているのでgit管理対象に入れるべきケースになる

詳しくは公式ドキュメントを参照のこと
https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html

refs VOICEVOX#128
qwerty2501 added a commit to qwerty2501/voicevox_core that referenced this issue Jul 23, 2022
* internalを構造体化して薄いラッパーにした

testコードを書きやすくするため、internalのコードを一部を除いて構造体のメンバ関数とした

refs VOICEVOX#128 (comment)

* lint errorを修正した
qwerty2501 added a commit to qwerty2501/voicevox_core that referenced this issue Jul 23, 2022
* OpenJtalkを実装した

refs VOICEVOX#128

* open_jtalk-rs更新

* Update crates/voicevox_core/src/engine/open_jtalk.rs

mecab2njdが抜けていたので追加

Co-authored-by: Gray Suitcase <[email protected]>

Co-authored-by: Gray Suitcase <[email protected]>
qwerty2501 added a commit to qwerty2501/voicevox_core that referenced this issue Jul 23, 2022
unsafeなコードを原則禁止にした

unsafe_codeをdenyにすることにより、うっかりunsafeコードを書くのを防ぐことが目的
例外としてc関数向けの実装である c_export moduleと、static領域に配置する必要があるStatus structについてはunsafeを使うことを許可している

refs VOICEVOX#128
qwerty2501 added a commit to qwerty2501/voicevox_core that referenced this issue Jul 23, 2022
linux,macosのshared libraryをバージョンつきのものを配置するように変更

サンプル実装より、現在配置してるshared
libraryはバージョンなしのため実行に失敗するためバージョンつきのものに変更する必要がある
refs VOICEVOX#128 VOICEVOX#197 (comment)
This was referenced Jul 23, 2022
@qwerty2501
Copy link
Contributor

pythonのexample書くにはmainとのconflictを解消してからにしたほうがよさそう

@qwerty2501
Copy link
Contributor

現状 https://github.com/VOICEVOX/voicevox_core/blob/main/core/_core.py は消えてる状態になってるんですが、これってengineからcore使う際に必要なんですかね?そうでないならexample内に実装を寄せてしまおうかと思うのですが

@qwerty2501
Copy link
Contributor

qwerty2501 commented Jul 24, 2022

あとWindowsのexampleは私の方ではやれなさそうです(環境無いので)

@PickledChair
Copy link
Member Author

PickledChair commented Jul 25, 2022

現状 https://github.com/VOICEVOX/voicevox_core/blob/main/core/_core.py は消えてる状態になってるんですが、これってengineからcore使う際に必要なんですかね?そうでないならexample内に実装を寄せてしまおうかと思うのですが

Python パッケージ化をどうするかによるのかもしれません。前の Python example では example を動作させる前にコアを setup.py で Python のパッケージとしてインストールする過程がありました。core/_core.py はそのパッケージの本体部分であり、core/__init__.py から import されている感じです。なので、Python パッケージ化する場合、どのようにファイルやディレクトリを用意するかは setup.py の仕様に合わせる必要がありそうです。

一方、Python example を現在のエンジンと同様に ctypes でコアをロードする形に書き換える場合は、コアの Python パッケージ化は不要になります。その場合は core/_core.pycore/__init__.py は不要になると思います。Python パッケージ化している目的がそれほど大きくなければ、この形にするのもありかもしれないと思いました。

あとWindowsのexampleは私の方ではやれなさそうです(環境無いので)

Windows 環境は一応あるのですが、普段 Visual Studio を全く触らないので、自分がやるとしたら調べながらの作業になり、結構時間がかかってしまいそうです……。元の Windows example を書いたのは @shigobu さんだったと思うのですが、お忙しくなければ新しいコアへの追従をお願いしたい感じがあります(それが一番早そうです)。もし @shigobu さんを含め他の方の都合がつかなければ、私がやることを検討したいと思います。

@Hiroshiba
Copy link
Member

Hiroshiba commented Jul 25, 2022

_core.pyを消すと変わる部分をまとめるとたぶんこの2つです。

前者は新しく生まれ変わるpython exampleで代替できそうです。逆にpip周りの知識がいらなくなって利便性上がりそう。
後者は対応すれば問題無さそうです。なので、消しちゃダメではななさそうに思いました。

パッケージとして配布するかに関しては・・・いったんこのブランチでは考えない方針にし、パッケージを配付するかを考えるissueを作る方針でどうでしょう。

@shigobu
Copy link
Contributor

shigobu commented Jul 26, 2022

Windows exampl修正します。

@PickledChair
Copy link
Member Author

PickledChair commented Jul 26, 2022

@shigobu ありがとうございます、本当に助かります……!(急にメンションを飛ばしてすみませんでした🙇)

以前からの変更点としては、

のような感じだと思います(他の変更点が漏れていたらすみません)。もし他に必要な情報等ありましたら PR 等で気軽にご質問ください……!

@qwerty2501
Copy link
Contributor

coreのheader fileもzip同梱のものを使う必要があります

Hiroshiba added a commit that referenced this issue Jul 27, 2022
* pythonのexampleを新しい形に変更した

refs #128

* typo修正

Co-authored-by: Hiroshiba <[email protected]>

* 不要な手順を削除した

Co-authored-by: Hiroshiba <[email protected]>
@Hiroshiba
Copy link
Member

製品版の方でもビルドできました!!(動作確認はまだですが。。)
いくつか修正したいポイントがあったので記念に(?)PR出します。

@Hiroshiba
Copy link
Member

お疲れ様でした!!!!!!!!!!!!!!!
続きは #213 でできればと思います!!!

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

5 participants