-
Notifications
You must be signed in to change notification settings - Fork 201
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
整理: Character
内部型を追加
#1320
整理: Character
内部型を追加
#1320
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
talker/singerという言葉は現れない方がわかりやすい気がしました!
とりあえずコメントまで 🙇
@Hiroshiba |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
高速化嬉しいです!
そもそもdeepcopyを取ってたのは流石に身長になりすぎだったかもと感じました!
内容
概要:
Character
内部型を追加してリファクタリングENGINE は内部で
Speaker
インスタンスをトリッキーに扱っており(例: deepcopy、placeholder 代入)、これが性能低下・見通し悪化の原因になっている。その原因は API BaseModel である Speaker を内部に流用した結果それに縛られていることにある(c.f. #1249)。
ゆえに ENGINE 内部に最適化した型をもてばこれを解決できる。
このような背景から、
Character
内部型を追加するリファクタリングを提案します。本 PR では第一段階として
filter_speakers_and_styles()
関数内部でのみ利用する。この活用だけでも deepcopy を削除でき、性能を向上できる(speedtest で
GET /speaker_info
20% 高速化を確認済み)。今後はキャストを API 側へ移し、内部での
Character
利用範囲を段階的に広げていく方針である。関連 Issue
part of #1313