Merge pull request #664 from Nikokrock/mr/improve_perf

Improve anod specs and plans loading performance

NEWS.md

@@ -4,6 +4,11 @@
 * Security enhancements:
   * e3.net.smtp.sendmail uses to ``SMTP_SSL`` by default
 
+* New Anod API Version 1.6:
+  * For performance issues declare dynamically "spec" function in the
+    Anod spec context rather than using e3.anod.loader.spec function
+    that relies on inspect module
+
 # Version 22.3.1 (2023-03-17)
 
 * Add rlimit binary for aarch64-darwin

src/e3/anod/loader.py

@@ -5,12 +5,13 @@
 import types
 import yaml
 
+from packaging.version import Version
 from typing import TYPE_CHECKING
 
 import e3.hash
 import e3.log
 from e3.anod.error import SandBoxError
-from e3.anod.spec import __version__
+from e3.anod.spec import __version__, check_api_version
 from e3.fs import ls
 
 logger = e3.log.getLogger("anod.loader")
@@ -69,7 +70,22 @@ def __init__(
         if not os.path.isdir(spec_dir):
             raise SandBoxError(f"spec directory {spec_dir} does not exist")
         self.spec_dir = spec_dir
-        self.api_version = __version__
+
+        # Read the API version file
+        version_file = os.path.join(self.spec_dir, "VERSION")
+        if os.path.isfile(version_file):
+            with open(version_file) as f:
+                content = f.read().strip()
+                if ":" not in content:
+                    raise SandBoxError(
+                        f"invalid VERSION file in spec dir {self.spec_dir}"
+                    )
+                self.api_version = content.split(":")[1].strip()
+            check_api_version(self.api_version)
+        else:
+            # If no VERSION file is found use the default API VERSION
+            self.api_version = __version__
+
         self.specs = {}
         self.repos: dict[str, dict[str, str]] = {}
 
@@ -81,36 +97,40 @@ def __init__(
         logger.debug("found %s specs", len(spec_list))
 
         # API == 1.4
-        yaml_files = ls(os.path.join(self.spec_dir, "*.yaml"), emit_log_record=False)
-        data_list = [os.path.basename(k)[:-5] for k in yaml_files]
-        logger.debug("found %s yaml files API 1.4 compatible", len(data_list))
-
-        # Match yaml files with associated specifications
-        for data in data_list:
-            candidate_specs = [
-                spec_file for spec_file in spec_list if data.startswith(spec_file)
-            ]
-            # We pick the longuest spec name
-            candidate_specs.sort(key=len)
-            if candidate_specs:
-                spec_list[candidate_specs[-1]]["data"].append(data)  # type: ignore
+        if Version(self.api_version) < Version("1.5"):
+            yaml_files = ls(
+                os.path.join(self.spec_dir, "*.yaml"), emit_log_record=False
+            )
+            data_list = [os.path.basename(k)[:-5] for k in yaml_files]
+            logger.debug("found %s yaml files API 1.4 compatible", len(data_list))
+
+            # Match yaml files with associated specifications
+            for data in data_list:
+                candidate_specs = [
+                    spec_file for spec_file in spec_list if data.startswith(spec_file)
+                ]
+                # We pick the longuest spec name
+                candidate_specs.sort(key=len)
+                if candidate_specs:
+                    spec_list[candidate_specs[-1]]["data"].append(data)  # type: ignore
 
         # Find yaml files that are API >= 1.5 compatible
-        new_yaml_files = ls(
-            os.path.join(self.spec_dir, "*", "*.yaml"), emit_log_record=False
-        )
-
-        for yml_f in new_yaml_files:
-            associated_spec = os.path.basename(os.path.dirname(yml_f))
-
-            # Keep only the yaml files associated with an .anod file
-            if associated_spec in spec_list:
-                # We're recording the relative path without the extension
-                suffix, _ = os.path.splitext(os.path.basename(yml_f))
-
-                spec_list[associated_spec]["data"].append(  # type: ignore
-                    os.path.join(associated_spec, suffix)
-                )
+        if Version(self.api_version) >= Version("1.5"):
+            new_yaml_files = ls(
+                os.path.join(self.spec_dir, "*", "*.yaml"), emit_log_record=False
+            )
+            logger.info(new_yaml_files)
+            for yml_f in new_yaml_files:
+                associated_spec = os.path.basename(os.path.dirname(yml_f))
+
+                # Keep only the yaml files associated with an .anod file
+                if associated_spec in spec_list:
+                    # We're recording the relative path without the extension
+                    suffix, _ = os.path.splitext(os.path.basename(yml_f))
+
+                    spec_list[associated_spec]["data"].append(  # type: ignore
+                        os.path.join(associated_spec, suffix)
+                    )
 
         # Create AnodModule objects
         for name, value in spec_list.items():
@@ -141,7 +161,12 @@ def __init__(
 
         # Declare spec prolog
         prolog_file = os.path.join(spec_dir, "prolog.py")
-        self.prolog_dict = {"spec_config": spec_config, "__spec_repository": self}
+        self.prolog_dict = {
+            "spec_config": spec_config,
+            "__spec_repository": self,
+            "spec": self.load,
+        }
+
         if os.path.exists(prolog_file):
             with open(prolog_file) as f:
                 exec(compile(f.read(), prolog_file, "exec"), self.prolog_dict)
@@ -268,39 +293,32 @@ def load(self, repository: AnodSpecRepository) -> Callable[..., Anod]:
         raise SandBoxError(f"cannot find Anod subclass in {self.path}", "load")
 
 
-fixed_spec_repository: AnodSpecRepository | None = None
-# A local cache for the spec_repository.
-# This is set by set_spec_repository(), and used by spec().
-
-
-def set_spec_repository(spec_repository: AnodSpecRepository) -> None:
-    """Set the spec repository to use within the lifetime of this process.
-
-    This can be used to set the spec repository to use when calling spec()
-    below, without having to inspect the stack every time, which can be
-    expensive.
-
-    :param spec_repository: the spec repository to set
-    """
-    global fixed_spec_repository
-    fixed_spec_repository = spec_repository
-
-
 def spec(name: str) -> Callable[..., Anod]:
     """Load an Anod spec class.
 
+    Obsolete: keep until all from e3.anod.loader import spec are removed from the
+    specs.
+
     Note that two spec having the same name cannot be loaded in the same
     process as e3 keeps a cache of loaded spec using the spec basename as a
     key.
     :param name: name of the spec to load
     """
-    if fixed_spec_repository is not None:
-        return fixed_spec_repository.load(name)
-
-    for k in inspect.stack()[1:]:
-        if "__spec_repository" in k[0].f_globals:
-            spec_repository = k[0].f_globals["__spec_repository"]
+    spec_repository: AnodSpecRepository | None = None
+
+    # Implementation note: context=0 means that the no source context is
+    # computed for each frame. This improves drastically the performance
+    # as it avoids reading the source file for each frame.
+    for k in inspect.stack(context=0)[1:]:
+        spec_repository = k[0].f_globals.get("__spec_repository")
+        if spec_repository is not None:
             break
 
     assert spec_repository is not None
+
+    if Version(spec_repository.api_version) >= Version("1.6"):
+        logger.error("e3.anod.loader.spec is only valid for API VERSION < 1.6")
+        raise SandBoxError(
+            "e3.anod.loader.spec is only valid for API VERSION < 1.6", "spec"
+        )
     return spec_repository.load(name)

src/e3/anod/spec.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import os
-from distutils.version import StrictVersion
+from packaging.version import Version
 from typing import TYPE_CHECKING
 
 import yaml
@@ -16,12 +16,20 @@
 from e3.anod.qualifiers_manager import QualifiersManager
 from e3.yaml import load_with_config
 
-# CURRENT API version
+# Default API version
 __version__ = "1.4"
 
-SUPPORTED_API = (__version__, "1.5")
-# The driver can support multiple version of the spec API, we currently support
-# only the version 1.4 and 1.5. Default is still 1.4
+SUPPORTED_API = (__version__, "1.5", "1.6")
+# API VERSIONS
+#
+# Version 1.4 (initial version)
+# Version 1.5
+#    NEW: YAML files are searched in subdirectories whose basename is the
+#    the associated spec basename.
+#    DEPRECATED: YAML files in same directory as the spec
+# Version 1.6
+#    NEW: Add support for spec function automatically declared inside each spec
+#    DEPRECATED: remove support of e3.anod.loader.spec
 
 logger = e3.log.getLogger("anod")
 spec_logger = e3.log.getLogger("anod.spec")
@@ -377,7 +385,7 @@ def load_config_file(
         :param selectors: additional selectors for extended mode
         """
         # Compute data file location and check for existence
-        if StrictVersion(self.api_version) >= StrictVersion("1.5"):
+        if Version(self.api_version) >= Version("1.5"):
             filename = os.path.join(self.name, suffix if suffix else "config")
         else:
             filename = "{}{}".format(self.name, "-" + suffix if suffix else "")

src/e3/electrolyt/plan.py

@@ -292,7 +292,12 @@ def _add_action(self, name: str, *args: Any, **kwargs: Any) -> None:
         plan_line = "unknown filename:unknown lineno"
         # Retrieve the plan line
         try:
-            caller_frames = inspect.getouterframes(frame=inspect.currentframe())
+            # Implementation note: context=0 means that the no source context is
+            # computed for each frame. This improves drastically the performance
+            # as it avoids reading the source file for each frame.
+            caller_frames = inspect.getouterframes(
+                frame=inspect.currentframe(), context=0
+            )
             caller_frames_in_plan = []
             for frame in caller_frames:
                 frame_filename = frame.filename

tests/tests_e3/anod/loader_test.py

@@ -125,8 +125,9 @@ def test_load_config(self):
     def test_load_config_api_1_5(self):
         sync_tree(self.spec_dir, "new_spec_dir")
         Run(["e3-sandbox", "migrate", "1.5", "new_spec_dir"], output=None)
+        with open("new_spec_dir/VERSION", "w") as fd:
+            fd.write("API VERSION:1.5")
         spec_repo = AnodSpecRepository("new_spec_dir")
-        spec_repo.api_version = "1.5"
         anod_class = spec_repo.load("withconfig")
         anod_instance = anod_class("", "build")
         assert anod_instance.test1() == 9