From 3b8f2d75d51977fbadbb8b2e1cad134d83dffaef Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:55:16 +0200 Subject: [PATCH] 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. --- scripts/check_buildbot_system.py | 3 +- scripts/common.py | 68 +++++++++++++++---- scripts/diff_truth.py | 4 +- scripts/export_native.py | 4 +- scripts/generate_truth.py | 3 +- scripts/refresh_data_dirs.py | 3 +- scripts/scraper/__init__.py | 11 ++- scripts/scraper/_hash_merge.py | 8 ++- scripts/scraper/base_scraper.py | 3 +- scripts/scraper/batocera_scraper.py | 6 +- scripts/scraper/bizhawk_scraper.py | 2 - scripts/scraper/fbneo_hash_scraper.py | 4 +- scripts/scraper/logiqx_parser.py | 7 +- scripts/scraper/mame_hash_scraper.py | 4 +- scripts/scraper/recalbox_scraper.py | 9 +-- .../targets/batocera_targets_scraper.py | 4 +- .../targets/emudeck_targets_scraper.py | 4 +- scripts/validate_pr.py | 4 +- 18 files changed, 109 insertions(+), 42 deletions(-) diff --git a/scripts/check_buildbot_system.py b/scripts/check_buildbot_system.py index 0a80dee1..4743cba7 100644 --- a/scripts/check_buildbot_system.py +++ b/scripts/check_buildbot_system.py @@ -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", "") diff --git a/scripts/common.py b/scripts/common.py index 4cc17875..632a91aa 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -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 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: diff --git a/scripts/diff_truth.py b/scripts/diff_truth.py index 74fe3d0f..738933d6 100644 --- a/scripts/diff_truth.py +++ b/scripts/diff_truth.py @@ -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: diff --git a/scripts/export_native.py b/scripts/export_native.py index 8df4b368..c9f00482 100644 --- a/scripts/export_native.py +++ b/scripts/export_native.py @@ -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: diff --git a/scripts/generate_truth.py b/scripts/generate_truth.py index 9bbc84b1..ba51a9e3 100644 --- a/scripts/generate_truth.py +++ b/scripts/generate_truth.py @@ -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) diff --git a/scripts/refresh_data_dirs.py b/scripts/refresh_data_dirs.py index d6ca7095..41adb09b 100644 --- a/scripts/refresh_data_dirs.py +++ b/scripts/refresh_data_dirs.py @@ -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", {}) diff --git a/scripts/scraper/__init__.py b/scripts/scraper/__init__.py index d859f727..00868fe7 100644 --- a/scripts/scraper/__init__.py +++ b/scripts/scraper/__init__.py @@ -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] = {} diff --git a/scripts/scraper/_hash_merge.py b/scripts/scraper/_hash_merge.py index 3574b188..0db1353e 100644 --- a/scripts/scraper/_hash_merge.py +++ b/scripts/scraper/_hash_merge.py @@ -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", []) diff --git a/scripts/scraper/base_scraper.py b/scripts/scraper/base_scraper.py index 484fb3ec..90fa589a 100644 --- a/scripts/scraper/base_scraper.py +++ b/scripts/scraper/base_scraper.py @@ -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. diff --git a/scripts/scraper/batocera_scraper.py b/scripts/scraper/batocera_scraper.py index 291c8a5a..81b2f31e 100644 --- a/scripts/scraper/batocera_scraper.py +++ b/scripts/scraper/batocera_scraper.py @@ -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() diff --git a/scripts/scraper/bizhawk_scraper.py b/scripts/scraper/bizhawk_scraper.py index 2a0d314a..34053bf4 100644 --- a/scripts/scraper/bizhawk_scraper.py +++ b/scripts/scraper/bizhawk_scraper.py @@ -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", diff --git a/scripts/scraper/fbneo_hash_scraper.py b/scripts/scraper/fbneo_hash_scraper.py index f1c4c8b4..ee97c176 100644 --- a/scripts/scraper/fbneo_hash_scraper.py +++ b/scripts/scraper/fbneo_hash_scraper.py @@ -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): diff --git a/scripts/scraper/logiqx_parser.py b/scripts/scraper/logiqx_parser.py index 56903f9f..b45a718f 100644 --- a/scripts/scraper/logiqx_parser.py +++ b/scripts/scraper/logiqx_parser.py @@ -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 " 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", "") diff --git a/scripts/scraper/recalbox_scraper.py b/scripts/scraper/recalbox_scraper.py index 259762fa..7992b50d 100644 --- a/scripts/scraper/recalbox_scraper.py +++ b/scripts/scraper/recalbox_scraper.py @@ -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"): diff --git a/scripts/scraper/targets/batocera_targets_scraper.py b/scripts/scraper/targets/batocera_targets_scraper.py index 91dd0b7e..c306b9e6 100644 --- a/scripts/scraper/targets/batocera_targets_scraper.py +++ b/scripts/scraper/targets/batocera_targets_scraper.py @@ -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]]: : {requireAnyOf: [BR2_PACKAGE_FOO]} """ try: - data = yaml.safe_load(text) + data = yaml_load(text) except yaml.YAMLError: return {} diff --git a/scripts/scraper/targets/emudeck_targets_scraper.py b/scripts/scraper/targets/emudeck_targets_scraper.py index ec3bdd0a..96c61a88 100644 --- a/scripts/scraper/targets/emudeck_targets_scraper.py +++ b/scripts/scraper/targets/emudeck_targets_scraper.py @@ -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: diff --git a/scripts/validate_pr.py b/scripts/validate_pr.py index 2c3f99d5..57a82a54 100644 --- a/scripts/validate_pr.py +++ b/scripts/validate_pr.py @@ -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