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

同期版APIを作る? #662

Closed
Hiroshiba opened this issue Oct 27, 2023 · 6 comments · Fixed by #705
Closed

同期版APIを作る? #662

Hiroshiba opened this issue Oct 27, 2023 · 6 comments · Fixed by #705
Labels
機能向上 要議論 実行する前に議論が必要そうなもの

Comments

@Hiroshiba
Copy link
Member

内容

Python版のAPIを眺めていたのですが、普段駆け出しのPythonユーザーがおそらく1回も見たことがないであろうasyncが必須になっていることに気づきました。
asyncはPythonのどの標準ライブラリにもおそらく現れておらず(?)、しばらくプログラミングをしていた方ですら扱い方を知らない人の方がおそらく多いくらい難しいと思います。
なんとなくの直感ですが、VOICEVOX COREを手に取ってくれたプログラマーの3割は脱落しそうかなと・・・。

可能であれば同期版APIを提供してあげるとユーザ数は飛躍的に伸びるだろうなと思いました。
どういう方法があるか、やるやらも含めて議論できるといいのかなと思い、issueを立ててみました。

Pros 良くなる点

初学者に優しくなる

Cons 悪くなる点

下手したら関数の量が倍になる

実現方法

Rust APIで同期版を作り、非同期が難しそうな言語では同期版を提供する。
(非同期が一般的な言語ではasync版だけでもいいかも・・・?)

VOICEVOXのバージョン

0.15より後?

その他

0.15では非同期版だけの提供でいいのかなと思いました!
できる限り優しいexampleにしてあげたいですね・・・!

@sevenc-nanashi
Copy link
Member

Rust APIで同期版を作り、非同期が難しそうな言語では同期版を提供する。

Rust API側(voicevox_coreグレート)は変える必要はないと思います。
それぞれの言語バインディングでRUNTIME.block_on(C APIのように)すればシグネチャの変更だけで済むと思います。

@Hiroshiba
Copy link
Member Author

Rust API側(voicevox_coreグレート)は変える必要はないと思います。 それぞれの言語バインディングでRUNTIME.block_on(C APIのように)すればシグネチャの変更だけで済むと思います。

その手もあるかなと思います。
最終的にRust APIでも同期版を提供する方針であれば、それを作って各言語でラップする形が良いのかなーと。

@qryxip
Copy link
Member

qryxip commented Oct 29, 2023

そのRUNTIME.block_onで包むのをRust APIで一括でやってしまえば、APIとしての見通しと統一性が向上するんじゃないかと思っています。

RUNTIME.block_onをFFI側から減らせればたぶんすっきりするし、混乱も減らせるんじゃないかと思います。
例えば今私が見つけたPython APIのここ、 #555 の後に #553 を入れたときに考慮が漏れてましたが、このtokio::sync::Mutexおよびそれに伴うRUNTIME.block_onはもう要らないはず。こういうのも減らせるかと。

synthesizer: Closable<Arc<Mutex<voicevox_core::Synthesizer>>, Self>,

@qryxip
Copy link
Member

qryxip commented Nov 1, 2023

手元でasync defの外でpyo3-asyncioの関数を呼んでみたのですが、その際起きることとしてはコルーチンが返されて後でTypeErrorになるのではなく、その場でRuntimeErrorを吐くようです。

async defの挙動としては怪しくなっている気もしてきますが、まあ好都合ではあるかもしれません。

>>> synthesizer = asyncio.run(Synthesizer.new(openjtalk))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: no running event loop

(追記) まあRuntimeError: no running event loopが発生しますみたいなタイトルのissueが一回くらいここに立つことにはなると思いますが。

@Hiroshiba
Copy link
Member Author

onnxruntimeがどうなってるのか、SessionのAPIを見てみました。

  • 同期的にコンストラクタで作る
  • 同期的にglobalな関数で作る
    • C
    • Java(envを作ってenv.createSession)
  • 非同期的にstaticメソッドで作る

onnxruntimeは全言語で全然違うインターフェースを持っていそうな雰囲気を受けました。別々な設計方針で開発してる・・・?(流石にそれはめちゃめちゃ大変そう。。。)

@qryxip
Copy link
Member

qryxip commented Mar 17, 2024

#702, #705, #706 により達成。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
機能向上 要議論 実行する前に議論が必要そうなもの
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants