From dc772d6dc7239bdcbe33ed449ffd4af6b210fe4e Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 26 Sep 2026 00:52:32 +0200 Subject: [PATCH] perf(applications): memoise the payload a render re-enters for get_merged_applications writes its cache in the last line of the function, and the re-entrancy guard returns above it. Every lookup('applications') a template makes while the payload is being rendered therefore rebuilt it: get_application_defaults deep-copies roughly two hundred roles, then the merge runs again. Profiled over one render of tests/integration/roles/test-e2e-cli: 382 calls, 18.765s of 29s, 13.7 million deepcopy calls. That test spent 208s in setUp for twenty renders and now spends about half. The guarded payload cannot go into _MERGED_APPLICATIONS_CACHE, because it carries no users and still holds its volumes. It is memoised on the render guard instead, which is thread-local, and the finally that releases the guard drops it, so it lives exactly one render. Three probes established the preconditions before the change: one cache key across all 381 nested calls, re-entrancy depth never above 2, and not one of the 381 payloads mutated after it was handed out. That last one was then re-measured across the whole suite with the memo made self-checking: 6861 tests and 5133 subtests, no caller wrote to what it was given. 113113e553 removed the same cost between tasks through the carrier fact. This removes what remains inside a single render. Co-Authored-By: Claude Opus 5 (1M context) --- .../python/utils/cache/test_applications.py | 51 +++++++++++++++++++ utils/cache/applications.py | 21 +++++++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/tests/unit/python/utils/cache/test_applications.py b/tests/unit/python/utils/cache/test_applications.py index b62eff677a..3ebae9ce25 100644 --- a/tests/unit/python/utils/cache/test_applications.py +++ b/tests/unit/python/utils/cache/test_applications.py @@ -652,5 +652,56 @@ class TestDeriveMcpPresence(unittest.TestCase): self.assertIs(variants[1]["mcp"]["enabled"], False) +class TestGetMergedApplicationsMemoisesTheGuardedPath(unittest.TestCase): + """A re-entrant call returns before the main cache is written, because its + payload carries no users and keeps its volumes. It is memoised on the + render guard instead, or one template render rebuilds the payload for + every ``lookup('applications')`` it makes. + """ + + def setUp(self) -> None: + _reset_cache_for_tests() + cache_apps._RENDER_GUARD.applications = False + cache_apps._RENDER_GUARD.nested_payloads = {} + self.addCleanup(setattr, cache_apps._RENDER_GUARD, "applications", False) + self.addCleanup(setattr, cache_apps._RENDER_GUARD, "nested_payloads", {}) + + def _merged(self, roles): + return cache_apps.get_merged_applications( + variables={}, roles_dir=roles, templar=None + ) + + def test_a_re_entrant_call_serves_the_payload_it_already_built(self): + with tempfile.TemporaryDirectory() as tmp: + roles = _seed_minimal_roles(Path(tmp)) + cache_apps._RENDER_GUARD.applications = True + + first = self._merged(roles) + second = self._merged(roles) + + self.assertIs(first, second) + + def test_the_memo_does_not_outlive_the_guard(self): + with tempfile.TemporaryDirectory() as tmp: + roles = _seed_minimal_roles(Path(tmp)) + cache_apps._RENDER_GUARD.applications = True + guarded = self._merged(roles) + + cache_apps._RENDER_GUARD.applications = False + cache_apps._RENDER_GUARD.nested_payloads = {} + _reset_cache_for_tests() + after = self._merged(roles) + + self.assertIsNot(guarded, after) + + def test_the_guarded_payload_never_reaches_the_main_cache(self): + with tempfile.TemporaryDirectory() as tmp: + roles = _seed_minimal_roles(Path(tmp)) + cache_apps._RENDER_GUARD.applications = True + self._merged(roles) + + self.assertEqual(cache_apps._MERGED_APPLICATIONS_CACHE, {}) + + if __name__ == "__main__": unittest.main() diff --git a/utils/cache/applications.py b/utils/cache/applications.py index 68dc5e4823..53bd526813 100644 --- a/utils/cache/applications.py +++ b/utils/cache/applications.py @@ -392,6 +392,14 @@ def get_merged_applications( roles_dir: str | os.PathLike[str] | None = None, templar: Any = None, ) -> dict[str, Any]: + """Return the applications payload for *variables*, cached per key. + + Exception: a re-entrant call returns before ``_MERGED_APPLICATIONS_CACHE`` + is written, because its payload carries no users and keeps its volumes. It + is therefore memoised on the render guard instead, which drops it when the + guard releases. Without that, one template render recomputed the payload + for every ``lookup('applications')`` it made. + """ from .users import get_merged_users variables = variables or {} @@ -406,13 +414,23 @@ def get_merged_applications( _MERGED_APPLICATIONS_CACHE[cache_key] = carried return carried + nested = getattr(_RENDER_GUARD, "applications", False) + if nested: + carried_nested = getattr(_RENDER_GUARD, "nested_payloads", {}).get(cache_key) + if carried_nested is not None: + return carried_nested + defaults = get_application_defaults(roles_dir=resolved_roles_dir) overrides = _resolve_override_mapping(variables, "applications", templar=templar) merged = merge_with_defaults(defaults, overrides) - if getattr(_RENDER_GUARD, "applications", False): + if nested: + payloads = getattr(_RENDER_GUARD, "nested_payloads", None) + if payloads is None: + payloads = _RENDER_GUARD.nested_payloads = {} + payloads[cache_key] = merged return merged for app_cfg in merged.values(): @@ -435,6 +453,7 @@ def get_merged_applications( ) finally: _RENDER_GUARD.applications = False + _RENDER_GUARD.nested_payloads = {} for app_id in rendered: raw_volumes = get_canonical_volumes(app_id)