mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-09 20:26:33 +00:00
perf(cache): let the sweep read the variants map without copying it
get_variants handed out a deep copy on every call so one caller's
mutation could not reach the next. services_overrides_for_round asks
for the map once per round, and a sweep resolves hundreds of rounds, so
the matrix planner paid that copy 460 times: 78 million deepcopy calls,
and get_variants alone accounting for 82 of the 163 seconds a profiled
build took.
The copy is now opt-out for that one caller. It reads the map and never
writes to it, resolve_pins takes it as a Mapping and has no mutation
site, and _deep_merge deep-copies both of its arguments, so nothing the
round produces keeps a reference into the cache. Fifteen other callers
keep the copy untouched.
Measured end to end on cli.meta.ci.playwright_coverage: 33.7s to 24.6s.
Equivalence is measured against the copying path for two fixed rounds,
291 roles each, rather than against a sweep -- two identical planner
runs assign different modes to some rows, so a plan diff cannot carry
that proof.
The test asserts identity rather than runtime, as the comparable
optimization in 0cb8bae88d did: a wall-clock bound on this machine is
the same flakiness the sweep already shows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
3a58067613
commit
e75ccc07cd
3 files changed
+49
-4
No files matched your search
+36
@@ -294,5 +294,41 @@ class TestMergedApplicationsUsesBaseConfig(unittest.TestCase):
|
||||
)
|
||||
|
||||
|
||||
class TestReadonlyVariants(unittest.TestCase):
|
||||
"""``readonly=True`` hands out the cached map itself.
|
||||
|
||||
A sweep resolves one round per matrix row and asks for the variants in
|
||||
each, so the defensive deep copy is paid hundreds of times for data
|
||||
nobody writes to. The guarantee is cheap to lose silently in a later
|
||||
refactor, which is why identity is asserted rather than runtime.
|
||||
"""
|
||||
|
||||
def setUp(self) -> None:
|
||||
_reset_cache_for_tests()
|
||||
self.tmp = Path(os.environ.get("TMPDIR", "/tmp")) / "readonly-variants-roles"
|
||||
_write_role(
|
||||
self.tmp,
|
||||
"web-app-one",
|
||||
config="postgres:\n enabled: true\n",
|
||||
meta="- services:\n postgres:\n enabled: false\n",
|
||||
)
|
||||
|
||||
def test_the_default_hands_out_a_copy(self) -> None:
|
||||
first = get_variants(roles_dir=str(self.tmp))
|
||||
second = get_variants(roles_dir=str(self.tmp))
|
||||
self.assertEqual(first, second)
|
||||
self.assertIsNot(first, second)
|
||||
|
||||
def test_readonly_hands_out_the_cached_object(self) -> None:
|
||||
first = get_variants(roles_dir=str(self.tmp), readonly=True)
|
||||
second = get_variants(roles_dir=str(self.tmp), readonly=True)
|
||||
self.assertIs(first, second)
|
||||
|
||||
def test_a_readonly_caller_sees_what_the_copy_would_have_shown(self) -> None:
|
||||
shared = get_variants(roles_dir=str(self.tmp), readonly=True)
|
||||
copied = get_variants(roles_dir=str(self.tmp))
|
||||
self.assertEqual(shared, copied)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user