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

Cache Headers for CI #195

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Cache Headers for CI #195

wants to merge 4 commits into from

Conversation

Kerilk
Copy link
Contributor

@Kerilk Kerilk commented Oct 19, 2022

This PR caches the result of the Headers install rather than checking them out, configuring them, and rebuilding them (and their tests) for each job. The cache depends on the main last commit, and thus should be refreshed if the main branch of the Headers is updated.

This PR yields significant CI performance improvements even when the cache is refreshed.
Quantifying globally is difficult given that the number of runners vary, but based on individual task runtimes, speedups I observed are:

  • Linux ~1.5
  • MacOS between 1.5 and 2
  • Windows between 2 and 3

@Kerilk Kerilk requested a review from MathiasMagnus October 19, 2022 05:11
@bashbaug
Copy link
Contributor

bashbaug commented Nov 7, 2022

Out of curiosity, did you do any experiments disabling testing for the headers (by passing -DBUILD_TESTING=0 to CMake when building the headers)? I'm wondering if that will provide similar benefits without requiring so many changes to CI.

@Kerilk
Copy link
Contributor Author

Kerilk commented Nov 8, 2022

You are correct that it would save a lot of time. But I also looked at those numbers, and the overhead of cloning/configuring/building (without test)/Installing is quite significant. We could still disable testing when building the cache to save additional time when the refresh is required. @MathiasMagnus should chime in as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants