From 6069d09b9d7193a1896dfc2816e0e51833a53e6b Mon Sep 17 00:00:00 2001 From: Pilou Date: Tue, 25 Sep 2018 22:24:25 +0200 Subject: [PATCH] Speed up VariableManager by preserving Templar state. (#45572) Maintain one Templar for the lifetime of VariableManager, calling set_available_variables() prior to each use, enabling _get_filter()'s cache to function correctly. It does not seem possible for concurrent calls into one (non-copied) VariableManager instance, and so it need not be reentrant. If that became a requirement, serializing its or Templar's entry points would be fine, as it's so CPU-heavy other threads will only fight with it for the GIL anyway. Reduces _get_filters() runtime 91%, get_vars() runtime 19%, function call count 16%, overall runtime 10%. Tested aginst dummy load comprised of the 12 disabled steps of debops.auth with an inventory of 80 hosts, which stresses variable processing and task setup. Before: 7447296 function calls (7253994 primitive calls) in 32.611 seconds Ordered by: cumulative time ncalls tottime percall cumtime percall filename:lineno(function) 1 0.000 0.000 32.762 32.762 ansible-playbook:3() 1 0.007 0.007 31.733 31.733 ansible-playbook:21() ... 1371/971 0.671 0.000 21.332 0.022 manager.py:154(get_vars) ... 3044 0.315 0.000 5.166 0.002 __init__.py:295(_get_filters) After: 6252978 function calls (6059638 primitive calls) in 29.055 seconds Ordered by: cumulative time ncalls tottime percall cumtime percall filename:lineno(function) 1 0.000 0.000 29.218 29.218 ansible-playbook:3() 1 0.007 0.007 28.159 28.159 ansible-playbook:21() ... 1371/971 0.675 0.000 17.211 0.018 manager.py:154(get_vars) ... 3044 0.028 0.000 0.441 0.000 __init__.py:295(_get_filters) --- lib/ansible/vars/manager.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py index a88b7ae9961..9a898f44389 100644 --- a/lib/ansible/vars/manager.py +++ b/lib/ansible/vars/manager.py @@ -79,7 +79,6 @@ class VariableManager: 'all_plugins_play', 'all_plugins_inventory', 'all_inventory']) def __init__(self, loader=None, inventory=None): - self._nonpersistent_fact_cache = defaultdict(dict) self._vars_cache = defaultdict(dict) self._extra_vars = defaultdict(dict) @@ -91,6 +90,7 @@ class VariableManager: self._omit_token = '__omit_place_holder__%s' % sha1(os.urandom(64)).hexdigest() self._options_vars = defaultdict(dict) self.safe_basedir = False + self._templar = Templar(loader=self._loader) # bad cache plugin is not fatal error try: @@ -331,7 +331,7 @@ class VariableManager: # and magic vars so we can properly template the vars_files entries temp_vars = combine_vars(all_vars, self._extra_vars) temp_vars = combine_vars(temp_vars, magic_variables) - templar = Templar(loader=self._loader, variables=temp_vars) + self._templar.set_available_variables(temp_vars) # we assume each item in the list is itself a list, as we # support "conditional includes" for vars_files, which mimics @@ -345,7 +345,7 @@ class VariableManager: # raise an error, which is silently ignored at this point. try: for vars_file in vars_file_list: - vars_file = templar.template(vars_file) + vars_file = self._templar.template(vars_file) if not (isinstance(vars_file, Sequence)): raise AnsibleError( "Invalid vars_files entry found: %r\n" @@ -461,8 +461,7 @@ class VariableManager: if self._inventory is not None: variables['groups'] = self._inventory.get_groups_dict() if play: - templar = Templar(loader=self._loader) - if templar.is_template(play.hosts): + if self._templar.is_template(play.hosts): pattern = 'all' else: pattern = play.hosts or 'all' @@ -495,16 +494,16 @@ class VariableManager: # as we're fetching vars before post_validate has been called on # the task that has been passed in vars_copy = existing_variables.copy() - templar = Templar(loader=self._loader, variables=vars_copy) + self._templar.set_available_variables(vars_copy) items = [] has_loop = True if task.loop_with is not None: if task.loop_with in lookup_loader: try: - loop_terms = listify_lookup_plugin_terms(terms=task.loop, templar=templar, + loop_terms = listify_lookup_plugin_terms(terms=task.loop, templar=self._templar, loader=self._loader, fail_on_undefined=True, convert_bare=False) - items = lookup_loader.get(task.loop_with, loader=self._loader, templar=templar).run(terms=loop_terms, variables=vars_copy) + items = lookup_loader.get(task.loop_with, loader=self._loader, templar=self._templar).run(terms=loop_terms, variables=vars_copy) except AnsibleUndefinedVariable: # This task will be skipped later due to this, so we just setup # a dummy array for the later code so it doesn't fail @@ -513,7 +512,7 @@ class VariableManager: raise AnsibleError("Failed to find the lookup named '%s' in the available lookup plugins" % task.loop_with) elif task.loop is not None: try: - items = templar.template(task.loop) + items = self._templar.template(task.loop) except AnsibleUndefinedVariable: # This task will be skipped later due to this, so we just setup # a dummy array for the later code so it doesn't fail @@ -530,8 +529,8 @@ class VariableManager: if item is not None: vars_copy[item_var] = item - templar.set_available_variables(vars_copy) - delegated_host_name = templar.template(task.delegate_to, fail_on_undefined=False) + self._templar.set_available_variables(vars_copy) + delegated_host_name = self._templar.template(task.delegate_to, fail_on_undefined=False) if delegated_host_name != task.delegate_to: cache_items = True if delegated_host_name is None: