-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Open
Labels
A-infraArea: CI issues and issues that require full access for GitHub/CIArea: CI issues and issues that require full access for GitHub/CIG-performance-projectGoal: For issues and PRs related to the Clippy Performance ProjectGoal: For issues and PRs related to the Clippy Performance Project
Description
Description
We're currently only using cache on the lintcheck.yml
workflow. This means that we're downloading and building the same crates a lot. We currently use about 50s on that Build step. About a third of the CI time.
We should use the rust-cache
Action for this purpose.
Version
NA
Additional Labels
No response
Metadata
Metadata
Assignees
Labels
A-infraArea: CI issues and issues that require full access for GitHub/CIArea: CI issues and issues that require full access for GitHub/CIG-performance-projectGoal: For issues and PRs related to the Clippy Performance ProjectGoal: For issues and PRs related to the Clippy Performance Project
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
BD103 commentedon Nov 26, 2024
Can I suggest
Leafwing-Studios/cargo-cache
for this purpose? It works the same asrust-cache
, but also sweeps thetarget
directory to remove stale artifacts that are not used.Even if that's decided against, I'm still interested in working on this issue as a whole. Just give me a 👍 to get started :)
blyxyas commentedon Nov 26, 2024
If you think that
Leafwing-Studios/cargo-cache
would fit better, feel free to use it! ❤blyxyas commentedon Nov 27, 2024
Btw, if you open a PR, remember to assign it to me by writing "r? @blyxyas" as the last line of the PR description.
flip1995 commentedon Dec 4, 2024
Please keep me in the loop with CI changes. I'm usually auto-assigned to PRs touching those files, if you don't add an explicit
r?
.BD103 commentedon May 10, 2025
It's been a few months since I said this, and I no longer believe
Leafwing-Studios/cargo-cache
is the best choice.Swatinem/rust-cache
now also sweeps thetarget
directory of stale artifacts, is better maintained, and has a few extra features that make it better.As a side note, when building in CI, I highly recommend setting
-C debuginfo=line-tables-only
. This option provides enough debug info for backtraces, but avoids including extra debug info that can only be accessed with a debugger likelldb
orgdb
. This should overall decrease cache sizes without hurting the ability to diagnose issues in CI :)Kobzol commentedon May 10, 2025
Note that since Clippy uses GitHub merge queues, any caching will be mostly useless, because only multiple pushes to the same PR will be cached. To apply caching in the merge queue itself, or on first pushes to PRs, you would need to run duplicated CI on the
master
branch (see https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Help.20with.20rust-analyzer's.20CI/with/510973141).BD103 commentedon May 13, 2025
Good to know! The repos I maintain run CI on
main
and only save the cache when run on themain
branch, so they can be shared by all PRs. Is there are particular reason CI isn't run onmaster
for Clippy?blyxyas commentedon May 13, 2025
We din't really need to update it in each PR, just in dependency changes (which are pretty rare).
Dependencies like askama or toml (and potentially clippy_dev) really could be optimized because we are downloading and building the same 500Kb of code again and again for each PR. I wasn't talking about caching the clippy project itself, I don't think that that's worth it.
flip1995 commentedon May 13, 2025
We're using GitHub merge queue. There the commit is tested on CI while on the temporary merge-queue branch. When this commit is then merged into master, it will have the same commit hash, so there won't be a CI run for it on
master
again.Caching merge-queue CI runs can also be problematic, should another workflow fail, the PR not get merged and the cache of a faster workflow uploaded.
Kobzol commentedon May 13, 2025
I don't think that there are issues with updating the cache on failed builds, by default the cache isn't even written on failure I think.
There's no need to do it, but if you want to get caching working with merge queues, clippy could run CI on master. It would kind of needlessly duplicate everything though.