From 462b61a4bcbcaef48ecac3c4712f9ed4a3317bc7 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:11:37 +0100 Subject: [PATCH] refactor: compute slot choices once per manifest generate_manifest called slot_choices, and pack_overrides called it again with the same five arguments. pack_overrides takes the choices when the caller holds them. Measured on generate_pack.py --all --manifest --offline: 48 calls and 7.0 s in slot_choices before, 24 calls and 4.3 s after (wall 45.7 s and 42.4 s, one run each on a loaded host). The manifests written are identical. --- scripts/generate_pack.py | 16 +++++++++------- scripts/slots.py | 12 +++++++----- tests/test_install_selection.py | 10 ++++++++++ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 3e3c2754c..8fa8faabb 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -4609,16 +4609,18 @@ def generate_manifest( # (cache file, size). Settled once the trees are walked. cache_held: dict[str, tuple[str, int]] = {} rename_notes: set[str] = set() + # The full manifest records every core's answer at a contested slot, so a + # target narrowed from it gets what that target's own pack serves. + whole = target_cores is None and source != "platform" and bool(emu_profiles) + slot_choices = ( + slots.slot_choices(config, emu_profiles, db, zip_contents, data_registry) + if whole else {} + ) + # Derived from the same choices: computed twice, it was 8% of a run. slot_overrides = slots.pack_overrides( config, emu_profiles, db, zip_contents, data_registry, target_cores=target_cores, source=source, - ) - # The full manifest records every core's answer at a contested slot, so a - # target narrowed from it gets what that target's own pack serves. - slot_choices = ( - slots.slot_choices(config, emu_profiles, db, zip_contents, data_registry) - if target_cores is None and source != "platform" and emu_profiles - else {} + choices=slot_choices if whole else None, ) def manifest_destination(full_destination: str) -> str: diff --git a/scripts/slots.py b/scripts/slots.py index 8e7f826cd..5fac56af1 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -514,6 +514,7 @@ def pack_overrides( data_dir_registry: dict | None = None, target_cores: set[str] | None = None, source: str = "full", + choices: dict[str, list[tuple[str, str | None]]] | None = None, ) -> dict[str, str]: """Full pack destination -> the file a platform pack serves there instead. @@ -522,7 +523,8 @@ def pack_overrides( existence mode the frontend never reads the bytes, so serving the emulator's file satisfies both. The ZIP builder, the install manifest and verify all read this: two of them deciding alone gave the one-line - installer different bytes than the ZIP. + installer different bytes than the ZIP. ``choices`` is slot_choices + for the same arguments when the caller already holds it. """ # A baseline-only pack serves what the platform declares, and a target's # pack answers to the cores that hardware builds: DuckStation does not @@ -531,10 +533,10 @@ def pack_overrides( return {} cores = set(resolve_platform_cores(config, profiles, target_cores=target_cores)) overrides: dict[str, str] = {} - for destination, choices in slot_choices( - config, profiles, db, zip_contents, data_dir_registry - ).items(): - path = next((path for emulator, path in choices if emulator in cores), None) + if choices is None: + choices = slot_choices(config, profiles, db, zip_contents, data_dir_registry) + for destination, ordered in choices.items(): + path = next((path for emulator, path in ordered if emulator in cores), None) if path: overrides[destination] = path return overrides diff --git a/tests/test_install_selection.py b/tests/test_install_selection.py index 71dbd5725..94f5589c8 100644 --- a/tests/test_install_selection.py +++ b/tests/test_install_selection.py @@ -1231,6 +1231,16 @@ class AnExtraBesideItsDeclarationBelowTheBiosFolder(unittest.TestCase): ) return {m for m in record.members if m.startswith("bios/")} + def test_the_slot_choices_are_computed_once(self): + """pack_overrides recomputed the choices the manifest had just + computed with the same arguments: 8% of a manifest run.""" + import slots + + real = slots.slot_choices + with mock.patch.object(slots, "slot_choices", side_effect=real) as counted: + self._manifest(None) + self.assertEqual(counted.call_count, 1) + def test_the_pack_and_the_installer_drop_the_same_files(self): full = self._manifest(None) files, _omitted = install.narrow(