spack.compiler/spack.util.libc: add caching (spack#47213)

* spack.compiler: cache output

* compute libc from the dynamic linker at most once per spack process

* wrap compiler cache entry in class, add type hints

* test compiler caching

* ensure tests do not populate user cache, and fix 2 tests

* avoid recursion: cache lookup -> compute key -> cflags -> real_version -> cache lookup

* allow compiler execution in test that depends on get_real_version

lib/spack/docs/conf.py

@@ -221,6 +221,7 @@ def setup(sphinx):
     ("py:class", "spack.filesystem_view.SimpleFilesystemView"),
     ("py:class", "spack.traverse.EdgeAndDepth"),
     ("py:class", "archspec.cpu.microarchitecture.Microarchitecture"),
+    ("py:class", "spack.compiler.CompilerCache"),
     # TypeVar that is not handled correctly
     ("py:class", "llnl.util.lang.T"),
 ]

lib/spack/spack/compiler.py

@@ -4,20 +4,23 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import contextlib
+import hashlib
 import itertools
+import json
 import os
 import platform
 import re
 import shutil
 import sys
 import tempfile
-from typing import List, Optional, Sequence
+from typing import Dict, List, Optional, Sequence
 
 import llnl.path
 import llnl.util.lang
 import llnl.util.tty as tty
 from llnl.util.filesystem import path_contains_subdirectory, paths_containing_libs
 
+import spack.caches
 import spack.error
 import spack.schema.environment
 import spack.spec
@@ -34,7 +37,7 @@
 
 
 @llnl.util.lang.memoized
-def _get_compiler_version_output(compiler_path, version_arg, ignore_errors=()):
+def _get_compiler_version_output(compiler_path, version_arg, ignore_errors=()) -> str:
     """Invokes the compiler at a given path passing a single
     version argument and returns the output.
 
@@ -57,7 +60,7 @@ def _get_compiler_version_output(compiler_path, version_arg, ignore_errors=()):
     return output
 
 
-def get_compiler_version_output(compiler_path, *args, **kwargs):
+def get_compiler_version_output(compiler_path, *args, **kwargs) -> str:
     """Wrapper for _get_compiler_version_output()."""
     # This ensures that we memoize compiler output by *absolute path*,
     # not just executable name. If we don't do this, and the path changes
@@ -290,6 +293,7 @@ def __init__(
         self.environment = environment or {}
         self.extra_rpaths = extra_rpaths or []
         self.enable_implicit_rpaths = enable_implicit_rpaths
+        self.cache = COMPILER_CACHE
 
         self.cc = paths[0]
         self.cxx = paths[1]
@@ -390,15 +394,11 @@ def real_version(self):
 
         E.g. C++11 flag checks.
         """
-        if not self._real_version:
-            try:
-                real_version = spack.version.Version(self.get_real_version())
-                if real_version == spack.version.Version("unknown"):
-                    return self.version
-                self._real_version = real_version
-            except spack.util.executable.ProcessError:
-                self._real_version = self.version
-        return self._real_version
+        real_version_str = self.cache.get(self).real_version
+        if not real_version_str or real_version_str == "unknown":
+            return self.version
+
+        return spack.version.StandardVersion.from_string(real_version_str)
 
     def implicit_rpaths(self) -> List[str]:
         if self.enable_implicit_rpaths is False:
@@ -445,9 +445,7 @@ def required_libs(self):
     @property
     def compiler_verbose_output(self) -> Optional[str]:
         """Verbose output from compiling a dummy C source file. Output is cached."""
-        if not hasattr(self, "_compile_c_source_output"):
-            self._compile_c_source_output = self._compile_dummy_c_source()
-        return self._compile_c_source_output
+        return self.cache.get(self).c_compiler_output
 
     def _compile_dummy_c_source(self) -> Optional[str]:
         cc = self.cc if self.cc else self.cxx
@@ -559,7 +557,7 @@ def fc_pic_flag(self):
     # Note: This is not a class method. The class methods are used to detect
     # compilers on PATH based systems, and do not set up the run environment of
     # the compiler. This method can be called on `module` based systems as well
-    def get_real_version(self):
+    def get_real_version(self) -> str:
         """Query the compiler for its version.
 
         This is the "real" compiler version, regardless of what is in the
@@ -569,14 +567,17 @@ def get_real_version(self):
         modifications) to enable the compiler to run properly on any platform.
         """
         cc = spack.util.executable.Executable(self.cc)
-        with self.compiler_environment():
-            output = cc(
-                self.version_argument,
-                output=str,
-                error=str,
-                ignore_errors=tuple(self.ignore_version_errors),
-            )
-            return self.extract_version_from_output(output)
+        try:
+            with self.compiler_environment():
+                output = cc(
+                    self.version_argument,
+                    output=str,
+                    error=str,
+                    ignore_errors=tuple(self.ignore_version_errors),
+                )
+                return self.extract_version_from_output(output)
+        except spack.util.executable.ProcessError:
+            return "unknown"
 
     @property
     def prefix(self):
@@ -603,7 +604,7 @@ def default_version(cls, cc):
 
     @classmethod
     @llnl.util.lang.memoized
-    def extract_version_from_output(cls, output):
+    def extract_version_from_output(cls, output: str) -> str:
         """Extracts the version from compiler's output."""
         match = re.search(cls.version_regex, output)
         return match.group(1) if match else "unknown"
@@ -732,3 +733,106 @@ def __init__(self, compiler, feature, flag_name, ver_string=None):
             )
             + " implement the {0} property and submit a pull request or issue.".format(flag_name),
         )
+
+
+class CompilerCacheEntry:
+    """Deserialized cache entry for a compiler"""
+
+    __slots__ = ["c_compiler_output", "real_version"]
+
+    def __init__(self, c_compiler_output: Optional[str], real_version: str):
+        self.c_compiler_output = c_compiler_output
+        self.real_version = real_version
+
+    @classmethod
+    def from_dict(cls, data: Dict[str, Optional[str]]):
+        if not isinstance(data, dict):
+            raise ValueError(f"Invalid {cls.__name__} data")
+        c_compiler_output = data.get("c_compiler_output")
+        real_version = data.get("real_version")
+        if not isinstance(real_version, str) or not isinstance(
+            c_compiler_output, (str, type(None))
+        ):
+            raise ValueError(f"Invalid {cls.__name__} data")
+        return cls(c_compiler_output, real_version)
+
+
+class CompilerCache:
+    """Base class for compiler output cache. Default implementation does not cache anything."""
+
+    def value(self, compiler: Compiler) -> Dict[str, Optional[str]]:
+        return {
+            "c_compiler_output": compiler._compile_dummy_c_source(),
+            "real_version": compiler.get_real_version(),
+        }
+
+    def get(self, compiler: Compiler) -> CompilerCacheEntry:
+        return CompilerCacheEntry.from_dict(self.value(compiler))
+
+
+class FileCompilerCache(CompilerCache):
+    """Cache for compiler output, which is used to determine implicit link paths, the default libc
+    version, and the compiler version."""
+
+    name = os.path.join("compilers", "compilers.json")
+
+    def __init__(self, cache: "spack.caches.FileCacheType") -> None:
+        self.cache = cache
+        self.cache.init_entry(self.name)
+        self._data: Dict[str, Dict[str, Optional[str]]] = {}
+
+    def _get_entry(self, key: str) -> Optional[CompilerCacheEntry]:
+        try:
+            return CompilerCacheEntry.from_dict(self._data[key])
+        except ValueError:
+            del self._data[key]
+        except KeyError:
+            pass
+        return None
+
+    def get(self, compiler: Compiler) -> CompilerCacheEntry:
+        # Cache hit
+        try:
+            with self.cache.read_transaction(self.name) as f:
+                assert f is not None
+                self._data = json.loads(f.read())
+                assert isinstance(self._data, dict)
+        except (json.JSONDecodeError, AssertionError):
+            self._data = {}
+
+        key = self._key(compiler)
+        value = self._get_entry(key)
+        if value is not None:
+            return value
+
+        # Cache miss
+        with self.cache.write_transaction(self.name) as (old, new):
+            try:
+                assert old is not None
+                self._data = json.loads(old.read())
+                assert isinstance(self._data, dict)
+            except (json.JSONDecodeError, AssertionError):
+                self._data = {}
+
+            # Use cache entry that may have been created by another process in the meantime.
+            entry = self._get_entry(key)
+
+            # Finally compute the cache entry
+            if entry is None:
+                self._data[key] = self.value(compiler)
+                entry = CompilerCacheEntry.from_dict(self._data[key])
+
+            new.write(json.dumps(self._data, separators=(",", ":")))
+
+            return entry
+
+    def _key(self, compiler: Compiler) -> str:
+        as_bytes = json.dumps(compiler.to_dict(), separators=(",", ":")).encode("utf-8")
+        return hashlib.sha256(as_bytes).hexdigest()
+
+
+def _make_compiler_cache():
+    return FileCompilerCache(spack.caches.MISC_CACHE)
+
+
+COMPILER_CACHE: CompilerCache = llnl.util.lang.Singleton(_make_compiler_cache)  # type: ignore

lib/spack/spack/compilers/aocc.py

@@ -116,5 +116,5 @@ def fflags(self):
     def _handle_default_flag_addtions(self):
         # This is a known issue for AOCC 3.0 see:
         # https://developer.amd.com/wp-content/resources/AOCC-3.0-Install-Guide.pdf
-        if self.real_version.satisfies(ver("3.0.0")):
+        if self.version.satisfies(ver("3.0.0")):
             return "-Wno-unused-command-line-argument " "-mllvm -eliminate-similar-expr=false"

lib/spack/spack/test/compilers/basics.py

@@ -3,8 +3,10 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 """Test basic behavior of compilers in Spack"""
+import json
 import os
 from copy import copy
+from typing import Optional
 
 import pytest
 
@@ -17,6 +19,7 @@
 import spack.util.module_cmd
 from spack.compiler import Compiler
 from spack.util.executable import Executable, ProcessError
+from spack.util.file_cache import FileCache
 
 
 def test_multiple_conflicting_compiler_definitions(mutable_config):
@@ -101,11 +104,14 @@ def verbose_flag(self):
 
 
 @pytest.mark.not_on_windows("Not supported on Windows (yet)")
-def test_implicit_rpaths(dirs_with_libfiles):
+def test_implicit_rpaths(dirs_with_libfiles, monkeypatch):
     lib_to_dirs, all_dirs = dirs_with_libfiles
-    compiler = MockCompiler()
-    compiler._compile_c_source_output = "ld " + " ".join(f"-L{d}" for d in all_dirs)
-    retrieved_rpaths = compiler.implicit_rpaths()
+    monkeypatch.setattr(
+        MockCompiler,
+        "_compile_dummy_c_source",
+        lambda self: "ld " + " ".join(f"-L{d}" for d in all_dirs),
+    )
+    retrieved_rpaths = MockCompiler().implicit_rpaths()
     assert set(retrieved_rpaths) == set(lib_to_dirs["libstdc++"] + lib_to_dirs["libgfortran"])
 
 
@@ -647,6 +653,7 @@ def test_raising_if_compiler_target_is_over_specific(config):
 
 
 @pytest.mark.not_on_windows("Not supported on Windows (yet)")
+@pytest.mark.enable_compiler_execution
 def test_compiler_get_real_version(working_env, monkeypatch, tmpdir):
     # Test variables
     test_version = "2.2.2"
@@ -736,6 +743,7 @@ def test_get_compilers(config):
     ) == [spack.compilers._compiler_from_config_entry(without_suffix)]
 
 
+@pytest.mark.enable_compiler_execution
 def test_compiler_get_real_version_fails(working_env, monkeypatch, tmpdir):
     # Test variables
     test_version = "2.2.2"
@@ -784,15 +792,13 @@ def _call(*args, **kwargs):
     compilers = spack.compilers.get_compilers([compiler_dict])
     assert len(compilers) == 1
     compiler = compilers[0]
-    try:
-        _ = compiler.get_real_version()
-        assert False
-    except ProcessError:
-        # Confirm environment does not change after failed call
-        assert "SPACK_TEST_CMP_ON" not in os.environ
+    assert compiler.get_real_version() == "unknown"
+    # Confirm environment does not change after failed call
+    assert "SPACK_TEST_CMP_ON" not in os.environ
 
 
 @pytest.mark.not_on_windows("Bash scripting unsupported on Windows (for now)")
+@pytest.mark.enable_compiler_execution
 def test_compiler_flags_use_real_version(working_env, monkeypatch, tmpdir):
     # Create compiler
     gcc = str(tmpdir.join("gcc"))
@@ -895,3 +901,57 @@ def test_compiler_environment(working_env):
     )
     with compiler.compiler_environment():
         assert os.environ["TEST"] == "yes"
+
+
+class MockCompilerWithoutExecutables(MockCompiler):
+    def __init__(self):
+        super().__init__()
+        self._compile_dummy_c_source_count = 0
+        self._get_real_version_count = 0
+
+    def _compile_dummy_c_source(self) -> Optional[str]:
+        self._compile_dummy_c_source_count += 1
+        return "gcc helloworld.c -o helloworld"
+
+    def get_real_version(self) -> str:
+        self._get_real_version_count += 1
+        return "1.0.0"
+
+
+def test_compiler_output_caching(tmp_path):
+    """Test that compiler output is cached on the filesystem."""
+    # The first call should trigger the cache to updated.
+    a = MockCompilerWithoutExecutables()
+    cache = spack.compiler.FileCompilerCache(FileCache(str(tmp_path)))
+    assert cache.get(a).c_compiler_output == "gcc helloworld.c -o helloworld"
+    assert cache.get(a).real_version == "1.0.0"
+    assert a._compile_dummy_c_source_count == 1
+    assert a._get_real_version_count == 1
+
+    # The second call on an equivalent but distinct object should not trigger compiler calls.
+    b = MockCompilerWithoutExecutables()
+    cache = spack.compiler.FileCompilerCache(FileCache(str(tmp_path)))
+    assert cache.get(b).c_compiler_output == "gcc helloworld.c -o helloworld"
+    assert cache.get(b).real_version == "1.0.0"
+    assert b._compile_dummy_c_source_count == 0
+    assert b._get_real_version_count == 0
+
+    # Cache schema change should be handled gracefully.
+    with open(cache.cache.cache_path(cache.name), "w") as f:
+        for k in cache._data:
+            cache._data[k] = "corrupted entry"
+        f.write(json.dumps(cache._data))
+
+    c = MockCompilerWithoutExecutables()
+    cache = spack.compiler.FileCompilerCache(FileCache(str(tmp_path)))
+    assert cache.get(c).c_compiler_output == "gcc helloworld.c -o helloworld"
+    assert cache.get(c).real_version == "1.0.0"
+
+    # Cache corruption should be handled gracefully.
+    with open(cache.cache.cache_path(cache.name), "w") as f:
+        f.write("corrupted cache")
+
+    d = MockCompilerWithoutExecutables()
+    cache = spack.compiler.FileCompilerCache(FileCache(str(tmp_path)))
+    assert cache.get(d).c_compiler_output == "gcc helloworld.c -o helloworld"
+    assert cache.get(d).real_version == "1.0.0"

lib/spack/spack/test/concretize.py

@@ -2316,6 +2316,7 @@ def test_compiler_match_constraints_when_selected(self):
 
     @pytest.mark.regression("36339")
     @pytest.mark.not_on_windows("Not supported on Windows")
+    @pytest.mark.enable_compiler_execution
     def test_compiler_with_custom_non_numeric_version(self, mock_executable):
         """Test that, when a compiler has a completely made up version, we can use its
         'real version' to detect targets and don't raise during concretization.