mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-11 05:53:23 -05:00
perf: read yaml through the c loader
Loading the emulator profiles is the most expensive step of every command here, and all forty call sites used the pure-Python scanner while libyaml sat unused in the same wheel. One shared yaml_load picks the C loader when pyyaml ships it: the 375 profiles parse in 0.18s instead of 1.39s, and verify --platform retroarch drops from 2.17s to 0.73s. The loader class is the same restricted one safe_load uses. es_bios.xml was parsed straight from the network while install.py already refused a document declaring entities; both now share one guard. Scrapers reach it through a single path bootstrap in the package rather than two ad-hoc ones.
This commit is contained in:
1 parent
ab6a3bb26d
commit
3b8f2d75d5
18 files changed
+109
-42
No files matched your search
@@ -21,6 +21,7 @@ import urllib.error
|
||||
import urllib.parse
|
||||
import urllib.request
|
||||
from pathlib import Path
|
||||
from common import yaml_load
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -55,7 +56,7 @@ def load_tracked_entries(
|
||||
print("Error: PyYAML required", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
with open(registry_path) as f:
|
||||
data = yaml.safe_load(f) or {}
|
||||
data = yaml_load(f) or {}
|
||||
entries: dict[str, tuple[str, str]] = {}
|
||||
for key, entry in data.get("data_directories", {}).items():
|
||||
url = entry.get("source_url", "")
|
||||
|
||||
+56
-12
@@ -16,10 +16,30 @@ import tempfile
|
||||
import urllib.error
|
||||
import urllib.parse
|
||||
import urllib.request
|
||||
import xml.etree.ElementTree as ET
|
||||
import zipfile
|
||||
import zlib
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def parse_untrusted_xml(content: str | bytes, label: str = "XML") -> ET.Element:
|
||||
"""Parse XML fetched from a third party.
|
||||
|
||||
ElementTree expands internal entities, so a document that declares them
|
||||
can make the parser build a payload far larger than the bytes downloaded.
|
||||
Nothing this project reads (DAT packs, es_bios.xml, Emulators.xml) ever
|
||||
declares one, so a declaration is grounds to refuse the document rather
|
||||
than something to expand carefully.
|
||||
|
||||
The check targets <!ENTITY rather than <!DOCTYPE because Logiqx DATs
|
||||
legitimately carry a doctype pointing at logiqx.com, and ElementTree does
|
||||
not resolve external entities, so a doctype alone fetches nothing.
|
||||
"""
|
||||
text = content if isinstance(content, str) else content.decode("utf-8", "replace")
|
||||
if "<!ENTITY" in text.upper():
|
||||
raise ValueError(f"XML entity declarations are not allowed in {label}")
|
||||
return ET.fromstring(content)
|
||||
|
||||
try:
|
||||
import yaml
|
||||
except ImportError:
|
||||
@@ -39,6 +59,30 @@ def require_yaml():
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
def _pick_yaml_loader():
|
||||
"""Prefer the libyaml loader when the wheel ships it.
|
||||
|
||||
Reading the emulator profiles is the single most expensive step of every
|
||||
command here: 375 files, and the pure-Python scanner accounts for roughly
|
||||
70% of a verify run. The C loader parses the same documents about eight
|
||||
times faster and pyyaml only exposes it when libyaml was available at
|
||||
build time, so the pure-Python class stays as the fallback.
|
||||
"""
|
||||
if yaml is None:
|
||||
return None
|
||||
return getattr(yaml, "CSafeLoader", None) or yaml.SafeLoader
|
||||
|
||||
|
||||
_YAML_LOADER = _pick_yaml_loader()
|
||||
|
||||
|
||||
def yaml_load(stream):
|
||||
"""Parse YAML from a string or file object, safely and quickly."""
|
||||
if yaml is None:
|
||||
return require_yaml() # exits with the install hint
|
||||
return yaml.load(stream, Loader=_YAML_LOADER)
|
||||
|
||||
|
||||
_ALL_ALGORITHMS = frozenset({"sha1", "md5", "sha256", "crc32", "adler32"})
|
||||
|
||||
|
||||
@@ -200,7 +244,7 @@ def load_platform_config(platform_name: str, platforms_dir: str = "platforms") -
|
||||
raise FileNotFoundError(f"Platform config not found: {config_file}")
|
||||
|
||||
with open(config_file) as f:
|
||||
config = yaml.safe_load(f) or {}
|
||||
config = yaml_load(f) or {}
|
||||
|
||||
# Resolve inheritance
|
||||
if "inherits" in config:
|
||||
@@ -227,7 +271,7 @@ def load_platform_config(platform_name: str, platforms_dir: str = "platforms") -
|
||||
shared_real = os.path.realpath(shared_path)
|
||||
if shared_real not in _shared_yml_cache:
|
||||
with open(shared_path) as f:
|
||||
_shared_yml_cache[shared_real] = yaml.safe_load(f) or {}
|
||||
_shared_yml_cache[shared_real] = yaml_load(f) or {}
|
||||
shared = _shared_yml_cache[shared_real]
|
||||
shared_groups = shared.get("shared_groups", {})
|
||||
for system in config.get("systems", {}).values():
|
||||
@@ -256,7 +300,7 @@ def load_platform_config(platform_name: str, platforms_dir: str = "platforms") -
|
||||
reg_real = os.path.realpath(registry_path)
|
||||
if reg_real not in _shared_yml_cache:
|
||||
with open(registry_path) as f:
|
||||
_shared_yml_cache[reg_real] = yaml.safe_load(f) or {}
|
||||
_shared_yml_cache[reg_real] = yaml_load(f) or {}
|
||||
reg = _shared_yml_cache[reg_real]
|
||||
reg_entry = reg.get("platforms", {}).get(platform_name, {})
|
||||
|
||||
@@ -289,7 +333,7 @@ def load_data_dir_registry(platforms_dir: str = "platforms") -> dict:
|
||||
if not os.path.exists(registry_path):
|
||||
return {}
|
||||
with open(registry_path) as f:
|
||||
data = yaml.safe_load(f) or {}
|
||||
data = yaml_load(f) or {}
|
||||
return data.get("data_directories", {})
|
||||
|
||||
|
||||
@@ -299,7 +343,7 @@ def load_platform_registry(platforms_dir: str = "platforms") -> dict:
|
||||
if not os.path.exists(registry_path):
|
||||
return {}
|
||||
with open(registry_path) as f:
|
||||
return (yaml.safe_load(f) or {}).get("platforms", {})
|
||||
return (yaml_load(f) or {}).get("platforms", {})
|
||||
|
||||
|
||||
def list_registered_platforms(
|
||||
@@ -315,7 +359,7 @@ def list_registered_platforms(
|
||||
if not os.path.exists(registry_path):
|
||||
return []
|
||||
with open(registry_path) as f:
|
||||
registry = yaml.safe_load(f) or {}
|
||||
registry = yaml_load(f) or {}
|
||||
platforms = []
|
||||
for name, meta in sorted(registry.get("platforms", {}).items()):
|
||||
status = meta.get("status", "active")
|
||||
@@ -345,7 +389,7 @@ def load_target_config(
|
||||
f"No target config for platform '{platform_name}': {target_file}"
|
||||
)
|
||||
with open(target_file) as f:
|
||||
data = yaml.safe_load(f) or {}
|
||||
data = yaml_load(f) or {}
|
||||
|
||||
targets = data.get("targets", {})
|
||||
|
||||
@@ -353,7 +397,7 @@ def load_target_config(
|
||||
overrides = {}
|
||||
if os.path.exists(overrides_file):
|
||||
with open(overrides_file) as f:
|
||||
all_overrides = yaml.safe_load(f) or {}
|
||||
all_overrides = yaml_load(f) or {}
|
||||
overrides = all_overrides.get(platform_name, {}).get("targets", {})
|
||||
|
||||
alias_index: dict[str, str] = {}
|
||||
@@ -400,13 +444,13 @@ def list_available_targets(
|
||||
if not os.path.exists(target_file):
|
||||
return []
|
||||
with open(target_file) as f:
|
||||
data = yaml.safe_load(f) or {}
|
||||
data = yaml_load(f) or {}
|
||||
|
||||
overrides_file = os.path.join(targets_dir, "_overrides.yml")
|
||||
overrides = {}
|
||||
if os.path.exists(overrides_file):
|
||||
with open(overrides_file) as f:
|
||||
all_overrides = yaml.safe_load(f) or {}
|
||||
all_overrides = yaml_load(f) or {}
|
||||
overrides = all_overrides.get(platform_name, {}).get("targets", {})
|
||||
|
||||
result = []
|
||||
@@ -944,7 +988,7 @@ def load_emulator_profiles(
|
||||
if f.name.endswith(".old.yml"):
|
||||
continue
|
||||
with open(f) as fh:
|
||||
profile = yaml.safe_load(fh) or {}
|
||||
profile = yaml_load(fh) or {}
|
||||
if "emulator" not in profile:
|
||||
continue
|
||||
if skip_aliases and profile.get("type") == "alias":
|
||||
@@ -1032,7 +1076,7 @@ def group_identical_platforms(
|
||||
try:
|
||||
raw_path = os.path.join(platforms_dir, f"{platform}.yml")
|
||||
with open(raw_path) as f:
|
||||
raw = yaml.safe_load(f) or {}
|
||||
raw = yaml_load(f) or {}
|
||||
inherits[platform] = "inherits" in raw
|
||||
config = load_platform_config(platform, platforms_dir)
|
||||
except FileNotFoundError:
|
||||
|
||||
@@ -16,7 +16,7 @@ import os
|
||||
import sys
|
||||
|
||||
sys.path.insert(0, os.path.dirname(__file__))
|
||||
from common import list_registered_platforms, load_platform_config, require_yaml
|
||||
from common import list_registered_platforms, load_platform_config, require_yaml, yaml_load
|
||||
from truth import diff_platform_truth
|
||||
|
||||
yaml = require_yaml()
|
||||
@@ -27,7 +27,7 @@ def _load_truth(truth_dir: str, platform: str) -> dict | None:
|
||||
if not os.path.exists(path):
|
||||
return None
|
||||
with open(path) as f:
|
||||
return yaml.safe_load(f) or {}
|
||||
return yaml_load(f) or {}
|
||||
|
||||
|
||||
def _format_terminal(report: dict) -> str:
|
||||
|
||||
@@ -9,7 +9,7 @@ from pathlib import Path
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent))
|
||||
|
||||
import yaml
|
||||
from common import list_registered_platforms, load_platform_config
|
||||
from common import list_registered_platforms, load_platform_config, yaml_load
|
||||
from exporter import discover_exporters
|
||||
|
||||
OUTPUT_FILENAMES: dict[str, str] = {
|
||||
@@ -60,7 +60,7 @@ def run(
|
||||
continue
|
||||
|
||||
with open(truth_file) as f:
|
||||
truth_data = yaml.safe_load(f) or {}
|
||||
truth_data = yaml_load(f) or {}
|
||||
|
||||
scraped: dict | None = None
|
||||
try:
|
||||
|
||||
@@ -20,6 +20,7 @@ from common import (
|
||||
load_platform_config,
|
||||
load_target_config,
|
||||
require_yaml,
|
||||
yaml_load,
|
||||
)
|
||||
from truth import generate_platform_truth
|
||||
|
||||
@@ -72,7 +73,7 @@ def main(argv: list[str] | None = None) -> None:
|
||||
# Load registry
|
||||
registry_path = os.path.join(args.platforms_dir, "_registry.yml")
|
||||
with open(registry_path) as f:
|
||||
registry = (yaml.safe_load(f) or {}).get("platforms", {})
|
||||
registry = (yaml_load(f) or {}).get("platforms", {})
|
||||
|
||||
# Load emulator profiles
|
||||
profiles = load_emulator_profiles(args.emulators_dir)
|
||||
|
||||
@@ -23,6 +23,7 @@ import urllib.error
|
||||
import urllib.request
|
||||
import zipfile
|
||||
from pathlib import Path
|
||||
from common import yaml_load
|
||||
|
||||
try:
|
||||
import yaml
|
||||
@@ -45,7 +46,7 @@ def load_registry(registry_path: str = DEFAULT_REGISTRY) -> dict[str, dict]:
|
||||
if not path.exists():
|
||||
raise FileNotFoundError(f"Registry not found: {registry_path}")
|
||||
with open(path) as f:
|
||||
data = yaml.safe_load(f) or {}
|
||||
data = yaml_load(f) or {}
|
||||
return data.get("data_directories", {})
|
||||
|
||||
|
||||
|
||||
@@ -10,9 +10,18 @@ from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import pkgutil
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
from .base_scraper import BaseScraper
|
||||
# Scrapers run both as `python -m scripts.scraper.x` and as plain scripts, and
|
||||
# they share helpers with the rest of scripts/. Putting that directory on the
|
||||
# path once here is what lets every submodule say `from common import ...`
|
||||
# instead of carrying its own bootstrap.
|
||||
_SCRIPTS_DIR = str(Path(__file__).resolve().parent.parent)
|
||||
if _SCRIPTS_DIR not in sys.path:
|
||||
sys.path.insert(0, _SCRIPTS_DIR)
|
||||
|
||||
from .base_scraper import BaseScraper # noqa: E402
|
||||
|
||||
_scrapers: dict[str, type] = {}
|
||||
|
||||
|
||||
@@ -15,6 +15,8 @@ from typing import Any
|
||||
|
||||
import yaml
|
||||
|
||||
from common import yaml_load
|
||||
|
||||
|
||||
_MAME_RELEASE_RE = re.compile(r"^0\.\d+")
|
||||
|
||||
@@ -294,7 +296,7 @@ def _diff_fbneo(
|
||||
|
||||
def _load_yaml(path: str) -> dict[str, Any]:
|
||||
with open(path, encoding="utf-8") as f:
|
||||
return yaml.safe_load(f) or {}
|
||||
return yaml_load(f) or {}
|
||||
|
||||
|
||||
def _load_json(path: str) -> dict[str, Any]:
|
||||
@@ -496,7 +498,7 @@ def _patch_bios_entries(text: str, files: list[dict]) -> str:
|
||||
def _append_new_entries(text: str, files: list[dict], original: str) -> str:
|
||||
"""Append new bios_zip entries (system=None) that aren't in the original."""
|
||||
# Parse original to get existing entry names (more reliable than text search)
|
||||
existing_data = yaml.safe_load(original) or {}
|
||||
existing_data = yaml_load(original) or {}
|
||||
existing_names = {f["name"] for f in existing_data.get("files", [])}
|
||||
|
||||
new_entries = []
|
||||
@@ -568,7 +570,7 @@ def _backup_and_write_fbneo(path: str, data: dict, hashes: dict) -> None:
|
||||
patched = _patch_core_version(original, data.get("core_version", ""))
|
||||
|
||||
# Identify new ROM entries by comparing parsed data keys, not text search
|
||||
existing_data = yaml.safe_load(original) or {}
|
||||
existing_data = yaml_load(original) or {}
|
||||
existing_keys = {
|
||||
(f["archive"], f["name"])
|
||||
for f in existing_data.get("files", [])
|
||||
|
||||
@@ -9,6 +9,7 @@ import urllib.request
|
||||
from abc import ABC, abstractmethod
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
from common import yaml_load
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -277,7 +278,7 @@ def scraper_cli(
|
||||
output_path = Path(args.output)
|
||||
if output_path.exists():
|
||||
with open(output_path) as f:
|
||||
existing = yaml.safe_load(f) or {}
|
||||
existing = yaml_load(f) or {}
|
||||
# Preserve existing keys not generated by the scraper.
|
||||
# Only keys present in the NEW config are considered scraper-generated.
|
||||
# Everything else in the existing file is preserved.
|
||||
|
||||
@@ -18,6 +18,8 @@ from pathlib import Path
|
||||
|
||||
import yaml
|
||||
|
||||
from common import yaml_load
|
||||
|
||||
from .base_scraper import BaseScraper, BiosRequirement
|
||||
|
||||
PLATFORM_NAME = "batocera"
|
||||
@@ -203,7 +205,7 @@ class Scraper(BaseScraper):
|
||||
raise ConnectionError(
|
||||
f"Failed to fetch {CONFIGGEN_DEFAULTS_URL}: {e}"
|
||||
) from e
|
||||
data = yaml.safe_load(raw)
|
||||
data = yaml_load(raw)
|
||||
cores: set[str] = set()
|
||||
standalone: set[str] = set()
|
||||
for system, cfg in data.items():
|
||||
@@ -386,7 +388,7 @@ class Scraper(BaseScraper):
|
||||
)
|
||||
if existing.exists():
|
||||
with open(existing) as f:
|
||||
old = yaml.safe_load(f) or {}
|
||||
old = yaml_load(f) or {}
|
||||
batocera_version = str(old.get("version", ""))
|
||||
|
||||
cores, standalone = self._fetch_cores()
|
||||
|
||||
@@ -54,8 +54,6 @@ STATUS_RANK = {
|
||||
"Ideal": 4,
|
||||
}
|
||||
|
||||
GAME_DATA_SYSTEMS = {"BSX", "Doom"}
|
||||
GAME_DATA_FILES = {"VEC_Minestorm.vec"}
|
||||
|
||||
SYSTEM_ID_MAP: dict[str, str] = {
|
||||
"32X": "sega-32x",
|
||||
|
||||
@@ -19,6 +19,8 @@ from typing import Any
|
||||
|
||||
import yaml
|
||||
|
||||
from common import yaml_load
|
||||
|
||||
from scripts.scraper._hash_merge import compute_diff, merge_fbneo_profile
|
||||
from scripts.scraper.fbneo_parser import parse_fbneo_source_tree
|
||||
|
||||
@@ -194,7 +196,7 @@ def _find_fbneo_profiles() -> list[Path]:
|
||||
if path.name.endswith(".old.yml"):
|
||||
continue
|
||||
try:
|
||||
data = yaml.safe_load(path.read_text(encoding="utf-8"))
|
||||
data = yaml_load(path.read_text(encoding="utf-8"))
|
||||
except (yaml.YAMLError, OSError):
|
||||
continue
|
||||
if not data or not isinstance(data, dict):
|
||||
|
||||
@@ -15,6 +15,8 @@ from __future__ import annotations
|
||||
import xml.etree.ElementTree as ET
|
||||
from dataclasses import dataclass, field
|
||||
|
||||
from common import parse_untrusted_xml
|
||||
|
||||
|
||||
@dataclass
|
||||
class LogiqxRom:
|
||||
@@ -46,10 +48,7 @@ def parse_logiqx(content: str | bytes) -> LogiqxDat:
|
||||
rejected: DAT files never define them, and expanding entities
|
||||
from untrusted packs opens entity-expansion attacks.
|
||||
"""
|
||||
haystack = content if isinstance(content, str) else content.decode("utf-8", "replace")
|
||||
if "<!ENTITY" in haystack.upper():
|
||||
raise ValueError("XML entity declarations are not allowed in DAT files")
|
||||
root = ET.fromstring(content)
|
||||
root = parse_untrusted_xml(content, "DAT files")
|
||||
dat = LogiqxDat()
|
||||
|
||||
header = root.find("header")
|
||||
|
||||
@@ -21,6 +21,8 @@ from typing import Any
|
||||
|
||||
import yaml
|
||||
|
||||
from common import yaml_load
|
||||
|
||||
from ._hash_merge import compute_diff, merge_mame_profile
|
||||
from .mame_parser import parse_mame_source_tree
|
||||
|
||||
@@ -165,7 +167,7 @@ def _find_mame_profiles() -> list[Path]:
|
||||
continue
|
||||
try:
|
||||
with open(path, encoding="utf-8") as f:
|
||||
data = yaml.safe_load(f)
|
||||
data = yaml_load(f)
|
||||
if not isinstance(data, dict):
|
||||
continue
|
||||
upstream = data.get("upstream", "")
|
||||
|
||||
@@ -16,7 +16,8 @@ Recalbox verification logic:
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import xml.etree.ElementTree as ET
|
||||
|
||||
from common import parse_untrusted_xml
|
||||
|
||||
from .base_scraper import BaseScraper, BiosRequirement
|
||||
|
||||
@@ -109,7 +110,7 @@ class Scraper(BaseScraper):
|
||||
def _fetch_cores(self) -> list[str]:
|
||||
"""Extract unique core names from es_bios.xml bios elements."""
|
||||
raw = self._fetch_raw()
|
||||
root = ET.fromstring(raw)
|
||||
root = parse_untrusted_xml(raw, "es_bios.xml")
|
||||
cores: set[str] = set()
|
||||
for bios_elem in root.findall(".//system/bios"):
|
||||
raw_core = bios_elem.get("core", "").strip()
|
||||
@@ -128,7 +129,7 @@ class Scraper(BaseScraper):
|
||||
if not self.validate_format(raw):
|
||||
raise ValueError("es_bios.xml format validation failed")
|
||||
|
||||
root = ET.fromstring(raw)
|
||||
root = parse_untrusted_xml(raw, "es_bios.xml")
|
||||
requirements = []
|
||||
seen = set()
|
||||
|
||||
@@ -177,7 +178,7 @@ class Scraper(BaseScraper):
|
||||
def fetch_full_requirements(self) -> list[dict]:
|
||||
"""Parse es_bios.xml preserving all Recalbox-specific fields."""
|
||||
raw = self._fetch_raw()
|
||||
root = ET.fromstring(raw)
|
||||
root = parse_untrusted_xml(raw, "es_bios.xml")
|
||||
requirements = []
|
||||
|
||||
for system_elem in root.findall(".//system"):
|
||||
|
||||
@@ -19,6 +19,8 @@ from datetime import datetime, timezone
|
||||
|
||||
import yaml
|
||||
|
||||
from common import yaml_load
|
||||
|
||||
from . import BaseTargetScraper
|
||||
|
||||
PLATFORM_NAME = "batocera"
|
||||
@@ -221,7 +223,7 @@ def _parse_es_systems(text: str) -> dict[str, list[str]]:
|
||||
<core_name>: {requireAnyOf: [BR2_PACKAGE_FOO]}
|
||||
"""
|
||||
try:
|
||||
data = yaml.safe_load(text)
|
||||
data = yaml_load(text)
|
||||
except yaml.YAMLError:
|
||||
return {}
|
||||
|
||||
|
||||
@@ -17,6 +17,8 @@ from datetime import datetime, timezone
|
||||
|
||||
import yaml
|
||||
|
||||
from common import yaml_load
|
||||
|
||||
from . import BaseTargetScraper
|
||||
|
||||
PLATFORM_NAME = "emudeck"
|
||||
@@ -134,7 +136,7 @@ class Scraper(BaseTargetScraper):
|
||||
if not os.path.exists(target_path):
|
||||
return []
|
||||
with open(target_path) as f:
|
||||
data = yaml.safe_load(f) or {}
|
||||
data = yaml_load(f) or {}
|
||||
# Find a target matching the architecture
|
||||
for tname, tinfo in data.get("targets", {}).items():
|
||||
if tinfo.get("architecture") == arch:
|
||||
|
||||
@@ -25,7 +25,7 @@ import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, os.path.dirname(__file__))
|
||||
from common import compute_hashes, list_registered_platforms, load_database
|
||||
from common import compute_hashes, list_registered_platforms, load_database, yaml_load
|
||||
|
||||
try:
|
||||
import yaml
|
||||
@@ -113,7 +113,7 @@ def load_platform_hashes(platforms_dir: str) -> dict:
|
||||
f = Path(platforms_dir) / f"{name}.yml"
|
||||
with open(f) as fh:
|
||||
try:
|
||||
config = yaml.safe_load(fh) or {}
|
||||
config = yaml_load(fh) or {}
|
||||
except yaml.YAMLError:
|
||||
continue
|
||||
|
||||
|
||||
Reference in new issue
Block a user