Perfy McPerferton (#58400)

* InventoryManager start of perf improvements

* 0 not 1

* More startswith to [0] improvements

* Remove unused var

* The hash doesn't need to be a string, start as a list, make it into a tuple

* set actually appears faster than frozenset, and these don't need to be frozen

* Cache hosts lists, to avoid extra get_hosts calls, pass to get_vars too

* negligible perf improvement, it could help with memory later

* Try the fast way, fallback to the safe way

* Revert to previous logic, linting fix

* Extend pre-caching to free

* Address test failures

* Hosts are strings

* Fix unit test

* host is a string

* update test assumption

* drop SharedPluginLoaderObj, pre-create a set, instead of 2 comparisons in the list comprehension

* Dedupe code

* Change to _hosts and _hosts_all in get_vars

* Add backwards compat for strategies that don't do set host caches

* Add deprecation message to SharedPluginLoaderObj

* Remove unused SharedPluginLoaderObj import

* Update docs/comments

* Remove debugging

* Indicate what patterh_hash is

* That won't work

* Re-fix tests

* Update _set_hosts_cache to accept the play directly, use without refresh in get_hosts_remaining and get_failed_hosts for backwards compat

* Rename variable to avoid confusion

* On add_host only manipulate _hosts_cache_all

* Add warning docs around _hosts and _hosts_all args

1 files changed, 47 insertions, 40 deletions

diff --git a/lib/ansible/inventory/manager.py b/lib/ansible/inventory/manager.py
index 32ec5ff3ea..7c328668d1 100644
--- a/lib/ansible/inventory/manager.py
+++ b/lib/ansible/inventory/manager.py
@@ -48,6 +48,18 @@ IGNORED_EXTS = [b'%s$' % to_bytes(re.escape(x)) for x in C.INVENTORY_IGNORE_EXTS
 
 IGNORED = re.compile(b'|'.join(IGNORED_ALWAYS + IGNORED_PATTERNS + IGNORED_EXTS))
 
+PATTERN_WITH_SUBSCRIPT = re.compile(
+    r'''^
+        (.+)                    # A pattern expression ending with...
+        \[(?:                   # A [subscript] expression comprising:
+            (-?[0-9]+)|         # A single positive or negative number
+            ([0-9]+)([:-])      # Or an x:y or x: range.
+            ([0-9]*)
+        )\]
+        $
+    ''', re.X
+)
+
 
 def order_patterns(patterns):
     ''' takes a list of patterns and reorders them by modifier to apply them consistently '''
@@ -57,9 +69,9 @@ def order_patterns(patterns):
     pattern_intersection = []
     pattern_exclude = []
     for p in patterns:
-        if p.startswith("!"):
+        if p[0] == "!":
             pattern_exclude.append(p)
-        elif p.startswith("&"):
+        elif p[0] == "&":
             pattern_intersection.append(p)
         elif p:
             pattern_regular.append(p)
@@ -316,7 +328,7 @@ class InventoryManager(object):
     def _match_list(self, items, pattern_str):
         # compile patterns
         try:
-            if not pattern_str.startswith('~'):
+            if not pattern_str[0] == '~':
                 pattern = re.compile(fnmatch.translate(pattern_str))
             else:
                 pattern = re.compile(pattern_str[1:])
@@ -341,41 +353,45 @@ class InventoryManager(object):
 
         # Check if pattern already computed
         if isinstance(pattern, list):
-            pattern_hash = u":".join(pattern)
+            pattern_list = pattern[:]
         else:
-            pattern_hash = pattern
+            pattern_list = [pattern]
 
-        if pattern_hash:
+        if pattern_list:
             if not ignore_limits and self._subset:
-                pattern_hash += u":%s" % to_text(self._subset, errors='surrogate_or_strict')
+                pattern_list.extend(self._subset)
 
             if not ignore_restrictions and self._restriction:
-                pattern_hash += u":%s" % to_text(self._restriction, errors='surrogate_or_strict')
+                pattern_list.extend(self._restriction)
+
+            # This is only used as a hash key in the self._hosts_patterns_cache dict
+            # a tuple is faster than stringifying
+            pattern_hash = tuple(pattern_list)
 
             if pattern_hash not in self._hosts_patterns_cache:
 
                 patterns = split_host_pattern(pattern)
-                hosts = self._evaluate_patterns(patterns)
+                hosts[:] = self._evaluate_patterns(patterns)
 
                 # mainly useful for hostvars[host] access
                 if not ignore_limits and self._subset:
                     # exclude hosts not in a subset, if defined
-                    subset_uuids = [s._uuid for s in self._evaluate_patterns(self._subset)]
-                    hosts = [h for h in hosts if h._uuid in subset_uuids]
+                    subset_uuids = set(s._uuid for s in self._evaluate_patterns(self._subset))
+                    hosts[:] = [h for h in hosts if h._uuid in subset_uuids]
 
                 if not ignore_restrictions and self._restriction:
                     # exclude hosts mentioned in any restriction (ex: failed hosts)
-                    hosts = [h for h in hosts if h.name in self._restriction]
+                    hosts[:] = [h for h in hosts if h.name in self._restriction]
 
                 self._hosts_patterns_cache[pattern_hash] = deduplicate_list(hosts)
 
             # sort hosts list if needed (should only happen when called from strategy)
             if order in ['sorted', 'reverse_sorted']:
-                hosts = sorted(self._hosts_patterns_cache[pattern_hash][:], key=attrgetter('name'), reverse=(order == 'reverse_sorted'))
+                hosts[:] = sorted(self._hosts_patterns_cache[pattern_hash][:], key=attrgetter('name'), reverse=(order == 'reverse_sorted'))
             elif order == 'reverse_inventory':
-                hosts = self._hosts_patterns_cache[pattern_hash][::-1]
+                hosts[:] = self._hosts_patterns_cache[pattern_hash][::-1]
             else:
-                hosts = self._hosts_patterns_cache[pattern_hash][:]
+                hosts[:] = self._hosts_patterns_cache[pattern_hash][:]
                 if order == 'shuffle':
                     shuffle(hosts)
                 elif order not in [None, 'inventory']:
@@ -398,12 +414,15 @@ class InventoryManager(object):
                 hosts.append(self._inventory.get_host(p))
             else:
                 that = self._match_one_pattern(p)
-                if p.startswith("!"):
-                    hosts = [h for h in hosts if h not in frozenset(that)]
-                elif p.startswith("&"):
-                    hosts = [h for h in hosts if h in frozenset(that)]
+                if p[0] == "!":
+                    that = set(that)
+                    hosts = [h for h in hosts if h not in that]
+                elif p[0] == "&":
+                    that = set(that)
+                    hosts = [h for h in hosts if h in that]
                 else:
-                    hosts.extend([h for h in that if h.name not in frozenset([y.name for y in hosts])])
+                    existing_hosts = set(y.name for y in hosts)
+                    hosts.extend([h for h in that if h.name not in existing_hosts])
         return hosts
 
     def _match_one_pattern(self, pattern):
@@ -444,7 +463,7 @@ class InventoryManager(object):
         Duplicate matches are always eliminated from the results.
         """
 
-        if pattern.startswith("&") or pattern.startswith("!"):
+        if pattern[0] in ("&", "!"):
             pattern = pattern[1:]
 
         if pattern not in self._pattern_cache:
@@ -469,27 +488,15 @@ class InventoryManager(object):
         """
 
         # Do not parse regexes for enumeration info
-        if pattern.startswith('~'):
+        if pattern[0] == '~':
             return (pattern, None)
 
         # We want a pattern followed by an integer or range subscript.
         # (We can't be more restrictive about the expression because the
         # fnmatch semantics permit [\[:\]] to occur.)
 
-        pattern_with_subscript = re.compile(
-            r'''^
-                (.+)                    # A pattern expression ending with...
-                \[(?:                   # A [subscript] expression comprising:
-                    (-?[0-9]+)|         # A single positive or negative number
-                    ([0-9]+)([:-])      # Or an x:y or x: range.
-                    ([0-9]*)
-                )\]
-                $
-            ''', re.X
-        )
-
         subscript = None
-        m = pattern_with_subscript.match(pattern)
+        m = PATTERN_WITH_SUBSCRIPT.match(pattern)
         if m:
             (pattern, idx, start, sep, end) = m.groups()
             if idx:
@@ -535,7 +542,7 @@ class InventoryManager(object):
                 results.extend(self._inventory.groups[groupname].get_hosts())
 
         # check hosts if no groups matched or it is a regex/glob pattern
-        if not matching_groups or pattern.startswith('~') or any(special in pattern for special in ('.', '?', '*', '[')):
+        if not matching_groups or pattern[0] == '~' or any(special in pattern for special in ('.', '?', '*', '[')):
             # pattern might match host
             matching_hosts = self._match_list(self._inventory.hosts, pattern)
             if matching_hosts:
@@ -585,7 +592,7 @@ class InventoryManager(object):
             return
         elif not isinstance(restriction, list):
             restriction = [restriction]
-        self._restriction = [h.name for h in restriction]
+        self._restriction = set(to_text(h.name) for h in restriction)
 
     def subset(self, subset_pattern):
         """
@@ -601,12 +608,12 @@ class InventoryManager(object):
             results = []
             # allow Unix style @filename data
             for x in subset_patterns:
-                if x.startswith("@"):
+                if x[0] == "@":
                     fd = open(x[1:])
-                    results.extend([l.strip() for l in fd.read().split("\n")])
+                    results.extend([to_text(l.strip()) for l in fd.read().split("\n")])
                     fd.close()
                 else:
-                    results.append(x)
+                    results.append(to_text(x))
             self._subset = results
 
     def remove_restriction(self):