From 585be38ee3b97c2b9a52216c36ac9ac7efb191ea Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 09:56:53 +0200 Subject: [PATCH 1/4] fix: transform each held content once Step 1b read 18 924 held files and members for 7 407 distinct contents and ran every transform on each read. A content already transformed is now skipped (the first path still keeps the match), the power-of-two crc shift operators are squared once for the pass instead of once per length, and a group pairs members by the sizes it holds. derivable.json and the step output are byte-identical, 33.4 s to 25.1 s. --- scripts/derivations.py | 77 +++++++++++++++++++++++---------------- tests/test_derivations.py | 43 ++++++++++++++++++++++ 2 files changed, 89 insertions(+), 31 deletions(-) diff --git a/scripts/derivations.py b/scripts/derivations.py index 4a71b91e..5be36596 100644 --- a/scripts/derivations.py +++ b/scripts/derivations.py @@ -82,36 +82,36 @@ def crc32_combine(crc1: int, crc2: int, len2: int) -> int: return _gf2_times(_shift_operator(len2), crc1) ^ crc2 +@functools.lru_cache(maxsize=None) +def _byte_shift(power: int) -> tuple[int, ...]: + """The operator for 2**power zero bytes appended, as zlib squares it: + three squarings of the one-bit operator give one byte, and each power + is the square of the one before.""" + if power: + return tuple(_gf2_square(list(_byte_shift(power - 1)))) + operator = [0xEDB88320] + [1 << (n - 1) for n in range(1, 32)] + for _ in range(3): + operator = _gf2_square(operator) + return tuple(operator) + + @functools.lru_cache(maxsize=None) def _shift_operator(len2: int) -> tuple[int, ...]: """The linear map zlib applies to crc(A) for len2 bytes appended. - Built once per length, as zlib's crc32_combine walks it: the chain of - squared operators depends on len2 alone, so it is squared once and the - operators the set bits select are composed. Rebuilding the chain for - each of the 32 columns spent 57 s of the provenance step in GF(2) - products. + The product of the power-of-two operators the set bits of len2 select. + Those powers are shared by every length, so each is squared once for the + whole pass: squaring the chain again for each of the 3 600 lengths a + romset pairs spent most of the crc work in GF(2) products. """ - odd = [0xEDB88320] + [1 << (n - 1) for n in range(1, 32)] - even = _gf2_square(odd) - odd = _gf2_square(even) - selected: list[list[int]] = [] - while True: - even = _gf2_square(odd) - if len2 & 1: - selected.append(even) - len2 >>= 1 - if not len2: - break - odd = _gf2_square(even) - if len2 & 1: - selected.append(odd) - len2 >>= 1 - if not len2: - break columns = [1 << bit for bit in range(32)] - for matrix in selected: - columns = [_gf2_times(matrix, column) for column in columns] + power = 0 + while len2: + if len2 & 1: + matrix = _byte_shift(power) + columns = [_gf2_times(matrix, column) for column in columns] + len2 >>= 1 + power += 1 return tuple(columns) @@ -201,6 +201,10 @@ def _derivable( found: dict[str, dict] = {} files = _held_sources(db) + # The same chip sits in many archives. Its transforms are a function of + # its bytes alone, and the first path read keeps the match, so a content + # already transformed has nothing left to find. + transformed: set[str] = set() for held in files: n = held["size"] useful = [t for t in TRANSFORMS if t[1](n) in sizes] @@ -209,31 +213,42 @@ def _derivable( data = _read(held, archives) if data is None: continue + content = hashlib.sha1(data).hexdigest() + if content in transformed: + continue + transformed.add(content) for name, out_size, transform in useful: out = transform(data) sha1 = wanted.get((out_size(n), hashlib.sha1(out).hexdigest())) if sha1 and sha1 not in found: found[sha1] = {"from": [held["path"]], "transform": name} + # Romsets repeat the same chip pairs: a combined crc is computed once. + combined: dict[tuple[str, str, int], int] = {} by_dir: dict[str, list[dict]] = defaultdict(list) for held in files: if held.get("crc32"): by_dir[held["group"]].append(held) for group in by_dir.values(): # A pair is only worth a crc when its sizes add up to a wanted one: - # indexed by size, a group is walked once per wanted size instead - # of once per member, and a romset of a few hundred chips stays - # linear. + # indexed by size, a member is paired once per size the group + # holds, and a romset of a few hundred chips stays linear. by_size: dict[int, list[dict]] = defaultdict(list) for held in group: by_size[held["size"]].append(held) for first in group: - for total in sizes: - for second in by_size.get(total - first["size"], ()): + for second_size, seconds in by_size.items(): + total = first["size"] + second_size + if total not in sizes: + continue + for second in seconds: if first is second: continue - crc = crc32_combine(int(first["crc32"], 16), - int(second["crc32"], 16), second["size"]) + key = (first["crc32"], second["crc32"], second["size"]) + crc = combined.get(key) + if crc is None: + crc = combined[key] = crc32_combine( + int(first["crc32"], 16), int(second["crc32"], 16), second["size"]) candidates = [s for s in by_size_crc.get((total, crc), []) if s not in found] if not candidates: diff --git a/tests/test_derivations.py b/tests/test_derivations.py index a346224e..580bf32e 100644 --- a/tests/test_derivations.py +++ b/tests/test_derivations.py @@ -13,6 +13,7 @@ import shutil import sys import tempfile import unittest +import unittest.mock import zlib from pathlib import Path @@ -103,6 +104,48 @@ class ARomsetMemberIsASource(unittest.TestCase): +class OneContentIsTransformedOnce(unittest.TestCase): + """A MAME chip sits in many archives: 18 924 reads for 7 407 contents, and + each read went through every transform again.""" + + def test_a_repeated_chip_is_transformed_once_and_keeps_its_first_path(self): + import zipfile + + root = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp")) + self.addCleanup(shutil.rmtree, root, True) + chip = os.urandom(64) + files = {} + for name in ("a.zip", "b.zip", "c.zip"): + archive = root / name + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("chip.rom", chip) + files[name] = {"path": str(archive), "size": archive.stat().st_size} + calls = [] + + def counted(transform): + def run(data): + calls.append(data) + return transform(data) + return run + + transforms = tuple((name, size, counted(fn)) for name, size, fn in derivations.TRANSFORMS) + swapped = derivations._swap16(chip) + with unittest.mock.patch.object(derivations, "TRANSFORMS", transforms): + found = derivations.derivable([_wanted(swapped)], {"files": files}) + self.assertEqual(found[hashlib.sha1(swapped).hexdigest()]["from"], + [f"{root / 'a.zip'}/chip.rom"]) + useful = [t for t in transforms if t[1](len(chip)) == len(chip)] + self.assertEqual(len(calls), len(useful)) + + def test_the_shared_power_operators_compose_to_zlib_for_every_bit(self): + a = os.urandom(17) + for size in [1 << bit for bit in range(25)] + [(1 << 25) - 1, 3624, 5]: + with self.subTest(size=size): + b = os.urandom(size) + self.assertEqual(derivations.crc32_combine(zlib.crc32(a), zlib.crc32(b), size), + zlib.crc32(b, zlib.crc32(a))) + + class CombineMatchesZlib(unittest.TestCase): """The operator is composed once per length, and it must stay zlib's.""" From 11c836cdeedef8916f27c337d3f10376a0ce5b04 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:00:53 +0200 Subject: [PATCH 2/4] fix: hash only same-size files in dedup scan Equal files have equal sizes, and the scan only uses the sha1. Files alone at their size are no longer read and the rest get one digest instead of five. Same 278 groups (645 paths), same dry-run output, 36.7 s to 1.9 s. --- scripts/dedup.py | 20 +++++++++++++++----- tests/test_dedup.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/scripts/dedup.py b/scripts/dedup.py index 7c7f9a2f..6c38b7cf 100644 --- a/scripts/dedup.py +++ b/scripts/dedup.py @@ -97,17 +97,27 @@ def _is_mame_dir(path: str) -> bool: def scan_duplicates(bios_dir: str) -> dict[str, list[str]]: - """Find all files grouped by SHA1, excluding no-dedup directories.""" - sha1_to_paths: dict[str, list[str]] = defaultdict(list) + """Files grouped by SHA1, excluding no-dedup directories. - for root, dirs, files in os.walk(bios_dir): + Two equal files have the same size, so only files that share a size are + read, and for the sha1 alone: hashing the whole tree with every digest + took 33 s for a scan that reads a quarter of it. A file alone at its + size is in no group of two and is left out. + """ + by_size: dict[int, list[str]] = defaultdict(list) + for root, _dirs, files in os.walk(bios_dir): for name in files: path = os.path.join(root, name) if _in_nodedup_dir(path): continue - sha1 = compute_hashes(path)["sha1"] - sha1_to_paths[sha1].append(path) + by_size[os.path.getsize(path)].append(path) + sha1_to_paths: dict[str, list[str]] = defaultdict(list) + for paths in by_size.values(): + if len(paths) < 2: + continue + for path in paths: + sha1_to_paths[compute_hashes(path, frozenset({"sha1"}))["sha1"]].append(path) return sha1_to_paths diff --git a/tests/test_dedup.py b/tests/test_dedup.py index 5bab6a64..02a1207d 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -309,5 +309,37 @@ class TheNoDedupListIsPinned(unittest.TestCase): self.assertEqual(expected - set(NODEDUP_DIRS), set()) +class OnlyFilesSharingASizeAreHashed(_Bios): + """Equal files have equal sizes. Hashing all 9 442 files with five + digests took 33 s for 278 groups that a sha1 of the 4 543 files sharing + a size finds the same.""" + + def test_a_file_alone_at_its_size_is_not_read(self): + from unittest import mock + + self.write("A/one.bin") + self.write("B/one.bin") + self.write("C/other.bin", b"a payload of its own length") + self.write("D/same_size.bin", b"IDENTICAL PAYLOAX") + calls: list[tuple[str, frozenset | None]] = [] + real = dedup.compute_hashes + + def spy(path, algorithms=None, cache=True): + calls.append((os.path.relpath(path, self.bios), algorithms)) + return real(path, algorithms, cache) + + with mock.patch.object(dedup, "compute_hashes", spy): + groups = dedup.scan_duplicates(str(self.bios)) + self.assertEqual( + sorted(path for path, _algos in calls), + sorted(os.path.join(*p.split("/")) for p in ("A/one.bin", "B/one.bin", "D/same_size.bin")), + ) + self.assertTrue(all(algos == frozenset({"sha1"}) for _path, algos in calls)) + multi = [paths for paths in groups.values() if len(paths) > 1] + self.assertEqual(len(multi), 1) + self.assertEqual(sorted(os.path.relpath(p, self.bios) for p in multi[0]), + sorted(os.path.join(*p.split("/")) for p in ("A/one.bin", "B/one.bin"))) + + if __name__ == "__main__": unittest.main() From d904116a79610aaeaf29853975690c2536db42e8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:03:27 +0200 Subject: [PATCH 3/4] fix: verify each 3DS file once per content check_crypto_validation keeps its answer keyed by the file and keys file as compute_hashes keys them (path, inode, size, mtime, ctime). verify --all ran the verifiers 35 times for 5 files (0.88 s), now 5 times (0.13 s). verify --all --json output is byte-identical. --- scripts/crypto_verify.py | 28 +++++++++++++++++++++++--- tests/test_crypto_verify.py | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/scripts/crypto_verify.py b/scripts/crypto_verify.py index 081ee2b5..efe75406 100644 --- a/scripts/crypto_verify.py +++ b/scripts/crypto_verify.py @@ -490,6 +490,23 @@ _CRYPTO_VERIFIERS: dict[str, Callable] = { } +def _stat_key(path: str | Path) -> tuple | None: + """(path, inode, size, mtime_ns, ctime_ns): the file as compute_hashes + keys it, None when it cannot be read.""" + try: + stat = Path(path).stat() + except OSError: + return None + return (str(Path(path).resolve()), stat.st_ino, stat.st_size, + stat.st_mtime_ns, stat.st_ctime_ns) + + +# (verifier, file key, keys file key) -> reason or None. verify --all ran the +# curve and RSA checks of the same five files once per platform, 35 runs for +# 5 answers: the answer depends on the file's bytes and the keys file alone. +_crypto_cache: dict[tuple, str | None] = {} + + def check_crypto_validation( local_path: str, filename: str, @@ -508,8 +525,13 @@ def check_crypto_validation( if not keys_file: return "crypto check skipped: no keys file found" + file_key, keys_key = _stat_key(local_path), _stat_key(keys_file) + key = (filename, file_key, keys_key) if file_key and keys_key else None + if key in _crypto_cache: + return _crypto_cache[key] keys = parse_keys_file(keys_file) valid, reason = verifier(local_path, keys) - if valid: - return None - return reason + result = None if valid else reason + if key is not None: + _crypto_cache[key] = result + return result diff --git a/tests/test_crypto_verify.py b/tests/test_crypto_verify.py index 14b563e5..53e8e1ab 100644 --- a/tests/test_crypto_verify.py +++ b/tests/test_crypto_verify.py @@ -19,6 +19,7 @@ import shutil import sys import tempfile import unittest +import unittest.mock from pathlib import Path REPO_ROOT = Path(__file__).resolve().parent.parent @@ -329,5 +330,44 @@ class KeysFile(unittest.TestCase): self.assertIsNone(crypto_verify.find_keys_file(self.tmp)) +class OneFileIsVerifiedOncePerContent(unittest.TestCase): + """verify --all ran the 3DS checks of the same five files once per + platform: 35 runs of pure-Python curve and RSA work for 5 answers.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.bios = Path(self._tmp.name) + target = self.bios / "Nintendo" / "3DS" + target.mkdir(parents=True) + (target / "aes_keys.txt").write_text(":AES\n") + self.file = target / "otp.bin" + self.file.write_bytes(b"\x00" * 0x100) + self.calls: list[bytes] = [] + + def verifier(path, keys): + data = Path(path).read_bytes() + self.calls.append(data) + return data[0] == 1, "first byte is not 1" + + patcher = unittest.mock.patch.dict(crypto_verify._CRYPTO_VERIFIERS, {"otp.bin": verifier}) + patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(crypto_verify._crypto_cache.clear) + self.addCleanup(self._tmp.cleanup) + + def check(self) -> str | None: + return crypto_verify.check_crypto_validation(str(self.file), "otp.bin", str(self.bios)) + + def test_the_same_file_is_verified_once(self): + self.assertEqual([self.check() for _ in range(7)], ["first byte is not 1"] * 7) + self.assertEqual(len(self.calls), 1) + + def test_a_rewritten_file_is_verified_again(self): + self.assertEqual(self.check(), "first byte is not 1") + self.file.write_bytes(b"\x01" + b"\x00" * 0x1FF) + self.assertIsNone(self.check()) + self.assertEqual(len(self.calls), 2) + + if __name__ == "__main__": unittest.main() From 78e172fd5f9823a6c1daef1f2177396b1bcdf97e Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:08:25 +0200 Subject: [PATCH 4/4] fix: index archive members once per profile archive_members scanned the whole file list on every call, and callers ask once per entry of the same profile: 38 million entries walked in one verify --all over mame.yml. Lists of 64 entries or more are now indexed by archive once (held so the id is not reused, indexed again when the length changes), members still read from the entries at each call. archive_members drops from 2.30 s to 0.14 s tottime under the profiler. verify --all (text and --json), slots --json and the retroarch manifest are byte-identical. --- scripts/common.py | 29 +++++++++++++++++++---- tests/test_archive_choice.py | 45 ++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/scripts/common.py b/scripts/common.py index 35b53465..249b65a2 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -1018,13 +1018,32 @@ def _rank_tail_candidates( ) +# id(files) -> (files, len(files), {archive: entries with a crc32}). Callers +# ask once per entry of the same profile: scanning the list each time walked +# 38 million entries in one verify --all over mame.yml. The list is held so +# its id is not reused, a list that changed length is indexed again, and the +# members are read from the entries at each call. Short lists are scanned +# directly: a fresh default [] per call would only fill the cache. +_ARCHIVE_INDEX_MIN = 64 +_archive_index: dict[int, tuple[list[dict], int, dict[str | None, list[dict]]]] = {} + + +def _member(f: dict) -> dict: + return {"crc32": str(f["crc32"]).lower(), "required": bool(f.get("required", True))} + + def archive_members(archive: str, files: list[dict]) -> list[dict]: """The members a profile reads inside *archive*, with what it requires.""" - return [ - {"crc32": str(f["crc32"]).lower(), "required": bool(f.get("required", True))} - for f in files - if f.get("archive") == archive and f.get("crc32") - ] + if len(files) < _ARCHIVE_INDEX_MIN: + return [_member(f) for f in files if f.get("archive") == archive and f.get("crc32")] + cached = _archive_index.get(id(files)) + if cached is None or cached[0] is not files or cached[1] != len(files): + index: dict[str | None, list[dict]] = {} + for f in files: + if f.get("crc32"): + index.setdefault(f.get("archive"), []).append(f) + cached = _archive_index[id(files)] = (files, len(files), index) + return [_member(f) for f in cached[2].get(archive, ())] def archive_entry(archive: str, files: list[dict]) -> dict: diff --git a/tests/test_archive_choice.py b/tests/test_archive_choice.py index 046742d3..039d11d2 100644 --- a/tests/test_archive_choice.py +++ b/tests/test_archive_choice.py @@ -409,5 +409,50 @@ class ALooseEntryNamingItsArchive(unittest.TestCase): self.assertEqual(report["total_files"], 1) +class ArchiveMembersIndexOnce(unittest.TestCase): + """with_archive_members asks once per entry of a profile: scanning the + whole list for each walked 38 million entries in one verify --all.""" + + class CountingList(list): + walks = 0 + + def __iter__(self): + type(self).walks += 1 + return super().__iter__() + + def entries(self) -> list[dict]: + files = self.CountingList() + self.CountingList.walks = 0 + for n in range(300): + files.append({"name": f"m{n}.rom", "archive": f"set{n % 30}.zip", + "crc32": f"{n:08X}", "required": n % 3 != 0}) + files.append({"name": "loose.bin", "crc32": "deadbeef"}) + files.append({"name": "nocrc.rom", "archive": "set0.zip"}) + return files + + @staticmethod + def scanned(archive, files) -> list[dict]: + return [{"crc32": str(f["crc32"]).lower(), "required": bool(f.get("required", True))} + for f in list.__iter__(files) if f.get("archive") == archive and f.get("crc32")] + + def test_a_profile_list_is_walked_once_and_answers_as_a_scan(self): + from common import archive_members + + files = self.entries() + for archive in [f"set{n}.zip" for n in range(30)] + ["absent.zip", "", None]: + with self.subTest(archive=archive): + self.assertEqual(archive_members(archive, files), self.scanned(archive, files)) + self.assertEqual(self.CountingList.walks, 1) + + def test_a_list_that_grew_is_indexed_again(self): + from common import archive_members + + files = self.entries() + archive_members("set1.zip", files) + files.append({"name": "late.rom", "archive": "set1.zip", "crc32": "ABCDEF01"}) + self.assertEqual(archive_members("set1.zip", files), self.scanned("set1.zip", files)) + self.assertIn({"crc32": "abcdef01", "required": True}, archive_members("set1.zip", files)) + + if __name__ == "__main__": unittest.main()