diff --git a/scripts/common.py b/scripts/common.py index 396a0f23..b3d368f9 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/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/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/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_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() 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() 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() 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."""