Skip to content

[master] Matchers can self-reference the loader - #70331

Open
gnagel wants to merge 8 commits into
saltstack:masterfrom
gnagel:feature/matcher-dunders
Open

gnagel wants to merge 8 commits into
saltstack:masterfrom
gnagel:feature/matcher-dunders

Conversation

@gnagel

@gnagel gnagel commented Sep 27, 2026 •

Copy link
Copy Markdown

What does this PR do?

This PR provides a significant performance optimization for matcher operations and resolves critical bugs related to context propagation within compound and nodegroup matchers.

This PR replaces and supersedes the following:

Performance Optimization:

  • Introduces the __matchers__ magic dunder and updates the loader to support it. This allows matchers to access the pre-loaded matcher registry via a high-speed $O(1)$ lookup, eliminating the need for redundant and expensive loader re-instantiations at every layer of the call stack.
  • Updates confirm_top to leverage this new __matchers__ dunder, ensuring the existing matcher registry is reused.

Bug Fixes (Context Propagation):

  • Updates salt/matchers/compound_match.py and salt/matchers/nodegroup_match.py to explicitly pass opts and minion_id through the matching chain. This ensures that engines requiring specific execution context (such as pillar data or minion identity) receive the correct parameters.

Benchmark comparison:

----------------------------------------------------------------------------------------------- benchmark: 2 tests ----------------------------------------------------------------------------------------------
Name (time in us)                    Min                   Max                  Mean              StdDev                Median                 IQR            Outliers          OPS            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_salt_matchers_dunder        14.8125 (1.0)         18.1000 (1.0)         15.6850 (1.0)        1.3846 (1.0)         15.0500 (1.0)        1.3625 (1.0)           1;0  63,755.0986 (1.0)           5          10
test_salt_loader_matchers     1,058.8750 (71.49)    1,746.5041 (96.49)    1,271.8633 (81.09)    276.5314 (199.72)   1,200.4375 (79.76)    288.2698 (211.58)        1;0     786.2480 (0.01)          5          10
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

What issues does this PR fix or reference?

Fixes: #61950

Previous Behavior

  • Performance: Matcher modules frequently re-loaded or re-instantiated the matcher registry, causing significant overhead during complex target evaluations.
  • Correctness: Compound and nodegroup matcher evaluations would lose the opts and minion_id context when evaluating nested or sub-matchers, leading to incorrect matching results in environments relying on dynamic pillar or grain data.

New Behavior

  • Performance: The __matchers__ dunder provides immediate access to the loaded matchers, caching the registry on the first pass and eliminating redundant I/O and computation.
  • Correctness: opts and minion_id are now consistently propagated through all matcher engines, ensuring predictable and accurate matching behavior.

Merge requirements satisfied?

Commits signed with GPG?

Yes

frebib and others added 4 commits September 26, 2026 11:29
This makes it so a loaded matcher doesn't have to load another instance
of the loader itself and can instead reuse the existing matchers that
are already loaded. This should speed up many matcher operations
considerably.

Signed-off-by: Joe Groocock <jgroocock@cloudflare.com>
Add changelog entry for PR saltstack#64607 and replace the now-obsolete
test_matchers_from_context test (which tested __context__ caching) with
tests that verify __matchers__ is injected and never causes recursive
salt.loader.matchers() calls.
@welcome

welcome Bot commented Sep 27, 2026

Copy link
Copy Markdown

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here's some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject.pdl@broadcom.com. We're glad you've joined our community and look forward to doing awesome things with you!

@gnagel
gnagel marked this pull request as ready for review September 27, 2026 19:19
@gnagel
gnagel requested a review from a team as a code owner September 27, 2026 19:19

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

top_matches slows down with every match and takes a lot of time with big pillars (slow deepcopy)

3 participants