Refactor: Performance improvements (#3521)

Co-authored-by: Guillaume Mulocher <[email protected]>

.pre-commit-config.yaml

@@ -10,7 +10,7 @@ repos:
         exclude_types: [svg, json]
         exclude: ansible_collections/arista/avd/molecule
       - id: check-added-large-files
-        exclude: ansible_collections/arista/avd/molecule
+        exclude: (ansible_collections/arista/avd/molecule|pickle$)
       - id: check-merge-conflict
         exclude: ansible_collections/arista/avd/molecule
 
@@ -153,15 +153,8 @@ repos:
         files: ansible_collections/arista/avd/plugins/vars/
 
       - id: schemas
-        name: Build AVD Schemas
-        entry: sh -c 'exec env ANSIBLE_COLLECTIONS_PATHS=`pwd` ansible-playbook --forks 10 arista.avd._build_schemas'
-        language: system
-        files: ansible_collections/arista/avd/roles/[a-z_]+/schemas
-        pass_filenames: false
-
-      - id: schemas
-        name: Build AVD Docs
-        entry: sh -c 'exec python-avd/scripts/build-schema-tables.py'
+        name: Build AVD Schemas and Docs
+        entry: sh -c 'exec python-avd/scripts/build-schemas.py'
         language: system
         files: ansible_collections/arista/avd/roles/[a-z_]+/schemas
         pass_filenames: false

ansible_collections/arista/avd/docs/plugins/Modules_and_action_plugins/batch_template.md

@@ -39,9 +39,7 @@ Destination file mode is hardcoded to 0o664.
   arista.avd.batch_template:
     template: avd_schema_documentation.j2
     dest_format_str: "{{ role_documentation_dir }}/{item}.md"
-    items: "{{ documentation_schema | list }}"
-  vars:
-    documentation_schema: "{{ role_name | arista.avd.convert_schema(type='documentation') }}"
+    items: "{{ data_in_list_form }}"
 ```
 
 ## Authors

ansible_collections/arista/avd/docs/plugins/Modules_and_action_plugins/validate_and_template.md

@@ -38,6 +38,7 @@ For Markdown files the plugin can also run md\_toc on the output before writing
 | <samp>md_toc_skip_lines</samp> | int | False | 0 |  | Pass this value as skip\_lines to add\_md\_toc. |
 | <samp>conversion_mode</samp> | str | False | debug | Valid values:<br>- <code>warning</code><br>- <code>info</code><br>- <code>debug</code><br>- <code>quiet</code><br>- <code>disabled</code> | Run data conversion in either \"warning\", \"info\", \"debug\", \"quiet\" or \"disabled\" mode.<br>Conversion will perform type conversion of input variables as defined in the schema.<br>Conversion is intended to help the user to identify minor issues with the input data, while still allowing the data to be validated.<br>During conversion, messages will generated with information about the host\(s\) and key\(s\) which required conversion.<br>conversion\_mode\:disabled means that conversion will not run.<br>conversion\_mode\:error will produce error messages and fail the task.<br>conversion\_mode\:warning will produce warning messages.<br>conversion\_mode\:info will produce regular log messages.<br>conversion\_mode\:debug will produce hidden messages viewable with \-v.<br>conversion\_mode\:quiet will not produce any messages. |
 | <samp>validation_mode</samp> | str | False | warning | Valid values:<br>- <code>error</code><br>- <code>warning</code><br>- <code>info</code><br>- <code>debug</code><br>- <code>disabled</code> | Run validation in either \"error\", \"warning\", \"info\", \"debug\" or \"disabled\" mode.<br>Validation will validate the input variables according to the schema.<br>During validation, messages will generated with information about the host\(s\) and key\(s\) which failed validation.<br>validation\_mode\:disabled means that validation will not run.<br>validation\_mode\:error will produce error messages and fail the task.<br>validation\_mode\:warning will produce warning messages.<br>validation\_mode\:info will produce regular log messages.<br>validation\_mode\:debug will produce hidden messages viewable with \-v. |
+| <samp>cprofile_file</samp> | str | False | None |  | Filename for storing cprofile data used to debug performance issues.<br>Running cprofile will slow down performance in itself, so only set this while troubleshooting. |
 
 ## Examples
 

ansible_collections/arista/avd/plugins/action/validate_and_template.py

@@ -5,6 +5,9 @@
 
 __metaclass__ = type
 
+import cProfile
+import pstats
+
 from ansible.errors import AnsibleActionFail
 from ansible.plugins.action import ActionBase, display
 
@@ -21,6 +24,11 @@ def run(self, tmp=None, task_vars=None):
         result = super().run(tmp, task_vars)
         del tmp  # tmp no longer has any effect
 
+        cprofile_file = self._task.args.get("cprofile_file")
+        if cprofile_file:
+            profiler = cProfile.Profile()
+            profiler.enable()
+
         # Validate Arguments
         self.templatefile = self._task.args.get("template")
         if not isinstance(self.templatefile, str):
@@ -78,6 +86,11 @@ def run(self, tmp=None, task_vars=None):
         if not result.get("failed"):
             result.update(self.template(task_vars, dest))
 
+        if cprofile_file:
+            profiler.disable()
+            stats = pstats.Stats(profiler).sort_stats("cumtime")
+            stats.dump_stats(cprofile_file)
+
         return result
 
     def template(self, task_vars, dest):

ansible_collections/arista/avd/plugins/modules/batch_template.py

@@ -35,7 +35,5 @@
   arista.avd.batch_template:
     template: avd_schema_documentation.j2
     dest_format_str: "{{ role_documentation_dir }}/{item}.md"
-    items: "{{ documentation_schema | list }}"
-  vars:
-    documentation_schema: "{{ role_name | arista.avd.convert_schema(type='documentation') }}"
+    items: "{{ data_in_list_form }}"
 """

ansible_collections/arista/avd/plugins/modules/validate_and_template.py

@@ -75,6 +75,12 @@
     default: "warning"
     type: str
     choices: [ "error", "warning", "info", "debug", "disabled" ]
+  cprofile_file:
+    description:
+      - Filename for storing cprofile data used to debug performance issues.
+      - Running cprofile will slow down performance in itself, so only set this while troubleshooting.
+    required: false
+    type: str
 """
 
 EXAMPLES = r"""

ansible_collections/arista/avd/plugins/plugin_utils/eos_designs_facts/uplinks.py

@@ -8,7 +8,6 @@
 from typing import TYPE_CHECKING
 
 from ansible_collections.arista.avd.plugins.filter.list_compress import list_compress
-from ansible_collections.arista.avd.plugins.filter.natural_sort import natural_sort
 from ansible_collections.arista.avd.plugins.filter.range_expand import range_expand
 from ansible_collections.arista.avd.plugins.plugin_utils.errors import AristaAvdError, AristaAvdMissingVariableError
 from ansible_collections.arista.avd.plugins.plugin_utils.utils import append_if_not_duplicate, default, get, unique
@@ -377,7 +376,7 @@ def uplink_peers(self: EosDesignsFacts) -> list:
         """
         uplink_switches = self.shared_utils.uplink_switches
         # Making sure each peer is unique
-        return natural_sort(unique(uplink_switch for uplink_switch in uplink_switches if uplink_switch in self.shared_utils.all_fabric_devices))
+        return unique(uplink_switch for uplink_switch in uplink_switches if uplink_switch in self.shared_utils.all_fabric_devices)
 
     @cached_property
     def _default_downlink_interfaces(self: EosDesignsFacts) -> list:

ansible_collections/arista/avd/plugins/plugin_utils/eos_designs_facts/vlans.py

@@ -9,7 +9,6 @@
 
 from ansible_collections.arista.avd.plugins.filter.convert_dicts import convert_dicts
 from ansible_collections.arista.avd.plugins.filter.list_compress import list_compress
-from ansible_collections.arista.avd.plugins.filter.natural_sort import natural_sort
 from ansible_collections.arista.avd.plugins.filter.range_expand import range_expand
 from ansible_collections.arista.avd.plugins.plugin_utils.utils import get
 
@@ -252,22 +251,28 @@ def _vlans(self: EosDesignsFacts) -> list[int]:
                 endpoint_vlans = self._endpoint_vlans
 
             for network_services_key in self.shared_utils.network_services_keys:
-                tenants = get(self._hostvars, network_services_key["name"])
+                tenants = get(self._hostvars, network_services_key["name"], default=[])
                 # Support legacy data model by converting nested dict to list of dict
-                tenants = convert_dicts(tenants, "name")
-                for tenant in natural_sort(tenants, "name"):
+                if isinstance(tenants, dict):
+                    tenants = convert_dicts(tenants, "name")
+
+                for tenant in tenants:
                     if not set(self.shared_utils.filter_tenants).intersection([tenant["name"], "all"]):
                         # Not matching tenant filters. Skipping this tenant.
                         continue
 
                     vrfs = tenant.get("vrfs", [])
                     # Support legacy data model by converting nested dict to list of dict
-                    vrfs = convert_dicts(vrfs, "name")
-                    for vrf in natural_sort(vrfs, "name"):
+                    if isinstance(vrfs, dict):
+                        vrfs = convert_dicts(vrfs, "name")
+
+                    for vrf in vrfs:
                         svis = vrf.get("svis", [])
                         # Support legacy data model by converting nested dict to list of dict
-                        svis = convert_dicts(svis, "id")
-                        for svi in natural_sort(svis, "id"):
+                        if isinstance(svis, dict):
+                            svis = convert_dicts(svis, "id")
+
+                        for svi in svis:
                             svi_tags = svi.get("tags", ["all"])
                             if "all" in match_tags or set(svi_tags).intersection(match_tags):
                                 if self.shared_utils.filter_only_vlans_in_use:
@@ -289,9 +294,10 @@ def _vlans(self: EosDesignsFacts) -> list[int]:
 
                     l2vlans = tenant.get("l2vlans", [])
                     # Support legacy data model by converting nested dict to list of dict
-                    l2vlans = convert_dicts(l2vlans, "id")
+                    if isinstance(l2vlans, dict):
+                        l2vlans = convert_dicts(l2vlans, "id")
 
-                    for l2vlan in natural_sort(l2vlans, "id"):
+                    for l2vlan in l2vlans:
                         l2vlan_tags = l2vlan.get("tags", ["all"])
                         if "all" in match_tags or set(l2vlan_tags).intersection(match_tags):
                             if self.shared_utils.filter_only_vlans_in_use:

ansible_collections/arista/avd/plugins/plugin_utils/schema/avddataconverter.py

@@ -3,7 +3,7 @@
 # that can be found in the LICENSE file.
 from __future__ import annotations
 
-from typing import Generator
+from typing import TYPE_CHECKING, Generator
 
 from ansible_collections.arista.avd.plugins.filter.convert_dicts import convert_dicts
 from ansible_collections.arista.avd.plugins.plugin_utils.errors import AvdConversionWarning, AvdDeprecationWarning
@@ -23,16 +23,16 @@
     "bool": bool,
 }
 
+if TYPE_CHECKING:
+    from .avdschema import AvdSchema
+
 
 class AvdDataConverter:
     """
     AvdDataConverter is used to convert AVD Data Types based on schema options.
-
-    avdschema argument is an instance of AvdSchema. Type hinting is not working because of circular import
-    TODO: Refactor to take a fully resolved schema as dict
     """
 
-    def __init__(self, avdschema):
+    def __init__(self, avdschema: AvdSchema):
         self._avdschema = avdschema
 
         # We run through all the regular keys first, to ensure that all data has been converted
@@ -44,15 +44,11 @@ def __init__(self, avdschema):
             "deprecation": self.deprecation,
         }
 
-    def convert_data(self, data, schema: dict = None, path: list[str] = None) -> Generator:
+    def convert_data(self, data, schema: dict, path: list[str] = None) -> Generator:
         """
         Perform in-place conversion of data according to the provided schema.
         Main entry function which is recursively called from the child functions performing the actual conversion of keys/items.
         """
-        if schema is None:
-            # Get fully resolved schema (where all $ref has been expanded recursively)
-            schema = self._avdschema.resolved_schema
-
         if path is None:
             path = []