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

ENH: 一部の関数をasync defに変更 #1134

Merged
merged 3 commits into from
Mar 27, 2024

Conversation

sabonerune
Copy link
Contributor

@sabonerune sabonerune commented Mar 21, 2024

内容

とりあえず単純にasync defに変更してもよさそうなところを変更します。

FastAPIのドキュメントには以下のようにあります。

await を使用して呼び出すべきサードパーティライブラリを使用している場合...async def を使用してpath operation 関数を宣言します。

データベース、API、ファイルシステムなどと通信し、await の使用をサポートしていないサードパーティライブラリ (現在のほとんどのデータベースライブラリに当てはまります) を使用している場合、次の様に、単に def を使用して通常通り path operation 関数 を宣言してください

アプリケーションが (どういうわけか) 他の何とも通信せず、応答を待つ必要がない場合は、async def を使用して下さい。

このうちの3番目に当たるものを変更します。

関連 Issue

その他

versionengine_manifestsupported_deviceが数百ms速くなっている感じはありますがdefの方が逆転することもあります。
元々短い処理であるので恐らく実感できるものではないと思います。

@sabonerune sabonerune requested a review from a team as a code owner March 21, 2024 14:58
@sabonerune sabonerune requested review from Hiroshiba and removed request for a team March 21, 2024 14:58
Copy link

github-actions bot commented Mar 21, 2024

Coverage Result

Resultを開く
Name Stmts Miss Cover
run.py 517 210 coverage-59%
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/cancellable_engine.py 94 72 coverage-23%
voicevox_engine/core/init.py 0 0 coverage-100%
voicevox_engine/core/core_adapter.py 81 6 coverage-93%
voicevox_engine/core/core_initializer.py 60 30 coverage-50%
voicevox_engine/core/core_wrapper.py 225 157 coverage-30%
voicevox_engine/dev/init.py 0 0 coverage-100%
voicevox_engine/dev/core/init.py 0 0 coverage-100%
voicevox_engine/dev/core/mock.py 65 2 coverage-97%
voicevox_engine/dev/tts_engine/init.py 0 0 coverage-100%
voicevox_engine/dev/tts_engine/mock.py 28 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifest.py 36 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifestLoader.py 12 0 coverage-100%
voicevox_engine/engine_manifest/init.py 0 0 coverage-100%
voicevox_engine/library_manager.py 92 4 coverage-96%
voicevox_engine/metas/Metas.py 36 0 coverage-100%
voicevox_engine/metas/MetasStore.py 28 1 coverage-96%
voicevox_engine/metas/init.py 0 0 coverage-100%
voicevox_engine/model.py 180 3 coverage-98%
voicevox_engine/morphing.py 71 4 coverage-94%
voicevox_engine/preset/Preset.py 13 0 coverage-100%
voicevox_engine/preset/PresetError.py 2 0 coverage-100%
voicevox_engine/preset/PresetManager.py 80 2 coverage-98%
voicevox_engine/preset/init.py 0 0 coverage-100%
voicevox_engine/setting/Setting.py 9 0 coverage-100%
voicevox_engine/setting/SettingLoader.py 20 0 coverage-100%
voicevox_engine/setting/init.py 0 0 coverage-100%
voicevox_engine/tts_pipeline/init.py 0 0 coverage-100%
voicevox_engine/tts_pipeline/kana_converter.py 88 1 coverage-99%
voicevox_engine/tts_pipeline/mora_mapping.py 7 0 coverage-100%
voicevox_engine/tts_pipeline/phoneme.py 34 0 coverage-100%
voicevox_engine/tts_pipeline/text_analyzer.py 146 6 coverage-96%
voicevox_engine/tts_pipeline/tts_engine.py 267 9 coverage-97%
voicevox_engine/user_dict/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/user_dict/user_dict.py 146 12 coverage-92%
voicevox_engine/utility/init.py 0 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 37 0 coverage-100%
voicevox_engine/utility/core_utility.py 6 0 coverage-100%
voicevox_engine/utility/core_version_utility.py 8 1 coverage-88%
voicevox_engine/utility/mutex_utility.py 13 0 coverage-100%
voicevox_engine/utility/path_utility.py 26 6 coverage-77%
voicevox_engine/utility/run_utility.py 10 7 coverage-30%
TOTAL 2443 533 coverage-78%

@tarepan
Copy link
Contributor

tarepan commented Mar 22, 2024

  • 👍️ async def 化を客観基準に則り進行しています。
  • 一部関数の async def 化について疑問点があります。

CDLL アクセスの async def

以下は Python バイトコードで完結し、他の何とも通信せず、応答を待つ必要がない に当たるものだと考えます:

  • check_disabled_mutable_api
  • version
  • core_versions
  • engine_manifest
  • validate_kana

一方で以下の関数は内部的に CDLL アクセスをしています:

  • speakers
  • singers
  • is_initialized_speaker
  • supported_devices

CDLL アクセスは次の特性を持っています:

  • 同期関数
  • 呼び出し前に GIL が開放される (ref)
  • 内部で同期 I/O しうる

このことから、CDLL アクセスはブロッキング I/O ライクに動く場合がありそうです。
となると 2 番目に該当するため、async def 化はプロファイリング含めた慎重な検討が必要そうに見えます。

@sabonerune
Copy link
Contributor Author

CDLLがGILをリリースすることを見落としていました。
戻しました。

Copy link
Contributor

@tarepan tarepan left a comment

Choose a reason for hiding this comment

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

LGTM👍️

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.

LGTM!

@Hiroshiba Hiroshiba merged commit f43881b into VOICEVOX:master Mar 27, 2024
4 checks passed
@sabonerune sabonerune deleted the enh/to-async branch March 27, 2024 10:26
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