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 generate-assets output in CI #1081

Merged
merged 8 commits into from Mar 9, 2024

Conversation

BD103
Copy link
Member

@BD103 BD103 commented Mar 8, 2024

Closes #1066.

This adds a cache for generate-assets, which is the CI job currently takes the longest at ~7 minutes, and reduces its runtime to 11 seconds. The cache is updated ever time the website is deployed using deploy.yml. The cache is then used in ci.yml when no run in the merge queue.

@BD103 BD103 added C-Enhancement New feature or request A-Build-System labels Mar 8, 2024
@BD103 BD103 requested a review from mockersf March 8, 2024 15:21
@BD103 BD103 marked this pull request as ready for review March 8, 2024 15:21
Copy link
Member

@TrialDragon TrialDragon left a comment

Choose a reason for hiding this comment

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

It looks good from the workflow. I'm still a bit hesitant since we can't test saving the cache until deployment.

@BD103
Copy link
Member Author

BD103 commented Mar 9, 2024

One thing I do need to check is whether actions/cache/save overwrites the previous cache of the same key. If it doesn't, I'll need to use the Github API to delete the old cache before creating a new one. (And that could lead to data races with multiple runners, if not handled properly.)

Edit:

This is in fact an issue. Caches cannot be overwritten once created.

image

I'm going to fix it by adding the date to the cache id, so it updates weekly.

@BD103
Copy link
Member Author

BD103 commented Mar 9, 2024

We did some testing (thanks @TrialDragon!). Saving and using the cache worked well, but we were unable to test using it with merge queues because it is a feature restricted to Github orgs and private repos.

Another note: using the cache reduces generate-assets times to 11 seconds!

image

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 9, 2024
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review Ready for a maintainer to consider for merging label Mar 9, 2024
@alice-i-cecile alice-i-cecile removed this pull request from the merge queue due to a manual request Mar 9, 2024
@BD103 BD103 marked this pull request as draft March 9, 2024 01:55
Caches cannot be overwritten after their creation, so the date ensures the cache is refreshed daily. This is especially true since `deploy.yml` is scheduled to run daily anyways, so there will always be a cache available for `ci.yml`.
I don't think `deploy.yml` should use any cached data, in case it ever becomes invalid. This is why `deploy.yml` only stores the cache. By deleting the crates.io cache, it may increase deploy times. If this becomes an issue, I'll make a follow-up PR and undo this change.
@BD103 BD103 marked this pull request as ready for review March 9, 2024 02:32
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 9, 2024
Merged via the queue into bevyengine:main with commit cbd158c Mar 9, 2024
10 checks passed
@BD103 BD103 deleted the assets-cache branch March 9, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System C-Enhancement New feature or request S-Ready-For-Final-Review Ready for a maintainer to consider for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce generate-assets duration
4 participants