共有ライブラリのロードでアーキテクチャを考慮する機能を追加 #327

HyodaKazuaki · 2022-02-16T09:24:44Z

内容

共有ライブラリのロードの際に、アーキテクチャを考慮することができるように変更しました。

その他

Pythonのplatformライブラリを利用しています。
WindowsとLinuxで判定を行います。
Windows arm向けのコアファイルcore_cpu_arm.dllとLinux arm向けのコアファイルlibcore_cpu_armhf.soで命名ルールが異なっているようです。

coveralls · 2022-02-16T09:26:06Z

Pull Request Test Coverage Report for Build 1852237076

1 of 30 (3.33%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-1.3%) to 76.375%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
voicevox_engine/synthesis_engine/core_wrapper.py	1	30	3.33%

Totals
Change from base Build 1847262255:	-1.3%
Covered Lines:	708
Relevant Lines:	927

💛 - Coveralls

github-actions · 2022-02-16T09:27:18Z

Coverage Result

Resultを開く

Name	Stmts	Miss
voicevox_engine/init.py	1	0
voicevox_engine/acoustic_feature_extractor.py	75	0
voicevox_engine/dev/synthesis_engine/init.py	2	0
voicevox_engine/dev/synthesis_engine/mock.py	36	2
voicevox_engine/full_context_label.py	171	6
voicevox_engine/kana_parser.py	86	1
voicevox_engine/model.py	75	7
voicevox_engine/mora_list.py	4	0
voicevox_engine/preset/Preset.py	12	0
voicevox_engine/preset/PresetLoader.py	34	1
voicevox_engine/preset/init.py	3	0
voicevox_engine/synthesis_engine/init.py	5	0
voicevox_engine/synthesis_engine/core_wrapper.py	164	146
voicevox_engine/synthesis_engine/make_synthesis_engines.py	52	44
voicevox_engine/synthesis_engine/synthesis_engine.py	108	2
voicevox_engine/synthesis_engine/synthesis_engine_base.py	62	7
voicevox_engine/utility/init.py	2	0
voicevox_engine/utility/connect_base64_waves.py	35	3
TOTAL	927	219

voicevox_engine/synthesis_engine/core_wrapper.py

Hiroshiba · 2022-02-16T09:54:03Z

@takana-v さん、 @PickledChair さん、もしよければレビューよろしくお願いします･･･！

takana-v

LGTM！

PickledChair

少し気になったところがあったのでコメントしました。ご確認いただければ幸いです。

また、動作自体は問題なさそうですが、if 分岐が多くなりメンテナンスが大変そうなコードになってきている気がしています（特に load_core 関数が肥大化してきています）。共通の処理を繰り返し書いているように見える箇所（x86_64 や aarch64 などのアーキテクチャの同じような判定が Windows と Linux の双方にある）もあるので、まとめられるところはまとめてしまった方が良い気もしました。

ただし、そのためにはコアの名前・OS・アーキテクチャ・CPU/GPUといった情報を一つのデータ構造にまとめる必要がありそうです。試しにコードに起こしてみたものを以下に示します（うまく動作するかどうかはテストしきれていません……）。この PR 内でこのような形にしてしまうかどうかについては、 @.HyodaKazuaki さんや他のレビュワーの方々のご意見をお聞きしたいと思います（この PR では既存のコードの拡張のみにすることになった場合、多分、後で私が改めて PR します）。

コアの情報をデータ構造にまとめたコードの例

import platform
from ctypes import CDLL
from pathlib import Path
from typing import Optional


CORE_INFOS = [
    # Windows
    { "name": "core.dll", "platform": "Windows", "is_gpu_core": True, "arch": "x86_64", "core_type": "libtorch" },
    { "name": "core_cpu.dll", "platform": "Windows", "is_gpu_core": False, "arch": "x86_64", "core_type": "libtorch" },
    { "name": "core_gpu_x64_nvidia.dll", "platform": "Windows", "is_gpu_core": True, "arch": "x86_64", "core_type": "onnxruntime" },
    { "name": "core_cpu_x64.dll", "platform": "Windows", "is_gpu_core": False, "arch": "x86_64", "core_type": "onnxruntime" },
    { "name": "core_cpu_x86.dll", "platform": "Windows", "is_gpu_core": False, "arch": "x86", "core_type": "onnxruntime" },
    { "name": "core_cpu_arm.dll", "platform": "Windows", "is_gpu_core": False, "arch": "armv7l", "core_type": "onnxruntime" },
    { "name": "core_cpu_arm64.dll", "platform": "Windows", "is_gpu_core": False, "arch": "aarch64", "core_type": "onnxruntime" },

    # Linux
    { "name": "libcore.so", "platform": "Linux", "is_gpu_core": True, "arch": "x86_64", "core_type": "libtorch" },
    { "name": "libcore_cpu.so", "platform": "Linux", "is_gpu_core": False, "arch": "x86_64", "core_type": "libtorch" },
    { "name": "libcore_gpu_x64_nvidia.so", "platform": "Linux", "is_gpu_core": True, "arch": "x86_64", "core_type": "onnxruntime" },
    { "name": "libcore_cpu_x64.so", "platform": "Linux", "is_gpu_core": False, "arch": "x86_64", "core_type": "onnxruntime" },
    { "name": "libcore_cpu_armhf.so", "platform": "Linux", "is_gpu_core": False, "arch": "armv7l", "core_type": "onnxruntime" },
    { "name": "libcore_cpu_arm64.so", "platform": "Linux", "is_gpu_core": False, "arch": "aarch64", "core_type": "onnxruntime" },

    # macOS
    { "name": "libcore_cpu_universal2.dylib", "platform": "Darwin", "is_gpu_core": False, "arch": "universal", "core_type": "onnxruntime" },
]


def get_arch_name() -> Optional[str]:
    """
    platform.machine() が特定のアーキテクチャ上で複数パターンの文字列を返し得るので、
    一意な文字列に変換する
    サポート外のアーキテクチャである場合、None を返す
    """
    machine = platform.machine()
    if machine == "x86_64" or machine == "x64" or machine == "AMD64":
        return "x86_64"
    elif machine == "i386" or machine == "x86":
        return "x86"
    elif machine in ["armv7l", "aarch64"]:
        return machine
    else:
        return None


def get_core_name(arch_name: str, platform_name: str, model_type: str, is_gpu_core: bool) -> Optional[str]:
    if platform_name == "Darwin":
        if (not is_gpu_core) and (arch_name == "x86_64" or arch_name == "aarch64"):
            arch_name = "universal"
        else:
            return None
    for core_info in CORE_INFOS:
        if (
            core_info["platform"] == platform_name
            and core_info["arch"] == arch_name
            and core_info["core_type"] == model_type
            and core_info["is_gpu_core"] == is_gpu_core
        ):
            return core_info["name"]
    return None


def get_suitable_core_name(model_type: str, is_gpu_core: bool) -> Optional[str]:
    arch_name = get_arch_name()
    if arch_name is None:
        return None
    platform_name = platform.system()
    return get_core_name(arch_name, platform_name, model_type, is_gpu_core)


def check_core_type(core_dir: Path) -> Optional[str]:
    libtorch_core_names = [
        get_suitable_core_name("libtorch", is_gpu_core=True),
        get_suitable_core_name("libtorch", is_gpu_core=False),
    ]
    onnxruntime_core_names = [
        get_suitable_core_name("onnxruntime", is_gpu_core=True),
        get_suitable_core_name("onnxruntime", is_gpu_core=False),
    ]
    if any([(core_dir / name).is_file() for name in libtorch_core_names if name]):
        return "libtorch"
    elif any([(core_dir / name).is_file() for name in onnxruntime_core_names if name]):
        return "onnxruntime"
    else:
        return None


def load_core(core_dir: Path, use_gpu: bool) -> CDLL:
    model_type = check_core_type(core_dir)
    if model_type is None:
        raise RuntimeError("コアが見つかりません")
    if use_gpu or model_type == "onnxruntime":
        core_name = get_suitable_core_name(model_type, is_gpu_core=True)
        if core_name:
            try:
                return CDLL(str((core_dir / core_name).resolve(strict=True)))
            except OSError:
                pass
    core_name = get_suitable_core_name(model_type, is_gpu_core=False)
    if core_name:
        try:
            return CDLL(str((core_dir / core_name).resolve(strict=True)))
        except OSError:
            if model_type == "libtorch":
                core_name = get_suitable_core_name(model_type, is_gpu_core=True)
                if core_name:
                    return CDLL(str((core_dir / core_name).resolve(strict=True)))
    else:
        raise RuntimeError(f"このコンピュータのアーキテクチャ {platform.machine()} で利用可能なコアがありません")
    raise RuntimeError("コアの読み込みに失敗しました")


if __name__ == "__main__":
    load_core(Path.cwd(), use_gpu=False)
    print("use cpu")
    load_core(Path.cwd(), use_gpu=True)
    print("use gpu")

voicevox_engine/synthesis_engine/core_wrapper.py

HyodaKazuaki · 2022-02-17T09:04:52Z

ただし、そのためにはコアの名前・OS・アーキテクチャ・CPU/GPUといった情報を一つのデータ構造にまとめる必要がありそうです。試しにコードに起こしてみたものを以下に示します（うまく動作するかどうかはテストしきれていません……）。この PR 内でこのような形にしてしまうかどうかについては、 @.HyodaKazuaki さんや他のレビュワーの方々のご意見をお聞きしたいと思います（この PR では既存のコードの拡張のみにすることになった場合、多分、後で私が改めて PR します）。

コード拝見しました。
おっしゃるとおり、if文の複雑さが増してきていたところでしたので、とても良い解決方法だと思います。
また、特にエラーとなりそうな箇所はありませんでした。
私としては@PickledChair に折角ここまで書き提案していただいたので、貢献者(コミッター)として名前を残していただいたほうが良いかと思います。
ですので、suggestという形でこのPRのrviewを入れてもらうか、別のPRとしてマージしていただくという形を取れればと思いますが、いかがでしょうか。
他の方の意見もお待ちしております。

PickledChair

@HyodaKazuaki ご意見（とご配慮）ありがとうございます。今回の PR のついでにリファクタリングが行えるかもしれないと思い提案しましたが、自分の案だと変更量が多いため、別 PR とした方が良さそうだと思えてきました。他にご意見がなければこの PR はこのままマージしようと思います！

PickledChair · 2022-02-18T11:29:41Z

マージします。PR ありがとうございました！

HyodaKazuaki added 2 commits February 14, 2022 16:10

change os detection method

94a6184

add machine type check

3e59989

Hiroshiba requested review from PickledChair and takana-v February 16, 2022 09:53

takana-v requested changes Feb 16, 2022

View reviewed changes

voicevox_engine/synthesis_engine/core_wrapper.py Outdated Show resolved Hide resolved

add new cores to check_core_type

6bbbe9c

takana-v approved these changes Feb 16, 2022

View reviewed changes

PickledChair requested changes Feb 16, 2022

View reviewed changes

voicevox_engine/synthesis_engine/core_wrapper.py Outdated Show resolved Hide resolved

change error message when model_type is None

d942fb1

PickledChair approved these changes Feb 17, 2022

View reviewed changes

PickledChair merged commit 2cdd49c into VOICEVOX:master Feb 18, 2022

HyodaKazuaki deleted the feature/add-machine-type-checking branch February 18, 2022 11:35

PickledChair mentioned this pull request Feb 20, 2022

Refactor: コアのロードまわりを整理 #333

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

共有ライブラリのロードでアーキテクチャを考慮する機能を追加 #327

共有ライブラリのロードでアーキテクチャを考慮する機能を追加 #327

HyodaKazuaki commented Feb 16, 2022

coveralls commented Feb 16, 2022 •

edited

Loading

github-actions bot commented Feb 16, 2022 •

edited

Loading

Hiroshiba commented Feb 16, 2022

takana-v left a comment

PickledChair left a comment •

edited

Loading

HyodaKazuaki commented Feb 17, 2022

PickledChair left a comment •

edited

Loading

PickledChair commented Feb 18, 2022

共有ライブラリのロードでアーキテクチャを考慮する機能を追加 #327

共有ライブラリのロードでアーキテクチャを考慮する機能を追加 #327

Conversation

HyodaKazuaki commented Feb 16, 2022

内容

関連 Issue

その他

coveralls commented Feb 16, 2022 • edited Loading

Pull Request Test Coverage Report for Build 1852237076

💛 - Coveralls

github-actions bot commented Feb 16, 2022 • edited Loading

Coverage Result

Hiroshiba commented Feb 16, 2022

takana-v left a comment

Choose a reason for hiding this comment

PickledChair left a comment • edited Loading

Choose a reason for hiding this comment

HyodaKazuaki commented Feb 17, 2022

PickledChair left a comment • edited Loading

Choose a reason for hiding this comment

PickledChair commented Feb 18, 2022

coveralls commented Feb 16, 2022 •

edited

Loading

github-actions bot commented Feb 16, 2022 •

edited

Loading

PickledChair left a comment •

edited

Loading

PickledChair left a comment •

edited

Loading