Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve hit-rate on buildcaches #43272

Merged
merged 9 commits into from
Apr 5, 2024

Conversation

alalazo
Copy link
Member

@alalazo alalazo commented Mar 19, 2024

This PR tries to modify the optimization criteria to have more hits on buildcaches by default.

Modifications:

  • Assign compiler and target mismatches to the child node of the edge with a mismatch (previously was the parent node)
  • Move optimization of runtime nodes at very low priority, so that it doesn't interfere on whether specs are reused or not
  • Prevent specs from pre-gcc-runtime to be reused (they'll be mostly broken at runtime)
  • Bumped compiler versions in unit tests to avoid issues where a compiler doesn't support some newer targets (Apple M1)

@spackbot-app spackbot-app bot added core PR affects Spack core functionality tests General test capability(ies) labels Mar 19, 2024
@haampie
Copy link
Member

haampie commented Mar 19, 2024

to the child node

Smart. On second thought: does it work with externals?

@alalazo alalazo force-pushed the solver/relax-os-and-target-matching branch from 06c0c69 to db6f8f9 Compare March 20, 2024 08:59
@alalazo alalazo force-pushed the solver/relax-os-and-target-matching branch from db6f8f9 to 2379050 Compare March 20, 2024 11:15
@alalazo
Copy link
Member Author

alalazo commented Mar 20, 2024

Example of the current state (the occasional icelake nodes are the ones not in the buildcache):
2024-03-20_22-59

@alalazo alalazo force-pushed the solver/relax-os-and-target-matching branch from 630a5fe to 5c5de4d Compare March 21, 2024 16:04
@alalazo
Copy link
Member Author

alalazo commented Mar 21, 2024

The changes here give a much better hit rate on store / buildcaches. I tried to benchmark on a baseline without any mirror and it's mostly the same. There's one consistent spike I need to investigate further:

radiuss.pr.nomirror.csv
radiuss.develop.nomirror.csv
radiuss.txt

radiuss

@alalazo alalazo marked this pull request as ready for review March 21, 2024 16:54
@alalazo alalazo force-pushed the solver/relax-os-and-target-matching branch from 5c5de4d to ed006df Compare March 25, 2024 10:28
@alalazo alalazo force-pushed the solver/relax-os-and-target-matching branch from ed006df to f550c55 Compare March 28, 2024 17:11
The mismatch occurs on an edge. Previously it was assigned
the parent priority, now it is assigned to the child
priority.

This should make reuse from buildcaches or store more likely,
since most mismatches will be counted with "reused" priority.
We don't want to e.g. switch other attributes because we
cannot reuse an old installed runtime.
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
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.
This is to avoid differences on m1
with respect to x86_64 machines
This is to avoid differences on m1
with respect to x86_64
@alalazo alalazo force-pushed the solver/relax-os-and-target-matching branch from 5376bef to 523c704 Compare March 29, 2024 10:52
@alalazo
Copy link
Member Author

alalazo commented Apr 2, 2024

I just tried to re-measure the benchmark above. There must be something in the hiptt spec that makes it very sensible to changes in the configuration1. For what is worth, I still see a spike, but this time in the other direction. As before the difference is always coming from the "solve" part.

  • Spack: 0.22.0.dev0 (523c704)
  • Python: 3.11.5
  • Platform: linux-ubuntu20.04-icelake
  • Concretizer: clingo

radiuss.pr.nomirror.csv
radiuss.develop.nomirror.csv
radiuss.txt

radiuss

Footnotes

  1. I still have to check what is the trigger. This time I have a few more compilers registered than in the test above.

@alalazo alalazo changed the title Relax matching constraints for reused specs Improve hit-rate on buildcaches Apr 5, 2024
@haampie
Copy link
Member

haampie commented Apr 5, 2024

I think this is fine. As per discussion in dm, users can always require: %gcc@13 to avoid incompatible mixing in case of fortran modules.

But we have to document it (in high impact changes for instance).

@alalazo alalazo added this to the v0.22.0 milestone Apr 5, 2024
@alalazo alalazo merged commit 826e0c0 into spack:develop Apr 5, 2024
33 of 35 checks passed
@alalazo alalazo deleted the solver/relax-os-and-target-matching branch April 5, 2024 18:10
tldahlgren pushed a commit to tldahlgren/spack that referenced this pull request Apr 23, 2024
* 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.
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Jun 15, 2024
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commands core PR affects Spack core functionality tests General test capability(ies)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants