[BUG] match function calls are slow at scale

[nicholasmhughes]

Description

While troubleshooting a workflow that included a lot of salt.match.compound() calls in Jinja, it was noticed that the function call contributed quite a bit of time to the overall workflow duration. Upon further investigation, it was found that every function call in the match module is invoking the Salt Loader on every run. To make matters worse, the compound matcher is invoking the Salt Loader again to pull in each distinct match type... so a call to compound matching invokes the Salt Loader twice on every call.

Setup

This simple script was used to test the compound match function calls at scale.

import argparse
import timeit

import salt.config
import salt.matchers.compound_match


def main() -> None:
    parser = argparse.ArgumentParser()
    parser.add_argument("-n", type=int, default=1000)
    args = parser.parse_args()

    __opts__ = salt.config.minion_config("/etc/salt/minion")

    salt.matchers.compound_match.__opts__ = __opts__

    print(
        timeit.timeit(
            lambda: salt.matchers.compound_match.match("G@foo:bar"),
            number=args.n,
        )
    )


if __name__ == "__main__":
    main()

Steps to Reproduce the behavior

Running the script above results in fairly slow operation at scale.

$ python match_test.py -n 100000
67.60197884199442

Expected behavior

Changing to direct imports of the matchers yields much faster results.

$ python match_test.py -n 100000
6.912001780990977

Direct import diff for reference.

diff --git a/salt/matchers/compound_match.py b/salt/matchers/compound_match.py
index 40ef2b03fa..bf82f3a531 100644
--- a/salt/matchers/compound_match.py
+++ b/salt/matchers/compound_match.py
@@ -7,6 +7,19 @@ import logging
 import salt.loader
 import salt.utils.minions
 
+# Instead of letting this hit the lazy loader, just statically import them.
+# This is _significantly_ faster, a good 10x in a simple benchmark.
+from salt.matchers import (
+    glob_match,
+    grain_match,
+    grain_pcre_match,
+    ipcidr_match,
+    list_match,
+    pcre_match,
+    pillar_match,
+    pillar_pcre_match,
+)
+
 HAS_RANGE = False
 try:
     import seco.range  # pylint: disable=unused-import
@@ -25,7 +38,6 @@ def match(tgt, opts=None, minion_id=None):
     if not opts:
         opts = __opts__
     nodegroups = opts.get("nodegroups", {})
-    matchers = salt.loader.matchers(opts)
     if not minion_id:
         minion_id = opts.get("id")
 
@@ -34,14 +46,14 @@ def match(tgt, opts=None, minion_id=None):
         return False
     log.debug("compound_match: %s ? %s", minion_id, tgt)
     ref = {
-        "G": "grain",
-        "P": "grain_pcre",
-        "I": "pillar",
-        "J": "pillar_pcre",
-        "L": "list",
+        "G": grain_match.match,
+        "P": grain_pcre_match.match,
+        "I": pillar_match.match,
+        "J": pillar_pcre_match.match,
+        "L": list_match.match,
         "N": None,  # Nodegroups should already be expanded
-        "S": "ipcidr",
-        "E": "pcre",
+        "S": ipcidr_match.match,
+        "E": pcre_match.match,
     }
     if HAS_RANGE:
         ref["R"] = "range"
@@ -101,17 +113,11 @@ def match(tgt, opts=None, minion_id=None):
             if target_info["delimiter"]:
                 engine_kwargs["delimiter"] = target_info["delimiter"]
 
-            results.append(
-                str(
-                    matchers["{}_match.match".format(engine)](
-                        *engine_args, **engine_kwargs
-                    )
-                )
-            )
+            results.append(str(engine(*engine_args, **engine_kwargs)))
 
         else:
             # The match is not explicitly defined, evaluate it as a glob
-            results.append(str(matchers["glob_match.match"](word, opts, minion_id)))
+            results.append(str(glob_match.match(word, opts, minion_id)))
 
     results = " ".join(results)
     log.debug('compound_match %s ? "%s" => "%s"', minion_id, tgt, results)
diff --git a/salt/modules/match.py b/salt/modules/match.py
index 878ab35436..329858881e 100644
--- a/salt/modules/match.py
+++ b/salt/modules/match.py
@@ -8,9 +8,21 @@ import logging
 import sys
 from collections.abc import Mapping
 
-import salt.loader
+import salt.utils.dictupdate
 from salt.defaults import DEFAULT_TARGET_DELIM
 from salt.exceptions import SaltException
+from salt.matchers import (
+    compound_match,
+    data_match,
+    glob_match,
+    grain_match,
+    grain_pcre_match,
+    ipcidr_match,
+    list_match,
+    pcre_match,
+    pillar_match,
+    pillar_pcre_match,
+)
 
 __func_alias__ = {"list_": "list"}
 
@@ -35,9 +47,8 @@ def compound(tgt, minion_id=None):
     if minion_id is not None:
         if not isinstance(minion_id, str):
             minion_id = str(minion_id)
-    matchers = salt.loader.matchers(__opts__)
     try:
-        ret = matchers["compound_match.match"](tgt, opts=__opts__, minion_id=minion_id)
+        ret = compound_match.match(tgt, opts=__opts__, minion_id=minion_id)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         ret = False
@@ -65,9 +76,8 @@ def ipcidr(tgt):
          - nodeclass: internal
 
     """
-    matchers = salt.loader.matchers(__opts__)
     try:
-        return matchers["ipcidr_match.match"](tgt, opts=__opts__)
+        return ipcidr_match.match(tgt, opts=__opts__)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False
@@ -96,11 +106,8 @@ def pillar_pcre(tgt, delimiter=DEFAULT_TARGET_DELIM):
         .. versionadded:: 0.16.4
         .. deprecated:: 2015.8.0
     """
-    matchers = salt.loader.matchers(__opts__)
     try:
-        return matchers["pillar_pcre_match.match"](
-            tgt, delimiter=delimiter, opts=__opts__
-        )
+        return pillar_pcre_match.match(tgt, delimiter=delimiter, opts=__opts__)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False
@@ -129,9 +136,8 @@ def pillar(tgt, delimiter=DEFAULT_TARGET_DELIM):
         .. versionadded:: 0.16.4
         .. deprecated:: 2015.8.0
     """
-    matchers = salt.loader.matchers(__opts__)
     try:
-        return matchers["pillar_match.match"](tgt, delimiter=delimiter, opts=__opts__)
+        return pillar_match.match(tgt, delimiter=delimiter, opts=__opts__)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False
@@ -147,9 +153,8 @@ def data(tgt):
 
         salt '*' match.data 'spam:eggs'
     """
-    matchers = salt.loader.matchers(__opts__)
     try:
-        return matchers["data_match.match"](tgt, opts=__opts__)
+        return data_match.match(tgt, opts=__opts__)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False
@@ -178,11 +183,8 @@ def grain_pcre(tgt, delimiter=DEFAULT_TARGET_DELIM):
         .. versionadded:: 0.16.4
         .. deprecated:: 2015.8.0
     """
-    matchers = salt.loader.matchers(__opts__)
     try:
-        return matchers["grain_pcre_match.match"](
-            tgt, delimiter=delimiter, opts=__opts__
-        )
+        return grain_pcre_match.match(tgt, delimiter=delimiter, opts=__opts__)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False
@@ -211,9 +213,8 @@ def grain(tgt, delimiter=DEFAULT_TARGET_DELIM):
         .. versionadded:: 0.16.4
         .. deprecated:: 2015.8.0
     """
-    matchers = salt.loader.matchers(__opts__)
     try:
-        return matchers["grain_match.match"](tgt, delimiter=delimiter, opts=__opts__)
+        return grain_match.match(tgt, delimiter=delimiter, opts=__opts__)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False
@@ -237,9 +238,8 @@ def list_(tgt, minion_id=None):
     if minion_id is not None:
         if not isinstance(minion_id, str):
             minion_id = str(minion_id)
-    matchers = salt.loader.matchers(__opts__)
     try:
-        return matchers["list_match.match"](tgt, opts=__opts__, minion_id=minion_id)
+        return list_match.match(tgt, opts=__opts__, minion_id=minion_id)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False
@@ -263,9 +263,8 @@ def pcre(tgt, minion_id=None):
     if minion_id is not None:
         if not isinstance(minion_id, str):
             minion_id = str(minion_id)
-    matchers = salt.loader.matchers(__opts__)
     try:
-        return matchers["pcre_match.match"](tgt, opts=__opts__, minion_id=minion_id)
+        return pcre_match.match(tgt, opts=__opts__, minion_id=minion_id)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False
@@ -289,10 +288,9 @@ def glob(tgt, minion_id=None):
     if minion_id is not None:
         if not isinstance(minion_id, str):
             minion_id = str(minion_id)
-    matchers = salt.loader.matchers(__opts__)
 
     try:
-        return matchers["glob_match.match"](tgt, opts=__opts__, minion_id=minion_id)
+        return glob_match.match(tgt, opts=__opts__, minion_id=minion_id)
     except Exception as exc:  # pylint: disable=broad-except
         log.exception(exc)
         return False

As far as I can tell, there's no reason to invoke the Salt Loader for known-present matchers since we're not relying on __virtual__() to selectively load matchers or storing the loaded matchers in a location (like __salt__) where they can be invoked without incurring loader overhead each time.

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
               Salt: 3004.2
 
Dependency Versions:
               cffi: Not Installed
           cherrypy: Not Installed
           dateutil: 2.7.3
          docker-py: Not Installed
              gitdb: 2.0.5
          gitpython: 2.1.11
             Jinja2: 2.10
            libgit2: Not Installed
           M2Crypto: Not Installed
               Mako: Not Installed
            msgpack: 0.5.6
       msgpack-pure: Not Installed
       mysql-python: Not Installed
          pycparser: Not Installed
           pycrypto: 2.6.1
       pycryptodome: 3.6.1
             pygit2: Not Installed
             Python: 3.7.3 (default, Jan 22 2021, 20:04:44)
       python-gnupg: Not Installed
             PyYAML: 3.13
              PyZMQ: 17.1.2
              smmap: 2.0.5
            timelib: Not Installed
            Tornado: 4.5.3
                ZMQ: 4.3.1
 
Salt Extensions:
 saltext.prometheus: 1.1.1
 
System Versions:
               dist: debian 10 buster
             locale: UTF-8
            machine: x86_64
            release: 4.19.0-20-amd64
             system: Linux
            version: Debian GNU/Linux 10 buster

Additional context
The slowdown here isn't significant enough to notice with "normal" targeting. The match functions would need to be called many times in quick succession, so this really affects function calls in Jinja and custom code.