Speed up CI by caching dependencies and virtual environment#606
Speed up CI by caching dependencies and virtual environment#606Sharkyii wants to merge 17 commits intomllam:mainfrom
Conversation
Removed fixed entries for metrics handling and script references from the CHANGELOG.
|
Thanks @Sharkyii 90% faster sounds fantastic! Currently there seems to be some issue with the clean-up of the .venv? |
|
Hi! I took a look at the current approach and noticed a potential issue around Right now, even though the workflow skips venv creation in one place, there are still cases where A more robust approach would be to ensure that venv creation happens strictly in one place and is fully guarded by the cache-hit condition, so that on cache hits we only reuse the restored environment and avoid any reinitialization. I experimented with this in a separate PR: #610 — it demonstrates a cache-aware setup where venv creation is skipped entirely on cache hits and the existing environment is reused safely. Happy to adapt this approach here — if you're okay with it, I can also contribute the changes directly to this branch. |
|
@archit7-beep thanks :) , but currently i am still working on this. |
…-lam into ci-caching-clean # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
|
@sadamov @archit7-beep |
|
This is definitely an improvement over the previous version But I had some suggestion
|
kshirajahere
left a comment
There was a problem hiding this comment.
Nice direction overall, especially adding two-layer caching. Few minor changes and update in /CHANGELOG.md
Co-authored-by: Kshiraja Nelapati <nelapati.kshiraja1@gmail.com>
|
I have now guarded installs with cache hit
The hash is important because, without it, the cache can keep stale packages even after dependencies change. That would mean CI runs against incorrect versions, which defeats the whole point of using a lockfile. The extra 30–60 seconds when dependencies update is a fair trade-off for reliable, deterministic testing. Thanks @archit7-beep @kshirajahere |
|
@sadamov review :) |
There was a problem hiding this comment.
Looks good! And the speedup is impressive @Sharkyii also thanks to @archit7-beep for their review!
I added some inline suggestions below. Noting the seperate PR for uv lockfile here:
Ephemeral uv.lock as cache key: uv lock is generated fresh on every run (no committed lock file, by design). If any upstream dep releases between two CI runs, the hash changes and the venv cache is invalidated. The stated 80-95% speedup may be optimistic during active periods. This will be resolved once #604 lands - it introduces uv sync with CPU/GPU extras and [tool.uv.sources], which enables a committed uv.lock and makes the cache key fully stable.
…jobs Co-authored-by: sadamov <45732287+sadamov@users.noreply.github.com>
Co-authored-by: sadamov <45732287+sadamov@users.noreply.github.com>
sadamov
left a comment
There was a problem hiding this comment.
The two-layer approach (uv download cache via enable-cache: true + explicit .venv cache) is sound. Two issues below.
Both the suggestions fixes some redundant code and simplifies the view. Co-authored-by: sadamov <45732287+sadamov@users.noreply.github.com>
Describe your changes
Adds smart two-layer caching to CI by storing both downloaded packages and the full virtual environment, so builds don’t start from scratch every time.
This significantly reduces install time by reusing dependencies when nothing has changed, while still staying reliable with lock-based cache keys.
Overall, it speeds up CI runs dramatically (up to ~90%) without changing any existing functionality or tests.
Issue Link
#605
Type of change
Checklist before requesting a review
pullwith--rebaseoption if possible).Checklist for reviewers
Each PR comes with its own improvements and flaws. The reviewer should check the following:
Author checklist after completed review
reflecting type of change (add section where missing):
Checklist for assignee