Memoize SaltStackVersion construction in warn_until() - #69924
Merged
Conversation
warn_until() built two fresh SaltStackVersion instances per call — one
for the target-version comparison and one for the running version.
SaltStackVersion.__init__ transitively allocates a packaging.version.
Version. Under stress the master EventPublisher was allocating ~1.4M
Version objects (2.5 GB of transient allocation churn) per 90 s
window; on hot deprecation-warning paths (TCPPubClient, TCPReqServer,
MessageClient) this drove Python's arena high-water mark and pinned
process RSS above its actual working set.
Wrap both constructions in functools.lru_cache-backed helpers:
_resolve_target_version_hashable — 32-slot cache keyed on the
common hashable inputs (int, tuple, str); returns None for
non-hashable input so the caller falls through to the inline
path unchanged
_resolve_current_version — 8-slot cache keyed on the
version_info tuple; effectively a one-time construction for a
given process
After the patch a 10 000-call warn_until loop makes 0 SaltStackVersion
constructions (100% reduction on the repeated-argument path).
WebSocket-transport master EventPublisher RSS peak: 271 MB -> 214 MB
(-57 MB / -21%) under 30 min stress.
dwoz
force-pushed
the
dwoz/perf/warn-until-memoize-3008x
branch
from
August 3, 2026 21:13
6dc4831 to
b8f6070
Compare
sujitdb
reviewed
Aug 3, 2026
twangboy
previously approved these changes
Aug 3, 2026
``SaltVersion`` is a ``namedtuple`` subclass, so it matches ``isinstance(version, tuple)`` and hits the ``lru_cache`` fast path added in saltstack#69921. But it defines ``__eq__`` without ``__hash__``, which makes it unhashable — ``hash(version)`` raises ``TypeError`` before the cache lookup can even start. On top of that, its tuple form is ``(name, info, released)``, not version parts, so even if it were hashable the resolver would build the wrong ``SaltStackVersion``. Route ``SaltVersion`` explicitly to its own branch before the fast path and add a regression test that guards it. Fixes CI failure in ``test_warn_until_good_version_argument[version3]``.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #69921
Summary
SaltStackVersionconstructions insidesalt.utils.versions.warn_until()withfunctools.lru_cache-backed helpers so hot paths that callwarn_until()per event (deprecatedTCPPubClient/TCPReqServer/MessageClientaliases) stop allocating freshSaltStackVersion(and, transitively,packaging.version.Version) objects on every callSaltVersion,SaltStackVersioninstances) is unchanged; only the common hashable inputs (int,tuple,str) go through the cacheRuntimeErroron unknown release names, same fall-through logicMeasured impact
memray on the master's
EventPublisherunder 4h stress showed ~1.4 Mpackaging.version.Versionallocations totalling 2.5 GB in a 90 s window; the counting test in the new suite confirms 100% reduction on the repeated-argument path (10 000warn_until(3009, ...)calls → 0SaltStackVersionconstructions after cache warmup).Per-process RSS impact on the WebSocket-transport master, 30 min stress:
Test plan
pytest tests/pytests/unit/utils/test_versions_warn_until_cache.py— 9 tests, all passing locallyRuntimeErrorwith the "Incorrect spelling" message