From baf38ffbeeb8d649d61a1395104d6762869e9cf9 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Thu, 22 Oct 2026 12:23:26 +0200 Subject: [PATCH] refactor: compute coverage once, index proofs EntryCategories asked every core in turn: 3.8 million proofs for 7,430 entries. ProofIndex looks each key up once, and verify --all --json is byte-identical (24 s to 17 s). generate_site and generate_readme now share one coverage, cached in tmp/coverage-cache keyed by its inputs, so the README step drops from 25 s to 3 s. --- scripts/exporter/candidates.py | 93 ++++++++++++++++++++++++--- scripts/generate_readme.py | 113 ++++++++++++++++++++++++++++++--- scripts/generate_site.py | 21 +++--- tests/test_coverage_cache.py | 100 +++++++++++++++++++++++++++++ tests/test_entry_categories.py | 53 +++++++++++++++- wiki/testing-guide.md | 2 +- 6 files changed, 349 insertions(+), 33 deletions(-) create mode 100644 tests/test_coverage_cache.py diff --git a/scripts/exporter/candidates.py b/scripts/exporter/candidates.py index 8b8c2b66..e4fe619f 100644 --- a/scripts/exporter/candidates.py +++ b/scripts/exporter/candidates.py @@ -136,6 +136,81 @@ class ReadEvidence: return None +# The order ReadEvidence.proof tries: a reader's proof is the first that holds. +_PROOF_ORDER = ("path", "launcher", "name", "hash", "directory", "data_directory") + + +class ProofIndex: + """ReadEvidence.proof for many readers at once, by lookup. + + Asking every core in turn cost one proof per core and entry: 3.8 million + calls to categorize 7,430 entries. Each key a reader can match is indexed + once, and an entry's own keys find the readers it concerns. The proof of + each reader is the one ReadEvidence.proof returns. + """ + + def __init__(self, readers: list[ReadEvidence]) -> None: + self._index: dict[str, dict[str, set[int]]] = { + kind: {} for kind in ("tail", "launched", "name", "hash", "cut", "dir", "tree", "member") + } + for position, evidence in enumerate(readers): + for kind, keys in ( + ("tail", evidence.tails), ("launched", evidence.launched), + ("name", evidence.names), ("hash", evidence.hashes), + ("dir", evidence.directories), ("tree", evidence.trees), + ("member", evidence.tree_members), + ): + for key in keys: + self._index[kind].setdefault(key, set()).add(position) + # A cut md5 names any hash it is the start of. + for value in evidence.hashes: + for length in range(_CUT_MD5, _FULL_MD5): + if len(value) >= length: + self._index["cut"].setdefault(value[:length], set()).add(position) + + def _hits(self, kind: str, keys) -> set[int]: + table = self._index[kind] + found: set[int] = set() + for key in keys: + found |= table.get(key, set()) + return found + + def proofs(self, entry: dict, destination: str) -> dict[int, str]: + """Reader position -> its proof, for the readers that have one.""" + names = { + str(n).casefold() + for n in (entry.get("name"), *(entry.get("aliases") or [])) + if n + } + tails = _tails(destination) + last = {tails[-1]} if tails else set() + hashes = { + v for f in (*HASH_FIELDS, "alt_md5") for v in _hash_values(entry, f) + } + cut = {v for v in hashes if _CUT_MD5 <= len(v) < _FULL_MD5} + # "/" + d in "/" + path: d is a run of whole segments ending in "/", + # so it is a run of the path's segments before the last one. + parts = tails[0].split("/") if tails else [] + runs = { + "/".join(parts[i:j]) + "/" + for i in range(len(parts)) for j in range(i + 1, len(parts)) + } + found = { + "path": self._hits("tail", tails[:-1]), + "launcher": self._hits("launched", names | last), + "name": self._hits("name", names | last), + "hash": self._hits("hash", hashes) | self._hits("cut", cut), + "directory": self._hits("dir", runs), + "data_directory": self._hits("tree", runs) + | self._index["member"].get(tails[0] if tails else "", set()), + } + result: dict[int, str] = {} + for kind in _PROOF_ORDER: + for position in found[kind]: + result.setdefault(position, kind) + return result + + def _loads_romsets(profile: dict) -> bool: """A core that opens ROM sets by name: MAME pulls a device's set from device_t::searchpath(), and no profile lists every device set.""" @@ -275,6 +350,8 @@ class EntryCategories: self._readers.append((kind, key, evidence)) self._launchers = ReadEvidence() self._launchers.add_launchers(config.get("launchers") or []) + self._proofs = ProofIndex([evidence for _kind, _key, evidence in self._readers] + + [self._launchers]) self._by_system: dict[str, list[str]] = {} for key in sorted(examined): for sid in profiles[key].get("systems") or []: @@ -313,14 +390,14 @@ class EntryCategories: def explain(self, entry: dict, destination: str, system: str) -> dict: """The category of an entry and what establishes it.""" readers: dict[str, list[dict]] = {BIOS: [], GAME_DATA: []} - for kind, key, evidence in self._readers: - proof = evidence.proof(entry, destination) - if proof is not None: - readers[kind].append({ - "emulator": key, "proof": proof, - "entries": self._matching(key, kind, entry), - }) - if self._launchers.proof(entry, destination) is not None: + proofs = self._proofs.proofs(entry, destination) + for position in sorted(p for p in proofs if p < len(self._readers)): + kind, key, _evidence = self._readers[position] + readers[kind].append({ + "emulator": key, "proof": proofs[position], + "entries": self._matching(key, kind, entry), + }) + if len(self._readers) in proofs: readers[BIOS].append({"emulator": "launcher", "proof": "launcher", "entries": []}) if system in self._system_runs: system_cores, unresolved = self._system_runs[system] diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index 1a8ab504..519d4202 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -111,6 +111,105 @@ def compute_coverage( } +_REPO = Path(__file__).resolve().parent.parent +# The site and the README read the same coverage: the first generator to run +# stores it here and the second reads it back, instead of verifying every +# platform twice (about a minute of the site build). +COVERAGE_CACHE_DIR = _REPO / "tmp" / "coverage-cache" +_COVERAGE_CACHE_FORMAT = "1" + + +def _coverage_inputs(db_path: Path, platforms_dir: str, emulators_dir: str) -> str: + """A digest of everything coverage is computed from. + + The database, the platform and emulator files, the code, the install + manifests, the data directory revisions and the state of bios/ on disk: + a change to any of them computes the coverage again. + """ + import hashlib + + digest = hashlib.sha256(_COVERAGE_CACHE_FORMAT.encode()) + + def add_file(path: Path) -> None: + digest.update(str(path).encode() + b"\0") + digest.update(path.read_bytes() if path.is_file() else b"") + + add_file(db_path.resolve()) + for root, pattern in ((Path(platforms_dir), "*"), (Path(emulators_dir), "*.yml"), + (_REPO / "scripts", "*.py"), (_REPO / "install", "*.json"), + (_REPO / "recipes", "*.json")): + for path in sorted(p for p in root.resolve().rglob(pattern) if p.is_file()): + add_file(path) + for path in (_REPO / ".gitignore", _REPO / "_mame_clones.json", + _REPO / "data" / ".versions.json"): + add_file(path) + bios = _REPO / "bios" + for path in sorted(bios.rglob("*")): + if path.is_file(): + stat = path.stat() + digest.update(f"{path.relative_to(bios)}\0{stat.st_size}\0{stat.st_mtime_ns}\n".encode()) + return digest.hexdigest() + + +def platform_coverages( + platform_names: list[str], + platforms_dir: str, + db: dict, + data_registry: dict | None = None, + emulators_dir: str = _EMULATORS_DIR, + db_path: str = "database.json", + report: Callable[[str, dict], None] | None = None, +) -> dict[str, dict]: + """compute_coverage for each platform, read from the cache when its + inputs have not changed since a generator last stored it. + + A platform with no YAML is left out, as compute_coverage raises for it. + ``report`` is called with each platform and its coverage. + """ + # pickle, not JSON: the coverage holds sets and enums that JSON would + # turn into lists and strings. The file is written by this function into + # the repository's own tmp/, never downloaded. + import pickle + + key = _coverage_inputs(Path(db_path), platforms_dir, emulators_dir) + names = sorted(platform_names) + cache = COVERAGE_CACHE_DIR / f"{key}.pickle" + coverages: dict[str, dict] | None = None + if cache.is_file(): + try: + with cache.open("rb") as handle: + stored = pickle.load(handle) + except (OSError, EOFError, pickle.UnpicklingError) as exc: + print(f" coverage cache unreadable, computing again: {exc}", file=sys.stderr) + else: + if stored.get("platforms") == names: + coverages = stored["coverages"] + print(f" coverage read from {cache.relative_to(_REPO)}") + if coverages is None: + coverages = {} + for name in names: + try: + coverages[name] = compute_coverage( + name, platforms_dir, db, data_registry, emulators_dir=emulators_dir, + ) + except FileNotFoundError as exc: + print(f" {name}: skipped ({exc})", file=sys.stderr) + COVERAGE_CACHE_DIR.mkdir(parents=True, exist_ok=True) + for stale in COVERAGE_CACHE_DIR.glob("*.pickle"): + stale.unlink() + scratch = cache.with_suffix(f".{os.getpid()}.tmp") + with scratch.open("wb") as handle: + pickle.dump({"platforms": names, "coverages": coverages}, handle, + protocol=pickle.HIGHEST_PROTOCOL) + scratch.replace(cache) + ordered = {name: coverages[name] for name in platform_names if name in coverages} + if report: + for name in names: + if name in ordered: + report(name, ordered[name]) + return ordered + + def release_totals(platform_name: str, record: dict) -> tuple[int | None, int | None]: """Files and bytes of the pack the latest release serves for a platform. @@ -347,7 +446,7 @@ def _catalog_matched_line(db: dict) -> list[str]: ] -def generate_readme(db: dict, platforms_dir: str) -> str: +def generate_readme(db: dict, platforms_dir: str, db_path: str = "database.json") -> str: total_files = db.get("total_files", 0) total_size = db.get("total_size", 0) ts = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") @@ -358,14 +457,8 @@ def generate_readme(db: dict, platforms_dir: str) -> str: data_registry = load_data_dir_registry(platforms_dir) - coverages = {} - for name in platform_names: - try: - coverages[name] = compute_coverage( - name, platforms_dir, db, data_registry - ) - except FileNotFoundError: - pass + coverages = platform_coverages(platform_names, platforms_dir, db, data_registry, + db_path=db_path) profiles = unique_emulator_profiles( load_emulator_profiles("emulators", skip_aliases=False) @@ -853,7 +946,7 @@ def main() -> None: # published counts taken from half of it. with contextlib.ExitStack() as held: hold_data_dirs(held, load_data_dir_registry(args.platforms_dir)) - readme = generate_readme(db, args.platforms_dir) + readme = generate_readme(db, args.platforms_dir, args.db) status = "Generated" if write_if_changed("README.md", readme) else "Unchanged" print(f"{status} ./README.md") diff --git a/scripts/generate_site.py b/scripts/generate_site.py index ac529ac8..8350170e 100644 --- a/scripts/generate_site.py +++ b/scripts/generate_site.py @@ -65,6 +65,7 @@ from nativemode import digest_algorithm, reads_file_contents yaml = require_yaml() from generate_readme import ( compute_coverage, + platform_coverages, contributing_text, download_links, extract_notes, @@ -5955,19 +5956,13 @@ def _generate(args: argparse.Namespace) -> None: suppl_names = _build_supplemental_index() print("Computing platform coverage...") - coverages = {} - for name in sorted(platform_names): - try: - cov = compute_coverage( - name, args.platforms_dir, db, data_registry, - emulators_dir=args.emulators_dir, - ) - coverages[name] = cov - print( - f" {cov['platform']}: {cov['present']}/{cov['total']} ({_pct(cov['present'], cov['total'])})" - ) - except FileNotFoundError as e: - print(f" {name}: skipped ({e})", file=sys.stderr) + coverages = platform_coverages( + sorted(platform_names), args.platforms_dir, db, data_registry, + emulators_dir=args.emulators_dir, db_path=args.db, + report=lambda _name, cov: print( + f" {cov['platform']}: {cov['present']}/{cov['total']} ({_pct(cov['present'], cov['total'])})" + ), + ) print("Loading emulator profiles...") profiles = load_emulator_profiles(args.emulators_dir, skip_aliases=False) diff --git a/tests/test_coverage_cache.py b/tests/test_coverage_cache.py new file mode 100644 index 00000000..b9b38398 --- /dev/null +++ b/tests/test_coverage_cache.py @@ -0,0 +1,100 @@ +"""The site and the README share one coverage computation. + +generate_site.py and generate_readme.py each verified every platform, a +minute of the site build spent twice on the same answer. The first to run +stores the coverage, keyed by its inputs, and the second reads it. A cache +that outlived a change to its inputs would publish stale counts, so these +tests change an input and expect a fresh computation. +""" + +from __future__ import annotations + +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +import generate_readme # noqa: E402 + + +class CoverageCache(unittest.TestCase): + def setUp(self) -> None: + self.work = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp" / "tests")) + self.platforms = self.work / "platforms" + self.platforms.mkdir() + (self.platforms / "alpha.yml").write_text("platform: Alpha\n") + self.emulators = self.work / "emulators" + self.emulators.mkdir() + self.db = self.work / "database.json" + self.db.write_text("{}") + self.calls: list[str] = [] + patches = [ + mock.patch.object(generate_readme, "COVERAGE_CACHE_DIR", self.work / "cache"), + mock.patch.object(generate_readme, "compute_coverage", self._compute), + ] + for patch in patches: + patch.start() + self.addCleanup(patch.stop) + + def tearDown(self) -> None: + import shutil + + shutil.rmtree(self.work, ignore_errors=True) + + def _compute(self, name, platforms_dir, db, data_registry=None, emulators_dir=""): + self.calls.append(name) + if name == "ghost": + raise FileNotFoundError(name) + return {"platform": name.title(), "missing_names": {"a.bin"}} + + def _run(self, names=("beta", "alpha")) -> dict: + return generate_readme.platform_coverages( + list(names), str(self.platforms), {}, {}, + emulators_dir=str(self.emulators), db_path=str(self.db), + ) + + def test_second_run_reads_the_first(self): + first = self._run() + self.assertEqual(self.calls, ["alpha", "beta"]) + second = self._run() + self.assertEqual(self.calls, ["alpha", "beta"]) + self.assertEqual(first, second) + # The caller's order, not the computation's. + self.assertEqual(list(second), ["beta", "alpha"]) + self.assertEqual(second["alpha"]["missing_names"], {"a.bin"}) + + def test_a_changed_input_computes_again(self): + self._run() + for change in ( + lambda: (self.platforms / "alpha.yml").write_text("platform: Alpha 2\n"), + lambda: self.db.write_text('{"files": {}}'), + lambda: (self.emulators / "core.yml").write_text("emulator: core\n"), + ): + self.calls.clear() + change() + self._run() + self.assertEqual(self.calls, ["alpha", "beta"]) + + def test_another_platform_list_computes_again(self): + self._run() + self.calls.clear() + self._run(("alpha",)) + self.assertEqual(self.calls, ["alpha"]) + + def test_a_platform_without_a_file_is_left_out(self): + self.assertEqual(list(self._run(("ghost", "alpha"))), ["alpha"]) + self.assertEqual(list(self._run(("ghost", "alpha"))), ["alpha"]) + + def test_one_cache_file_is_kept(self): + self._run() + (self.platforms / "alpha.yml").write_text("platform: Alpha 3\n") + self._run() + self.assertEqual(len(list((self.work / "cache").glob("*.pickle"))), 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_entry_categories.py b/tests/test_entry_categories.py index d6cbb324..3d2791a9 100644 --- a/tests/test_entry_categories.py +++ b/tests/test_entry_categories.py @@ -13,7 +13,14 @@ from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "scripts")) import verify # noqa: E402 -from exporter.candidates import BIOS, GAME_DATA, UNREAD, EntryCategories # noqa: E402 +from exporter.candidates import ( # noqa: E402 + BIOS, + GAME_DATA, + UNREAD, + EntryCategories, + ProofIndex, + ReadEvidence, +) PROFILES = { "bluemsx": { @@ -144,5 +151,49 @@ class StrictCountsFirmwareOnly(unittest.TestCase): self.assertEqual(evidence["diabdat.mpq"]["read_by"][0]["emulator"], "devilutionx") +class IndexedProof(unittest.TestCase): + """ProofIndex answers what ReadEvidence.proof answers, reader by reader.""" + + def test_every_kind_matches_the_direct_proof(self): + md5 = "490f666e1afb15b7362b406ed1cea246" + readers = [] + for profile in ( + {"files": [{"name": "a.bin", "path": "sys/dir/a.bin"}]}, + {"files": [{"name": "other.rom", "aliases": ["B.ROM"]}]}, + {"files": [{"name": "c.bin", "md5": md5}]}, + {"files": [{"name": "tree/", "type": "directory", "path": "data/tree/"}]}, + {"files": [], "data_directories": [{"ref": "x", "destination": "assets/sys"}]}, + {"files": [{"name": "d.bin", "sha1": "ab" * 20, "crc32": "12345678"}]}, + ): + evidence = ReadEvidence() + evidence.add_profile(profile, {}) + readers.append(evidence) + launcher = ReadEvidence() + launcher.add_launchers([{"names": ["scph101.bin"]}]) + readers.append(launcher) + index = ProofIndex(readers) + cases = [ + ({"name": "a.bin"}, "bios/sys/dir/a.bin"), + ({"name": "a.bin"}, "elsewhere/a.bin"), + ({"name": "x", "aliases": ["b.rom"]}, "x"), + ({"name": "y", "md5": md5[:29]}, "y"), + ({"name": "y", "md5": md5.upper()}, "y"), + ({"name": "z", "md5": md5[:28]}, "z"), + ({"name": "z"}, "root/data/tree/deep/z"), + ({"name": "z"}, "data/tree"), + ({"name": "z"}, "a/assets/sys/z.png"), + ({"name": "scph101.bin"}, "bios/scph101.bin"), + ({"name": "q", "crc32": "12345678, 9"}, "q"), + ({"name": "q"}, ""), + ] + for entry, destination in cases: + expected = { + position: proof for position, evidence in enumerate(readers) + if (proof := evidence.proof(entry, destination)) is not None + } + with self.subTest(entry=entry, destination=destination): + self.assertEqual(index.proofs(entry, destination), expected) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index 5cf5e803..51bda600 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -51,7 +51,7 @@ Every module in `tests/`, by the area it guards. | Handheld port | stubbed PortMaster dialogs, fake installer | `test_port` | | Truth and native export | synthetic + cached originals | `test_export_counts`, `test_export_native_layer`, `test_export_report`, `test_exporters`, `test_truth_data_dir_readers`, `test_truth_diff`, `test_truth_layout` | | Scrapers, upstream and provenance | inline sources, synthetic | `test_check_freshness`, `test_dedup`, `test_elec_scrapers`, `test_fbneo_parser`, `test_find_pinned_revision`, `test_hash_merge`, `test_launchers`, `test_launchers_emudeck`, `test_launchers_recalbox`, `test_launchers_retrobat`, `test_launchers_retrodeck`, `test_launchers_retropie`, `test_launchers_rocknix`, `test_mame_devices`, `test_mame_parser`, `test_mame_ref_audit`, `test_profile_sync`, `test_provenance`, `test_refresh_data_dirs`, `test_refresh_failures`, `test_refresh_stale`, `test_retrodeck_scraper_errors`, `test_romset_recipes`, `test_scraper_contract`, `test_upstream`, `test_upstream_index` | -| Site, docs and repository contracts | rendered pages, sources | `test_contributing_text`, `test_data_dir_refs`, `test_dataclass_defaults`, `test_doc_pages`, `test_doc_surfaces`, `test_optional_dependencies`, `test_prose_style`, `test_site_availability`, `test_site_cores`, `test_site_emulator_page`, `test_site_exports`, `test_site_funnel`, `test_site_images`, `test_site_platform_hashes`, `test_site_render`, `test_site_system_links`, `test_site_validation`, `test_site_write`, `test_validate_pr`, `test_validate_pr_flow`, `test_validate_pr_hashes`, `test_workflow_paths` | +| Site, docs and repository contracts | rendered pages, sources | `test_contributing_text`, `test_coverage_cache`, `test_data_dir_refs`, `test_dataclass_defaults`, `test_doc_pages`, `test_doc_surfaces`, `test_optional_dependencies`, `test_prose_style`, `test_site_availability`, `test_site_cores`, `test_site_emulator_page`, `test_site_exports`, `test_site_funnel`, `test_site_images`, `test_site_platform_hashes`, `test_site_render`, `test_site_system_links`, `test_site_validation`, `test_site_write`, `test_validate_pr`, `test_validate_pr_flow`, `test_validate_pr_hashes`, `test_workflow_paths` | ## Test architecture