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

More caching work, be explicit about paths, use ccache in cibuildwheel #28

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

timkpaine
Copy link
Member

@timkpaine timkpaine commented Feb 6, 2024

As per title, more work to ensure Linux builds in particular are properly caching vcpkg and ccache artifacts.

@timkpaine timkpaine added the tag: internal Issues and PRs for maintainance of the project - not interesting to external users label Feb 6, 2024
@timkpaine
Copy link
Member Author

timkpaine commented Feb 6, 2024

Looks like caches are coming back now for linux:
Screenshot 2024-02-06 at 15 54 19

Will have to see if we can afford to cache per-python or if just caching per os/arch is sufficient.

@ngoldbaum
Copy link
Collaborator

I think you need to make the caches per-python-version, otherwise you get errors like this one from the post setup cache step for the python 3.10 run:

Failed to save: Unable to reserve cache with key ccache-ccache-Linux-X64-, another job may be creating this cache. More details: Cache already exists. Scope: refs/pull/28/merge, Key: ccache-ccache-Linux-X64-, Version: 15c80211763d03468c2c9070680654f9264282cf4daa2c6ceac80f2e3eaeb295

@timkpaine
Copy link
Member Author

I think you need to make the caches per-python-version, otherwise you get errors like this one from the post setup cache step for the python 3.10 run:

Failed to save: Unable to reserve cache with key ccache-ccache-Linux-X64-, another job may be creating this cache. More details: Cache already exists. Scope: refs/pull/28/merge, Key: ccache-ccache-Linux-X64-, Version: 15c80211763d03468c2c9070680654f9264282cf4daa2c6ceac80f2e3eaeb295

That shouldn't actually matter as so long as one of them writes it, stuff will be saved. But the compilation relies on python headers, so by not saving per-python we have to recompile more than we would otherwise. The problem with saving per-python is we take up more cache space overall, which increases the likelihood of something we need being evicted.

@timkpaine
Copy link
Member Author

rerunning all jobs to see if these changes had any noticeable effect 😓

@timkpaine
Copy link
Member Author

Seems to be doing the trick but I'm already anticipating some more caching PRs 😅

@timkpaine timkpaine marked this pull request as ready for review February 7, 2024 00:42
Copy link
Collaborator

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, much faster!

@ngoldbaum ngoldbaum merged commit a377395 into main Feb 7, 2024
10 checks passed
@ngoldbaum ngoldbaum deleted the tkp/cache3 branch February 7, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: internal Issues and PRs for maintainance of the project - not interesting to external users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants