Improve hit-rate on buildcaches (#43272)

* Relax compiler and target mismatches

The mismatch occurs on an edge. Previously it was assigned
the parent priority, now it is assigned the child priority.

This should make reuse from buildcaches or store more likely,
since most mismatches will be counted with "reused" priority.

* Optimize version badness for runtimes at very low priority

We don't want to e.g. switch other attributes because we
cannot reuse an old installed runtime.

* Optimize runtime attributes at very low priority

This is such that the version of the runtime would
not influence whether we should reuse a spec.

Compiler mismatches are considered for runtimes,
to avoid situations where compiling foo%gcc@9
brings in gcc-runtime%gcc@13 if gcc@13 is among
the available compilers

* Exclude specs without runtimes from reuse

This should ensure that we do not reuse specs that
could be broken, as they expect the compiler to be
installed in a specific place.

lib/spack/spack/solver/asp.py

@@ -3371,7 +3371,7 @@ def _is_reusable(spec: spack.spec.Spec, packages, local: bool) -> bool:
         return False
 
     if not spec.external:
-        return True
+        return _has_runtime_dependencies(spec)
 
     # Cray external manifest externals are always reusable
     if local:
@@ -3396,6 +3396,19 @@ def _is_reusable(spec: spack.spec.Spec, packages, local: bool) -> bool:
     return False
 
 
+def _has_runtime_dependencies(spec: spack.spec.Spec) -> bool:
+    if not WITH_RUNTIME:
+        return True
+
+    if spec.compiler.name == "gcc" and not spec.dependencies("gcc-runtime"):
+        return False
+
+    if spec.compiler.name == "oneapi" and not spec.dependencies("intel-oneapi-runtime"):
+        return False
+
+    return True
+
+
 class Solver:
     """This is the main external interface class for solving.
 

lib/spack/spack/solver/concretize.lp

@@ -80,6 +80,7 @@ unification_set(SetID, VirtualNode)
 
 
 #defined multiple_unification_sets/1.
+#defined runtime/1.
 
 %----
 % Rules to break symmetry and speed-up searches
@@ -1494,18 +1495,20 @@ opt_criterion(40, "compiler mismatches that are not from CLI").
 #minimize{ 0@240: #true }.
 #minimize{ 0@40: #true }.
 #minimize{
-    1@40+Priority,PackageNode,DependencyNode
+    1@40+Priority,PackageNode,node(ID, Dependency)
-    : compiler_mismatch(PackageNode, DependencyNode),
+    : compiler_mismatch(PackageNode, node(ID, Dependency)),
-      build_priority(PackageNode, Priority)
+      build_priority(node(ID, Dependency), Priority),
+      not runtime(Dependency)
 }.
 
 opt_criterion(39, "compiler mismatches that are not from CLI").
 #minimize{ 0@239: #true }.
 #minimize{ 0@39: #true }.
 #minimize{
-    1@39+Priority,PackageNode,DependencyNode
+    1@39+Priority,PackageNode,node(ID, Dependency)
-    : compiler_mismatch_required(PackageNode, DependencyNode),
+    : compiler_mismatch_required(PackageNode, node(ID, Dependency)),
-      build_priority(PackageNode, Priority)
+      build_priority(node(ID, Dependency), Priority),
+      not runtime(Dependency)
 }.
 
 opt_criterion(30, "non-preferred OS's").
@@ -1522,9 +1525,10 @@ opt_criterion(25, "version badness").
 #minimize{ 0@225: #true }.
 #minimize{ 0@25: #true }.
 #minimize{
-    Weight@25+Priority,PackageNode
+    Weight@25+Priority,node(X, Package)
-    : version_weight(PackageNode, Weight),
+    : version_weight(node(X, Package), Weight),
-      build_priority(PackageNode, Priority)
+      build_priority(node(X, Package), Priority),
+      not runtime(Package)
 }.
 
 % Try to use all the default values of variants
@@ -1543,9 +1547,10 @@ opt_criterion(15, "non-preferred compilers").
 #minimize{ 0@215: #true }.
 #minimize{ 0@15: #true }.
 #minimize{
-    Weight@15+Priority,PackageNode
+    Weight@15+Priority,node(X, Package)
-    : node_compiler_weight(PackageNode, Weight),
+    : node_compiler_weight(node(X, Package), Weight),
-      build_priority(PackageNode, Priority)
+      build_priority(node(X, Package), Priority),
+      not runtime(Package)
 }.
 
 % Minimize the number of mismatches for targets in the DAG, try
@@ -1554,18 +1559,55 @@ opt_criterion(10, "target mismatches").
 #minimize{ 0@210: #true }.
 #minimize{ 0@10: #true }.
 #minimize{
-    1@10+Priority,PackageNode,Dependency
+    1@10+Priority,PackageNode,node(ID, Dependency)
-    : node_target_mismatch(PackageNode, Dependency),
+    : node_target_mismatch(PackageNode, node(ID, Dependency)),
-      build_priority(PackageNode, Priority)
+      build_priority(node(ID, Dependency), Priority),
+      not runtime(Dependency)
 }.
 
 opt_criterion(5, "non-preferred targets").
 #minimize{ 0@205: #true }.
 #minimize{ 0@5: #true }.
 #minimize{
-    Weight@5+Priority,PackageNode
+    Weight@5+Priority,node(X, Package)
-    : node_target_weight(PackageNode, Weight),
+    : node_target_weight(node(X, Package), Weight),
-      build_priority(PackageNode, Priority)
+      build_priority(node(X, Package), Priority),
+      not runtime(Package)
+}.
+
+
+% Minimize the number of compiler mismatches for runtimes
+opt_criterion(4, "compiler mismatches (runtimes)").
+#minimize{ 0@204: #true }.
+#minimize{ 0@4: #true }.
+#minimize{
+    1@4,PackageNode,node(ID, Dependency)
+    : compiler_mismatch(PackageNode, node(ID, Dependency)), runtime(Dependency)
+}.
+#minimize{
+    1@4,PackageNode,node(ID, Dependency)
+    : compiler_mismatch_required(PackageNode, node(ID, Dependency)), runtime(Dependency)
+}.
+
+
+% Choose more recent versions for runtimes
+opt_criterion(3, "version badness (runtimes)").
+#minimize{ 0@203: #true }.
+#minimize{ 0@3: #true }.
+#minimize{
+    Weight@3,node(X, Package)
+    : version_weight(node(X, Package), Weight),
+      runtime(Package)
+}.
+
+% Choose best target for runtimes
+opt_criterion(2, "non-preferred targets (runtimes)").
+#minimize{ 0@202: #true }.
+#minimize{ 0@2: #true }.
+#minimize{
+    Weight@2,node(X, Package)
+    : node_target_weight(node(X, Package), Weight),
+      runtime(Package)
 }.
 
 % Choose more recent versions for nodes

lib/spack/spack/target.py

@@ -142,7 +142,7 @@ def optimization_flags(self, compiler):
         # custom spec.
         compiler_version = compiler.version
         version_number, suffix = archspec.cpu.version_components(compiler.version)
-        if not version_number or suffix not in ("", "apple"):
+        if not version_number or suffix:
             # Try to deduce the underlying version of the compiler, regardless
             # of its name in compilers.yaml. Depending on where this function
             # is called we might get either a CompilerSpec or a fully fledged

lib/spack/spack/test/cmd/compiler.py

@@ -112,10 +112,10 @@ def test_compiler_find_no_apple_gcc(no_compilers_yaml, working_env, mock_executa
 @pytest.mark.regression("37996")
 def test_compiler_remove(mutable_config, mock_packages):
     """Tests that we can remove a compiler from configuration."""
-    assert spack.spec.CompilerSpec("gcc@=4.8.0") in spack.compilers.all_compiler_specs()
+    assert spack.spec.CompilerSpec("gcc@=9.4.0") in spack.compilers.all_compiler_specs()
-    args = spack.util.pattern.Bunch(all=True, compiler_spec="gcc@4.8.0", add_paths=[], scope=None)
+    args = spack.util.pattern.Bunch(all=True, compiler_spec="gcc@9.4.0", add_paths=[], scope=None)
     spack.cmd.compiler.compiler_remove(args)
-    assert spack.spec.CompilerSpec("gcc@=4.8.0") not in spack.compilers.all_compiler_specs()
+    assert spack.spec.CompilerSpec("gcc@=9.4.0") not in spack.compilers.all_compiler_specs()
 
 
 @pytest.mark.regression("37996")
@@ -124,10 +124,10 @@ def test_removing_compilers_from_multiple_scopes(mutable_config, mock_packages):
     site_config = spack.config.get("compilers", scope="site")
     spack.config.set("compilers", site_config, scope="user")
 
-    assert spack.spec.CompilerSpec("gcc@=4.8.0") in spack.compilers.all_compiler_specs()
+    assert spack.spec.CompilerSpec("gcc@=9.4.0") in spack.compilers.all_compiler_specs()
-    args = spack.util.pattern.Bunch(all=True, compiler_spec="gcc@4.8.0", add_paths=[], scope=None)
+    args = spack.util.pattern.Bunch(all=True, compiler_spec="gcc@9.4.0", add_paths=[], scope=None)
     spack.cmd.compiler.compiler_remove(args)
-    assert spack.spec.CompilerSpec("gcc@=4.8.0") not in spack.compilers.all_compiler_specs()
+    assert spack.spec.CompilerSpec("gcc@=9.4.0") not in spack.compilers.all_compiler_specs()
 
 
 @pytest.mark.not_on_windows("Cannot execute bash script on Windows")

lib/spack/spack/test/cmd/spec.py

@@ -31,7 +31,7 @@ def test_spec():
 
 
 @pytest.mark.only_clingo("Known failure of the original concretizer")
-def test_spec_concretizer_args(mutable_config, mutable_database):
+def test_spec_concretizer_args(mutable_config, mutable_database, do_not_check_runtimes_on_reuse):
     """End-to-end test of CLI concretizer prefs.
 
     It's here to make sure that everything works from CLI

lib/spack/spack/test/concretize.py

@@ -254,7 +254,7 @@ def gcc11_with_flags(compiler_factory):
 # This must use the mutable_config fixture because the test
 # adjusting_default_target_based_on_compiler uses the current_host fixture,
 # which changes the config.
-@pytest.mark.usefixtures("mutable_config", "mock_packages")
+@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse")
 class TestConcretize:
     def test_concretize(self, spec):
         check_concretize(spec)
@@ -614,7 +614,7 @@ def test_my_dep_depends_on_provider_of_my_virtual_dep(self):
         spec.normalize()
         spec.concretize()
 
-    @pytest.mark.parametrize("compiler_str", ["clang", "gcc", "gcc@10.2.1", "clang@:12.0.0"])
+    @pytest.mark.parametrize("compiler_str", ["clang", "gcc", "gcc@10.2.1", "clang@:15.0.0"])
     def test_compiler_inheritance(self, compiler_str):
         spec_str = "mpileaks %{0}".format(compiler_str)
         spec = Spec(spec_str).concretized()
@@ -877,7 +877,7 @@ def test_concretize_anonymous_dep(self, spec_str):
             # Unconstrained versions select default compiler (gcc@4.5.0)
             ("bowtie@1.4.0", "%gcc@10.2.1"),
             # Version with conflicts and no valid gcc select another compiler
-            ("bowtie@1.3.0", "%clang@12.0.0"),
+            ("bowtie@1.3.0", "%clang@15.0.0"),
             # If a higher gcc is available still prefer that
             ("bowtie@1.2.2 os=redhat6", "%gcc@11.1.0"),
         ],
@@ -1439,7 +1439,7 @@ def test_os_selection_when_multiple_choices_are_possible(
     @pytest.mark.regression("22718")
     @pytest.mark.parametrize(
         "spec_str,expected_compiler",
-        [("mpileaks", "%gcc@10.2.1"), ("mpileaks ^mpich%clang@12.0.0", "%clang@12.0.0")],
+        [("mpileaks", "%gcc@10.2.1"), ("mpileaks ^mpich%clang@15.0.0", "%clang@15.0.0")],
     )
     def test_compiler_is_unique(self, spec_str, expected_compiler):
         s = Spec(spec_str).concretized()
@@ -1727,7 +1727,7 @@ def test_reuse_with_unknown_package_dont_raise(self, tmpdir, temporary_store, mo
         [
             (["libelf", "libelf@0.8.10"], 1),
             (["libdwarf%gcc", "libelf%clang"], 2),
-            (["libdwarf%gcc", "libdwarf%clang"], 4),
+            (["libdwarf%gcc", "libdwarf%clang"], 3),
             (["libdwarf^libelf@0.8.12", "libdwarf^libelf@0.8.13"], 4),
             (["hdf5", "zmpi"], 3),
             (["hdf5", "mpich"], 2),

lib/spack/spack/test/concretize_compiler_runtimes.py

@@ -7,6 +7,8 @@
 
 import pytest
 
+import archspec.cpu
+
 import spack.paths
 import spack.repo
 import spack.solver.asp
@@ -24,9 +26,7 @@ def _concretize_with_reuse(*, root_str, reused_str):
     reused_spec = spack.spec.Spec(reused_str).concretized()
     setup = spack.solver.asp.SpackSolverSetup(tests=False)
     driver = spack.solver.asp.PyclingoDriver()
-    result, _, _ = driver.solve(
+    result, _, _ = driver.solve(setup, [spack.spec.Spec(f"{root_str}")], reuse=[reused_spec])
-        setup, [spack.spec.Spec(f"{root_str} ^{reused_str}")], reuse=[reused_spec]
-    )
     root = result.specs[0]
     return root, reused_spec
 
@@ -47,7 +47,7 @@ def enable_runtimes():
 
 
 def test_correct_gcc_runtime_is_injected_as_dependency(runtime_repo):
-    s = spack.spec.Spec("a%gcc@10.2.1 ^b%gcc@4.8.0").concretized()
+    s = spack.spec.Spec("a%gcc@10.2.1 ^b%gcc@9.4.0").concretized()
     a, b = s["a"], s["b"]
 
     # Both a and b should depend on the same gcc-runtime directly
@@ -78,9 +78,28 @@ def test_external_nodes_do_not_have_runtimes(runtime_repo, mutable_config, tmp_p
     "root_str,reused_str,expected,nruntime",
     [
         # The reused runtime is older than we need, thus we'll add a more recent one for a
-        ("a%gcc@10.2.1", "b%gcc@4.8.0", {"a": "gcc-runtime@10.2.1", "b": "gcc-runtime@4.8.0"}, 2),
+        ("a%gcc@10.2.1", "b%gcc@9.4.0", {"a": "gcc-runtime@10.2.1", "b": "gcc-runtime@9.4.0"}, 2),
         # The root is compiled with an older compiler, thus we'll reuse the runtime from b
-        ("a%gcc@4.8.0", "b%gcc@10.2.1", {"a": "gcc-runtime@10.2.1", "b": "gcc-runtime@10.2.1"}, 1),
+        ("a%gcc@9.4.0", "b%gcc@10.2.1", {"a": "gcc-runtime@10.2.1", "b": "gcc-runtime@10.2.1"}, 1),
+        # Same as before, but tests that we can reuse from a more generic target
+        pytest.param(
+            "a%gcc@9.4.0",
+            "b%gcc@10.2.1 target=x86_64",
+            {"a": "gcc-runtime@10.2.1 target=x86_64", "b": "gcc-runtime@10.2.1 target=x86_64"},
+            1,
+            marks=pytest.mark.skipif(
+                str(archspec.cpu.host().family) != "x86_64", reason="test data is x86_64 specific"
+            ),
+        ),
+        pytest.param(
+            "a%gcc@10.2.1",
+            "b%gcc@9.4.0 target=x86_64",
+            {"a": "gcc-runtime@10.2.1 target=x86_64", "b": "gcc-runtime@9.4.0 target=x86_64"},
+            2,
+            marks=pytest.mark.skipif(
+                str(archspec.cpu.host().family) != "x86_64", reason="test data is x86_64 specific"
+            ),
+        ),
     ],
 )
 def test_reusing_specs_with_gcc_runtime(root_str, reused_str, expected, nruntime, runtime_repo):
@@ -104,8 +123,8 @@ def test_reusing_specs_with_gcc_runtime(root_str, reused_str, expected, nruntime
     [
         # Ensure that, whether we have multiple runtimes in the DAG or not,
         # we always link only the latest version
-        ("a%gcc@10.2.1", "b%gcc@4.8.0", ["gcc-runtime@10.2.1"], ["gcc-runtime@4.8.0"]),
+        ("a%gcc@10.2.1", "b%gcc@9.4.0", ["gcc-runtime@10.2.1"], ["gcc-runtime@9.4.0"]),
-        ("a%gcc@4.8.0", "b%gcc@10.2.1", ["gcc-runtime@10.2.1"], ["gcc-runtime@4.8.0"]),
+        ("a%gcc@9.4.0", "b%gcc@10.2.1", ["gcc-runtime@10.2.1"], ["gcc-runtime@9.4.0"]),
     ],
 )
 def test_views_can_handle_duplicate_runtime_nodes(

lib/spack/spack/test/concretize_preferences.py

@@ -105,7 +105,7 @@ def test_preferred_variants_from_wildcard(self):
 
     @pytest.mark.parametrize(
         "compiler_str,spec_str",
-        [("gcc@=4.8.0", "mpileaks"), ("clang@=12.0.0", "mpileaks"), ("gcc@=4.8.0", "openmpi")],
+        [("gcc@=9.4.0", "mpileaks"), ("clang@=15.0.0", "mpileaks"), ("gcc@=9.4.0", "openmpi")],
     )
     def test_preferred_compilers(self, compiler_str, spec_str):
         """Test preferred compilers are applied correctly"""

lib/spack/spack/test/conftest.py

@@ -2013,3 +2013,12 @@ def _factory(*, spec, operating_system):
 def host_architecture_str():
     """Returns the broad architecture family (x86_64, aarch64, etc.)"""
     return str(archspec.cpu.host().family)
+
+
+def _true(x):
+    return True
+
+
+@pytest.fixture()
+def do_not_check_runtimes_on_reuse(monkeypatch):
+    monkeypatch.setattr(spack.solver.asp, "_has_runtime_dependencies", _true)

lib/spack/spack/test/data/config/compilers.yaml

@@ -1,6 +1,6 @@
 compilers:
 - compiler:
-    spec: gcc@=4.8.0
+    spec: gcc@=9.4.0
     operating_system: {linux_os.name}{linux_os.version}
     paths:
       cc: /path/to/gcc
@@ -10,7 +10,7 @@ compilers:
     modules: []
     target: {target}
 - compiler:
-    spec: gcc@=4.8.0
+    spec: gcc@=9.4.0
     operating_system: redhat6
     paths:
       cc: /path/to/gcc
@@ -20,7 +20,7 @@ compilers:
     modules: []
     target: {target}
 - compiler:
-    spec: clang@=12.0.0
+    spec: clang@=15.0.0
     operating_system: {linux_os.name}{linux_os.version}
     paths:
       cc: /path/to/clang

lib/spack/spack/test/data/config/packages.yaml

@@ -16,7 +16,7 @@ packages:
   externalvirtual:
     buildable: False
     externals:
-    - spec: externalvirtual@2.0%clang@12.0.0
+    - spec: externalvirtual@2.0%clang@15.0.0
       prefix: /path/to/external_virtual_clang
     - spec: externalvirtual@1.0%gcc@10.2.1
       prefix: /path/to/external_virtual_gcc

lib/spack/spack/test/data/modules/lmod/complex_hierarchy.yaml

@@ -4,7 +4,7 @@ lmod:
   hash_length: 0
 
   core_compilers:
-    - 'clang@12.0.0'
+    - 'clang@15.0.0'
 
   core_specs:
     - 'mpich@3.0.1'

lib/spack/spack/test/data/modules/lmod/core_compilers.yaml

@@ -2,4 +2,4 @@ enable:
   - lmod
 lmod:
   core_compilers:
-    - 'clang@12.0.0'
+    - 'clang@15.0.0'

lib/spack/spack/test/data/modules/lmod/core_compilers_at_equal.yaml

@@ -2,4 +2,4 @@ enable:
   - lmod
 lmod:
   core_compilers:
-    - 'clang@=12.0.0'
+    - 'clang@=15.0.0'

lib/spack/spack/test/modules/lmod.py

@@ -29,7 +29,7 @@
 ]
 
 
-@pytest.fixture(params=["clang@=12.0.0", "gcc@=10.2.1"])
+@pytest.fixture(params=["clang@=15.0.0", "gcc@=10.2.1"])
 def compiler(request):
     return request.param
 
@@ -59,7 +59,7 @@ def test_layout_for_specs_compiled_with_core_compilers(
         we can use both ``compiler@version`` and ``compiler@=version`` to specify a core compiler.
         """
         module_configuration(modules_config)
-        module, spec = factory("libelf%clang@12.0.0")
+        module, spec = factory("libelf%clang@15.0.0")
         assert "Core" in module.layout.available_path_parts
 
     def test_file_layout(self, compiler, provider, factory, module_configuration):
@@ -78,7 +78,7 @@ def test_file_layout(self, compiler, provider, factory, module_configuration):
         # is transformed to r"Core" if the compiler is listed among core
         # compilers
         # Check that specs listed as core_specs are transformed to "Core"
-        if compiler == "clang@=12.0.0" or spec_string == "mpich@3.0.1":
+        if compiler == "clang@=15.0.0" or spec_string == "mpich@3.0.1":
             assert "Core" in layout.available_path_parts
         else:
             assert compiler.replace("@=", "/") in layout.available_path_parts